FP 2094 - Empty record

Clean Code Report False-positive / False-negative...
1

Then tell us:

  • What language is this for? - c#
  • Which rule? S2094 C# static code analysis
  • Why do you believe it’s a false-positive/false-negative? The rule doesn’t account for how Equals and GetHashCode are overridden on record types.
  • Are you using
    • SonarQube for IDE - which IDE/version? - VS2022 / SonarQube for Visual Studio 2022 v8.9.0.11507
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)

When I create an empty record SonarQube reports issue S2094 saying there is no reason to have an empty class, and that it should be an interface instead.

However, the benefit of using an empty record is that I can then check if two instances of that type are equal and it will then check if the two instances have the same type and if yes do they have the same property values. This is helpful when using things like Dictionaries.

So in a sense, the record is not an empty class, but comes with an invisible overriden implementation of Equals and GetHashCode.

For instance, if you run the following code you will see it prints true when the variables are declared using record types, but false when the variables are declared using the interface.

using System;

public class Program
{
	public static void Main()
	{
		Console.WriteLine(new EmptyRecord() == new EmptyRecord()); // True
		
		AbstractRecord recordFromAbstractA = new Record("Test");
		AbstractRecord recordFromAbstractB = new Record("Test");

		Console.WriteLine(recordFromAbstractA == recordFromAbstractB); // True

		IInterface recordFromInterfaceA = new Record("Test");
		IInterface recordFromInterfaceB = new Record("Test");

		Console.WriteLine(recordFromInterfaceA == recordFromInterfaceB); // False
	}

	private interface IInterface;
	private abstract record AbstractRecord;

	private record Record(string Name) : AbstractRecord, IInterface;
		
	private record EmptyRecord;
}

Therefore I suggest disabling the rule for record types.

4

Hi @RJM,

welcome to our community!
I don’t fully get the reasoning behind this, what would be the use case exactly?
I doesn’t seem to me a FP, I will discuss this internally and come back if we reconsider.
In the meantime, I suggest you disabling the rule or accepting these issues.

Best,

6

Hi,

The use case is any scenario where you want to have something other than reference equality among a family of types.

Also, it is objectively a false positive. The code

public abstract record BaseRecord;

is syntatic sugar for the following, with an added rule that non record classes may not inherit from it: (Generated with SharpLab)

[Nullable(0)]
private abstract class BaseRecord : IEquatable<BaseRecord>
{
	[CompilerGenerated]
	protected virtual Type EqualityContract
	{
		[CompilerGenerated]
		get
		{
			return typeof(BaseRecord);
		}
	}

	[CompilerGenerated]
	public override string ToString()
	{
		StringBuilder stringBuilder = new StringBuilder();
		stringBuilder.Append("BaseRecord");
		stringBuilder.Append(" { ");
		if (PrintMembers(stringBuilder))
		{
			stringBuilder.Append(' ');
		}
		stringBuilder.Append('}');
		return stringBuilder.ToString();
	}

	[CompilerGenerated]
	protected virtual bool PrintMembers(StringBuilder builder)
	{
		return false;
	}

	[NullableContext(2)]
	[CompilerGenerated]
	public static bool operator !=(BaseRecord left, BaseRecord right)
	{
		return !(left == right);
	}

	[NullableContext(2)]
	[CompilerGenerated]
	public static bool operator ==(BaseRecord left, BaseRecord right)
	{
		return (object)left == right || ((object)left != null && left.Equals(right));
	}

	[CompilerGenerated]
	public override int GetHashCode()
	{
		return EqualityComparer<Type>.Default.GetHashCode(EqualityContract);
	}

	[NullableContext(2)]
	[CompilerGenerated]
	public override bool Equals(object obj)
	{
		return Equals(obj as BaseRecord);
	}

	[NullableContext(2)]
	[CompilerGenerated]
	public virtual bool Equals(BaseRecord other)
	{
		return (object)this == other || ((object)other != null && EqualityContract == other.EqualityContract);
	}

	[CompilerGenerated]
	public abstract BaseRecord <Clone>$();

	[CompilerGenerated]
	protected BaseRecord(BaseRecord original)
	{
	}

	protected BaseRecord()
	{
	}
}

which does not trigger the SonarLint warning.

7

Incidentally, the fact that you can be sure only reference record types will ever inherit from the base abstract record is another point in its favour over an empty interface.

9

Hello @RJMm

This is an interesting case, and the point of value equality semantic seems valid. However, this case is not enough to change the rule.

Thinking about it, also an empty class is not really empty either, as it comes with whatever object contains, that can be used in a similar way.

We think the best for now is to suppress this issue with a comment about your reasoning.

Let me know your point of view.

Thanks!

1 Like
10

Hi Mary & Cristian,

As you can probably guess I disagree, but I’ve learned to live with the warning suppression :slight_smile:

Thanks for looking into it anyway.

Best regards,
Robert

1 Like