Add MA0186: Report missing nullable attributes on Equals and TryGetValue parameters

README.md

@@ -200,6 +200,7 @@ If you are already using other analyzers, you can check [which rules are duplica
 |[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|string.Format should use a format string with placeholders|⚠️|✔️|❌|
 |[MA0184](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0184.md)|Style|Do not use interpolated string without parameters|👻|✔️|✔️|
 |[MA0185](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0185.md)|Performance|Simplify string.Create when all parameters are culture invariant|ℹ️|✔️|✔️|
+|[MA0186](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0186.md)|Design|Equals method should use \[NotNullWhen(true)\] on the parameter|ℹ️|❌|❌|
 
 <!-- rules -->
 

docs/README.md

@@ -184,6 +184,7 @@
 |[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|string.Format should use a format string with placeholders|<span title='Warning'>⚠️</span>|✔️|❌|
 |[MA0184](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0184.md)|Style|Do not use interpolated string without parameters|<span title='Hidden'>👻</span>|✔️|✔️|
 |[MA0185](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0185.md)|Performance|Simplify string.Create when all parameters are culture invariant|<span title='Info'>ℹ️</span>|✔️|✔️|
+|[MA0186](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0186.md)|Design|Equals method should use \[NotNullWhen(true)\] on the parameter|<span title='Info'>ℹ️</span>|❌|❌|
 
 |Id|Suppressed rule|Justification|
 |--|---------------|-------------|
@@ -751,6 +752,9 @@ dotnet_diagnostic.MA0184.severity = silent
 
 # MA0185: Simplify string.Create when all parameters are culture invariant
 dotnet_diagnostic.MA0185.severity = suggestion
+
+# MA0186: Equals method should use [NotNullWhen(true)] on the parameter
+dotnet_diagnostic.MA0186.severity = none
 ```
 
 # .editorconfig - all rules disabled
@@ -1304,4 +1308,7 @@ dotnet_diagnostic.MA0184.severity = none
 
 # MA0185: Simplify string.Create when all parameters are culture invariant
 dotnet_diagnostic.MA0185.severity = none
+
+# MA0186: Equals method should use [NotNullWhen(true)] on the parameter
+dotnet_diagnostic.MA0186.severity = none
 ```

docs/Rules/MA0186.md

@@ -0,0 +1,118 @@
+# MA0186 - Equals method should use \[NotNullWhen(true)\] on the parameter
+<!-- sources -->
+Source: [MissingNotNullWhenAttributeOnEqualsAnalyzer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer/Rules/MissingNotNullWhenAttributeOnEqualsAnalyzer.cs)
+<!-- sources -->
+
+## Description
+
+This rule reports missing nullable attributes on method parameters to improve nullable reference type analysis:
+
+1. When implementing `Equals(object?)` or `IEquatable<T>.Equals(T?)`, the parameter should be annotated with `[NotNullWhen(true)]` to inform the compiler that when the method returns `true`, the parameter is guaranteed to be non-null.
+
+2. When implementing `IDictionary<TKey, TValue>.TryGetValue`, the `value` parameter should be annotated with `[MaybeNullWhen(false)]` to inform the compiler that when the method returns `false`, the parameter may be null.
+
+## Motivation
+
+### Equals methods
+
+Without the `[NotNullWhen(true)]` attribute, the compiler cannot track that a successful equality comparison means the parameter is non-null. This can lead to unnecessary null checks or null-forgiving operators in code that follows an equality check.
+
+````c#
+// ❌ Without the attribute
+public override bool Equals(object? obj)
+{
+    return obj is MyType other && /* comparison */;
+}
+
+// Usage
+if (myInstance.Equals(someObj))
+{
+    // Compiler doesn't know someObj is not null here
+    someObj.ToString(); // Warning CS8602
+}
+
+// ✅ With the attribute
+public override bool Equals([NotNullWhen(true)] object? obj)
+{
+    return obj is MyType other && /* comparison */;
+}
+
+// Usage
+if (myInstance.Equals(someObj))
+{
+    // Compiler knows someObj is not null here
+    someObj.ToString(); // No warning
+}
+````
+
+### TryGetValue methods
+
+Without the `[MaybeNullWhen(false)]` attribute, the compiler cannot track that when `TryGetValue` returns `false`, the out parameter may be null.
+
+````c#
+// ❌ Without the attribute
+public bool TryGetValue(TKey key, out TValue? value)
+{
+    // Implementation
+}
+
+// ✅ With the attribute
+public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue? value)
+{
+    // Implementation
+}
+
+// Usage
+if (dictionary.TryGetValue(key, out var value))
+{
+    value.ToString(); // Compiler knows value is not null
+}
+````
+
+## How to fix violations
+
+### For Equals methods
+
+Add the `[NotNullWhen(true)]` attribute to the parameter:
+
+````c#
+using System.Diagnostics.CodeAnalysis;
+
+public class MyType : IEquatable<MyType>
+{
+    public override bool Equals([NotNullWhen(true)] object? obj)
+    {
+        return Equals(obj as MyType);
+    }
+
+    public bool Equals([NotNullWhen(true)] MyType? other)
+    {
+        return other is not null && /* comparison logic */;
+    }
+}
+````
+
+### For TryGetValue methods
+
+Add the `[MaybeNullWhen(false)]` attribute to the value parameter:
+
+````c#
+using System.Diagnostics.CodeAnalysis;
+using System.Collections.Generic;
+
+public class MyDictionary : IDictionary<string, string?>
+{
+    public bool TryGetValue(string key, [MaybeNullWhen(false)] out string? value)
+    {
+        // Implementation
+    }
+}
+````
+
+## Configuration
+
+This rule is disabled by default as it's a suggestion for improved nullable annotations. To enable it, add the following to your `.editorconfig` file:
+
+````editorconfig
+dotnet_diagnostic.MA0186.severity = suggestion
+````

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

@@ -550,3 +550,6 @@ dotnet_diagnostic.MA0184.severity = silent
 
 # MA0185: Simplify string.Create when all parameters are culture invariant
 dotnet_diagnostic.MA0185.severity = suggestion
+
+# MA0186: Equals method should use [NotNullWhen(true)] on the parameter
+dotnet_diagnostic.MA0186.severity = none

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

@@ -550,3 +550,6 @@ dotnet_diagnostic.MA0184.severity = none
 
 # MA0185: Simplify string.Create when all parameters are culture invariant
 dotnet_diagnostic.MA0185.severity = none
+
+# MA0186: Equals method should use [NotNullWhen(true)] on the parameter
+dotnet_diagnostic.MA0186.severity = none

src/Meziantou.Analyzer/Internals/TypeSymbolExtensions.cs

@@ -48,6 +48,14 @@ public static bool Implements(this ITypeSymbol classSymbol, ITypeSymbol? interfa
         return classSymbol.AllInterfaces.Any(interfaceType.IsEqualTo);
     }
 
+    public static bool ImplementsGenericInterface(this ITypeSymbol classSymbol, ITypeSymbol? interfaceType)
+    {
+        if (interfaceType is null)
+            return false;
+
+        return classSymbol.AllInterfaces.Any(iface => iface.OriginalDefinition.IsEqualTo(interfaceType.OriginalDefinition));
+    }
+
     public static bool IsOrImplements(this ITypeSymbol symbol, ITypeSymbol? interfaceType)
     {
         if (interfaceType is null)

src/Meziantou.Analyzer/RuleIdentifiers.cs

@@ -185,6 +185,7 @@ internal static class RuleIdentifiers
     public const string StringFormatShouldBeConstant = "MA0183";
     public const string DoNotUseInterpolatedStringWithoutParameters = "MA0184";
     public const string SimplifyStringCreateWhenAllParametersAreCultureInvariant = "MA0185";
+    public const string MissingNotNullWhenAttributeOnEquals = "MA0186";
 
     public static string GetHelpUri(string identifier)
     {

src/Meziantou.Analyzer/Rules/MissingNotNullWhenAttributeOnEqualsAnalyzer.cs

@@ -0,0 +1,194 @@
+using System.Collections.Immutable;
+using System.Runtime.CompilerServices;
+using Meziantou.Analyzer.Internals;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Meziantou.Analyzer.Rules;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class MissingNotNullWhenAttributeOnEqualsAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly DiagnosticDescriptor EqualsRule = new(
+        RuleIdentifiers.MissingNotNullWhenAttributeOnEquals,
+        title: "Equals method should use [NotNullWhen(true)] on the parameter",
+        messageFormat: "Equals method should use [NotNullWhen(true)] on parameter '{0}'",
+        RuleCategories.Design,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: false,
+        description: "",
+        helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.MissingNotNullWhenAttributeOnEquals));
+
+    private static readonly DiagnosticDescriptor TryGetValueRule = new(
+        RuleIdentifiers.MissingNotNullWhenAttributeOnEquals,
+        title: "TryGetValue method should use [MaybeNullWhen(false)] on the value parameter",
+        messageFormat: "TryGetValue method should use [MaybeNullWhen(false)] on parameter '{0}'",
+        RuleCategories.Design,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: false,
+        description: "",
+        helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.MissingNotNullWhenAttributeOnEquals));
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(EqualsRule, TryGetValueRule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+        context.RegisterCompilationStartAction(ctx =>
+        {
+            var notNullWhenAttributeSymbol = ctx.Compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.NotNullWhenAttribute");
+            var maybeNullWhenAttributeSymbol = ctx.Compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute");
+            var iequatableOfTSymbol = ctx.Compilation.GetBestTypeByMetadataName("System.IEquatable`1");
+            var idictionaryOfTSymbol = ctx.Compilation.GetBestTypeByMetadataName("System.Collections.Generic.IDictionary`2");
+
+            if (idictionaryOfTSymbol != null && maybeNullWhenAttributeSymbol is not null)
+            {
+                var tryGetValueSymbols = idictionaryOfTSymbol.GetMembers("TryGetValue");
+                if (tryGetValueSymbols.Length == 1)
+                {
+                    var tryGetValueSymbol = tryGetValueSymbols[0];
+                    ctx.RegisterSymbolAction(context =>
+                    {
+                        var namedType = (INamedTypeSymbol)context.Symbol;
+                        foreach (var interfaceType in namedType.AllInterfaces)
+                        {
+                            if (!interfaceType.ConstructedFrom.IsEqualTo(idictionaryOfTSymbol))
+                                continue;
+
+                            var dictionaryTryGetValueSymbols = interfaceType.GetMembers("TryGetValue");
+                            if (dictionaryTryGetValueSymbols.Length != 1)
+                                continue;
+
+                            var implementation = namedType.FindImplementationForInterfaceMember(dictionaryTryGetValueSymbols[0]) as IMethodSymbol;
+                            if (implementation is null)
+                                continue;
+
+                            if (implementation.Parameters.Length != 2)
+                                continue;
+
+                            var valueParameter = implementation.Parameters[1];
+
+                            // Check if the parameter is an out parameter
+                            if (valueParameter.RefKind != RefKind.Out)
+                                continue;
+
+                            // Check if the parameter is nullable
+                            if (valueParameter.NullableAnnotation != NullableAnnotation.Annotated)
+                                continue;
+
+                            // Check if the parameter already has [MaybeNullWhen(false)] attribute
+                            if (HasMaybeNullWhenAttribute(valueParameter, maybeNullWhenAttributeSymbol, expectedValue: false))
+                                continue;
+
+                            // Report diagnostic
+                            context.ReportDiagnostic(TryGetValueRule, valueParameter, valueParameter.Name);
+                        }
+                    }, SymbolKind.NamedType);
+                }
+
+
+            }
+
+            if (notNullWhenAttributeSymbol is not null)
+            {
+                context.RegisterSymbolAction(context =>
+                {
+                    var method = (IMethodSymbol)context.Symbol;
+                    if (method.Name is nameof(object.Equals))
+                    {
+                        if (!method.ReturnType.IsBoolean())
+                            return;
+
+                        if (method.Parameters.Length != 1)
+                            return;
+
+                        if (method.IsStatic)
+                            return;
+
+                        var parameter = method.Parameters[0];
+
+                        // Check if the parameter is nullable, this also ensures nullable annotations are enabled for the parameter
+                        if (parameter.NullableAnnotation != NullableAnnotation.Annotated)
+                            return;
+
+
+                        // Check if it's Equals(object?) override using helper
+                        var isObjectEqualsOverride = false;
+                        if (method.IsOverride && parameter.Type.IsObject())
+                        {
+                            // Verify it's overriding object.Equals by checking the base member
+                            var currentMethod = method.OverriddenMethod;
+                            while (currentMethod is not null)
+                            {
+                                if (currentMethod.ContainingType.IsObject())
+                                {
+                                    isObjectEqualsOverride = true;
+                                    break;
+                                }
+                                currentMethod = currentMethod.OverriddenMethod;
+                            }
+                        }
+
+                        // Check if it's IEquatable<T>.Equals(T?) implementation using helper
+                        var isIEquatableEquals = false;
+                        if (iequatableOfTSymbol is not null && method.ContainingType is not null && !method.ContainingType.IsValueType)
+                        {
+                            if (method.IsInterfaceImplementation())
+                            {
+                                var interfaceMethod = method.GetImplementingInterfaceSymbol();
+                                if (interfaceMethod is not null &&
+                                    interfaceMethod.ContainingType is INamedTypeSymbol interfaceType &&
+                                    interfaceType.ConstructedFrom.IsEqualTo(iequatableOfTSymbol))
+                                {
+                                    isIEquatableEquals = true;
+                                }
+                            }
+                        }
+
+                        if (!isObjectEqualsOverride && !isIEquatableEquals)
+                            return;
+
+                        // Check if the parameter already has [NotNullWhen(true)] attribute
+                        if (HasNotNullWhenAttribute(parameter, notNullWhenAttributeSymbol, expectedValue: true))
+                            return;
+
+                        // Report diagnostic
+                        context.ReportDiagnostic(EqualsRule, parameter, parameter.Name);
+                    }
+                }, SymbolKind.Method);
+            }
+        });
+    }
+
+    private static bool HasNotNullWhenAttribute(IParameterSymbol parameter, INamedTypeSymbol notNullWhenAttributeSymbol, bool expectedValue)
+    {
+        foreach (var attribute in parameter.GetAttributes())
+        {
+            if (attribute.AttributeClass.IsEqualTo(notNullWhenAttributeSymbol))
+            {
+                if (attribute.ConstructorArguments.Length == 1 && attribute.ConstructorArguments[0].Value is bool value && value == expectedValue)
+                {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    private static bool HasMaybeNullWhenAttribute(IParameterSymbol parameter, INamedTypeSymbol maybeNullWhenAttributeSymbol, bool expectedValue)
+    {
+        foreach (var attribute in parameter.GetAttributes())
+        {
+            if (attribute.AttributeClass.IsEqualTo(maybeNullWhenAttributeSymbol))
+            {
+                if (attribute.ConstructorArguments.Length == 1 && attribute.ConstructorArguments[0].Value is bool value && value == expectedValue)
+                {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+}