MA0169 - Detect equality operators that should be replaced with Equals method

README.md

@@ -184,6 +184,7 @@ If you are already using other analyzers, you can check [which rules are duplica
 |[MA0166](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0166.md)|Usage|Forward the TimeProvider to methods that take one|ℹ️|✔️|✔️|
 |[MA0167](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0167.md)|Usage|Use an overload with a TimeProvider argument|ℹ️|❌|❌|
 |[MA0168](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0168.md)|Performance|Use readonly struct for in or ref readonly parameter|ℹ️|❌|❌|
+|[MA0169](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0169.md)|Design|Use Equals method instead of operator|⚠️|✔️|❌|
 
 <!-- rules -->
 

docs/README.md

@@ -168,6 +168,7 @@
 |[MA0166](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0166.md)|Usage|Forward the TimeProvider to methods that take one|<span title='Info'>ℹ️</span>|✔️|✔️|
 |[MA0167](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0167.md)|Usage|Use an overload with a TimeProvider argument|<span title='Info'>ℹ️</span>|❌|❌|
 |[MA0168](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0168.md)|Performance|Use readonly struct for in or ref readonly parameter|<span title='Info'>ℹ️</span>|❌|❌|
+|[MA0169](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0169.md)|Design|Use Equals method instead of operator|<span title='Warning'>⚠️</span>|✔️|❌|
 
 |Id|Suppressed rule|Justification|
 |--|---------------|-------------|
@@ -680,6 +681,9 @@ dotnet_diagnostic.MA0167.severity = none
 
 # MA0168: Use readonly struct for in or ref readonly parameter
 dotnet_diagnostic.MA0168.severity = none
+
+# MA0169: Use Equals method instead of operator
+dotnet_diagnostic.MA0169.severity = warning
 ```
 
 # .editorconfig - all rules disabled
@@ -1185,4 +1189,7 @@ dotnet_diagnostic.MA0167.severity = none
 
 # MA0168: Use readonly struct for in or ref readonly parameter
 dotnet_diagnostic.MA0168.severity = none
+
+# MA0169: Use Equals method instead of operator
+dotnet_diagnostic.MA0169.severity = none
 ```

docs/Rules/MA0169.md

@@ -0,0 +1,44 @@
+# MA0169 - Use Equals method instead of operator
+
+When a type overrides the `Equals` method, but does not define the equality operators, using `==` or `!=` will do a reference comparison. This can lead to unexpected behavior, as the `Equals` method may be overridden to provide a value comparison. This rule is to ensure that the `Equals` method is used.
+
+````c#
+Sample a = default;
+Sample b = default;
+
+_ = a == b; // ok, Equals is not overridden
+
+class Sample { }
+````
+
+````c#
+Sample a = default;
+Sample b = default;
+
+_ = a == b; // ok, the equality operator are defined
+
+
+class Sample
+{
+    public static bool operator ==(Sample a, Sample b) => true;
+    public static bool operator !=(Sample a, Sample b) => false;
+    public override bool Equals(object obj) => true;
+    public override int GetHashCode() => 0;
+}
+````
+
+````c#
+Sample a = default;
+Sample b = default;
+
+_ = a.Equals(b); // ok
+_ = object.Reference`Equals(a, b); // ok
+_ = a == b; // non-compliant
+
+class Sample
+{
+    public override bool Equals(object obj) => true;
+    public override int GetHashCode() => 0;
+}
+````
+

src/Meziantou.Analyzer.Pack/configuration/default.editorconfig

@@ -502,3 +502,6 @@ dotnet_diagnostic.MA0167.severity = none
 
 # MA0168: Use readonly struct for in or ref readonly parameter
 dotnet_diagnostic.MA0168.severity = none
+
+# MA0169: Use Equals method instead of operator
+dotnet_diagnostic.MA0169.severity = warning

src/Meziantou.Analyzer.Pack/configuration/none.editorconfig

@@ -502,3 +502,6 @@ dotnet_diagnostic.MA0167.severity = none
 
 # MA0168: Use readonly struct for in or ref readonly parameter
 dotnet_diagnostic.MA0168.severity = none
+
+# MA0169: Use Equals method instead of operator
+dotnet_diagnostic.MA0169.severity = none

src/Meziantou.Analyzer/Internals/MethodSymbolExtensions.cs

@@ -1,4 +1,4 @@
-using System.Linq;
+using System.Linq;
 using Microsoft.CodeAnalysis;
 
 namespace Meziantou.Analyzer.Internals;
@@ -56,6 +56,41 @@ private static bool IsInterfaceImplementation(this ISymbol symbol)
             .FirstOrDefault(interfaceMember => SymbolEqualityComparer.Default.Equals(symbol, symbol.ContainingType.FindImplementationForInterfaceMember(interfaceMember)));
     }
 
+    public static bool IsOrOverrideMethod(this IMethodSymbol? symbol, IMethodSymbol? baseMethod)
+    {
+        if (symbol is null || baseMethod is null)
+            return false;
+
+        if (symbol.IsEqualTo(baseMethod))
+            return true;
+
+        while (symbol is not null)
+        {
+            if (symbol.IsEqualTo(baseMethod))
+                return true;
+
+            symbol = symbol.OverriddenMethod!;
+        }
+
+        return false;
+    }
+
+    public static bool OverrideMethod(this IMethodSymbol? symbol, IMethodSymbol? baseMethod)
+    {
+        if (symbol is null || baseMethod is null)
+            return false;
+
+        while (symbol.OverriddenMethod is not null)
+        {
+            if (symbol.OverriddenMethod.IsEqualTo(baseMethod))
+                return true;
+
+            symbol = symbol.OverriddenMethod!;
+        }
+
+        return false;
+    }
+
     public static bool IsUnitTestMethod(this IMethodSymbol methodSymbol)
     {
         var attributes = methodSymbol.GetAttributes();

src/Meziantou.Analyzer/RuleIdentifiers.cs

@@ -171,6 +171,7 @@ internal static class RuleIdentifiers
     public const string UseAnOverloadThatHasTimeProviderWhenAvailable = "MA0166";
     public const string UseAnOverloadThatHasTimeProvider = "MA0167";
     public const string UseReadOnlyStructForRefReadOnlyParameters = "MA0168";
+    public const string UseEqualsMethodInsteadOfOperator = "MA0169";
 
     public static string GetHelpUri(string identifier)
     {

src/Meziantou.Analyzer/Rules/UseEqualsMethodInsteadOfOperatorAnalyzer.cs

@@ -0,0 +1,77 @@
+using System.Collections.Immutable;
+using System.Linq;
+using Meziantou.Analyzer.Internals;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Meziantou.Analyzer.Rules;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class UseEqualsMethodInsteadOfOperatorAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly DiagnosticDescriptor Rule = new(
+        RuleIdentifiers.UseEqualsMethodInsteadOfOperator,
+        title: "Use Equals method instead of operator",
+        messageFormat: "Use Equals method instead of == or != operator",
+        RuleCategories.Design,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "",
+        helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseEqualsMethodInsteadOfOperator));
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+        context.RegisterCompilationStartAction(context =>
+        {
+            if (context.Compilation.GetSpecialType(SpecialType.System_Object).GetMembers("Equals").FirstOrDefault() is not IMethodSymbol objectEqualsSymbol)
+                return;
+
+            context.RegisterOperationAction(context => AnalyzerBinaryOperation(context, objectEqualsSymbol), OperationKind.Binary);
+        });
+    }
+
+    private static void AnalyzerBinaryOperation(OperationAnalysisContext context, IMethodSymbol objectEqualsSymbol)
+    {
+        var operation = (IBinaryOperation)context.Operation;
+        if (operation is { OperatorKind: BinaryOperatorKind.Equals or BinaryOperatorKind.NotEquals, OperatorMethod: null })
+        {
+            if (IsNull(operation.LeftOperand) || IsNull(operation.RightOperand))
+                return;
+
+            var leftType = operation.LeftOperand.UnwrapImplicitConversionOperations().Type;
+            if (operation.IsLifted)
+            {
+                leftType = leftType.GetUnderlyingNullableTypeOrSelf();
+            }
+
+            if (leftType is null)
+                return;
+
+            if (leftType.IsValueType)
+                return;
+
+            switch (leftType.SpecialType)
+            {
+                case SpecialType.System_Enum:
+                case SpecialType.System_Object:
+                case SpecialType.System_String:
+                    return;
+            }
+
+            var overrideEqualsSymbol = leftType.GetMembers("Equals").OfType<IMethodSymbol>().FirstOrDefault(m => m.IsOrOverrideMethod(objectEqualsSymbol));
+            if (overrideEqualsSymbol is not null)
+            {
+                context.ReportDiagnostic(Rule, operation);
+            }
+        }
+    }
+
+    public static bool IsNull(IOperation operation)
+        => operation.UnwrapConversionOperations() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
+}

tests/Meziantou.Analyzer.Test/Rules/UseEqualsMethodInsteadOfOperatorAnalyzerTests.cs

@@ -0,0 +1,108 @@
+using System.Threading.Tasks;
+using Meziantou.Analyzer.Rules;
+using TestHelper;
+using Xunit;
+
+namespace Meziantou.Analyzer.Test.Rules;
+public class UseEqualsMethodInsteadOfOperatorAnalyzerTests
+{
+    private static ProjectBuilder CreateProjectBuilder()
+    {
+        return new ProjectBuilder()
+            .WithTargetFramework(Helpers.TargetFramework.Net9_0)
+            .WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication)
+            .WithAnalyzer<UseEqualsMethodInsteadOfOperatorAnalyzer>();
+    }
+
+    [Theory]
+    [InlineData("System.Net.IPAddress")]
+    public async Task Report_EqualsOperator(string type)
+    {
+        await CreateProjectBuilder()
+            .WithSourceCode($$"""
+                {{type}} a = null;
+                {{type}} b = null;
+                _ = [|a == b|];
+                """)
+              .ValidateAsync();
+    }
+
+    [Theory]
+    [InlineData("char")]
+    [InlineData("string")]
+    [InlineData("sbyte")]
+    [InlineData("byte")]
+    [InlineData("short")]
+    [InlineData("ushort")]
+    [InlineData("int")]
+    [InlineData("uint")]
+    [InlineData("long")]
+    [InlineData("ulong")]
+    [InlineData("System.Int128")]
+    [InlineData("System.UInt128")]
+    [InlineData("System.Half")]
+    [InlineData("float")]
+    [InlineData("double")]
+    [InlineData("decimal")]
+    [InlineData("System.DayOfWeek")]
+    [InlineData("System.DayOfWeek?")]
+    public async Task NoReport_EqualsOperator(string type)
+    {
+        await CreateProjectBuilder()
+            .WithSourceCode($$"""
+                {{type}} a = default;
+                {{type}} b = default;
+                _ = a == b;
+                """)
+              .ValidateAsync();
+    }
+
+    [Fact]
+    public async Task ClassWithParentEqualsMethod()
+    {
+        await CreateProjectBuilder()
+            .WithSourceCode($$"""
+                B a = default;
+                B b = default;
+                _ = a == b;
+
+                class A
+                {
+                    public override bool Equals(object obj) => throw null;
+                }
+
+                class B : A
+                {
+                }
+                """)
+              .ValidateAsync();
+    }
+
+    [Fact]
+    public async Task ClassWithoutEqualsMethod()
+    {
+        await CreateProjectBuilder()
+            .WithSourceCode($$"""
+                Sample a = default;
+                Sample b = default;
+                _ = a == b;
+
+                class Sample {}
+                """)
+              .ValidateAsync();
+    }
+
+    [Fact]
+    public async Task RecordWithoutEqualsMethod()
+    {
+        await CreateProjectBuilder()
+            .WithSourceCode($$"""
+                Sample a = default;
+                Sample b = default;
+                _ = a == b; // Operator is implemented by the record
+
+                record Sample {}
+                """)
+              .ValidateAsync();
+    }
+}