Add analyzer "Invalid argument null check" (RCS1256)

ChangeLog.md

@@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add analyzer "Simplify argument null check" ([RCS1255](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1255.md)) ([#994](https://github.com/JosefPihrt/Roslynator/pull/994)).
   - Use `ArgumentNullException.ThrowIfNull` instead of `if` null check.
   - Not enabled by default
+- Add analyzer "Invalid argument null check" ([RCS1256](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1256.md)) ([#888](https://github.com/JosefPihrt/Roslynator/pull/888)).
+  - This analyzer reports null checks of arguments that are:
+	- annotated as nullable reference type.
+	- optional and its default value is `null`.
 
 ### Changed
 

src/Analyzers.CodeFixes/CSharp/CodeFixes/InvalidArgumentNullCheckCodeFixProvider.cs

@@ -0,0 +1,40 @@
+// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Immutable;
+using System.Composition;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Roslynator.CodeFixes;
+
+namespace Roslynator.CSharp.CodeFixes;
+
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(InvalidArgumentNullCheckCodeFixProvider))]
+[Shared]
+public sealed class InvalidArgumentNullCheckCodeFixProvider : BaseCodeFixProvider
+{
+    public override ImmutableArray<string> FixableDiagnosticIds
+    {
+        get { return ImmutableArray.Create(DiagnosticIdentifiers.InvalidArgumentNullCheck); }
+    }
+
+    public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        SyntaxNode root = await context.GetSyntaxRootAsync().ConfigureAwait(false);
+
+        if (!TryFindFirstAncestorOrSelf(root, context.Span, out StatementSyntax statement))
+            return;
+
+        Document document = context.Document;
+        Diagnostic diagnostic = context.Diagnostics[0];
+
+        CodeAction codeAction = CodeAction.Create(
+            "Remove null check",
+            ct => context.Document.RemoveStatementAsync(statement, ct),
+            GetEquivalenceKey(diagnostic));
+
+        context.RegisterCodeFix(codeAction, diagnostic);
+    }
+}

src/Analyzers/Analyzers.xml

@@ -5744,7 +5744,7 @@ void M()
   <Analyzer Identifier="SimplifyArgumentNullCheck">
     <Id>RCS1255</Id>
     <Title>Simplify argument null check.</Title>
-    <Category>General</Category>
+    <Category>Roslynator</Category>
     <DefaultSeverity>Info</DefaultSeverity>
     <IsEnabledByDefault>false</IsEnabledByDefault>
     <Summary>Use `ArgumentNullException.ThrowIfNull` instead of `if` null check.</Summary>
@@ -5758,4 +5758,15 @@ void M()
       </Sample>
     </Samples>
   </Analyzer>
+  <Analyzer Identifier="InvalidArgumentNullCheck">
+    <Id>RCS1256</Id>
+    <Title>Invalid argument null check.</Title>
+    <Category>Roslynator</Category>
+    <DefaultSeverity>Info</DefaultSeverity>
+    <IsEnabledByDefault>true</IsEnabledByDefault>
+    <Summary>This analyzer reports null checks of arguments that are:
+- annotated as nullable reference type.
+- optional and its default value is `null`.
+    </Summary>
+  </Analyzer>
 </Analyzers>

src/Analyzers/CSharp/Analysis/InvalidArgumentNullCheckAnalyzer.cs

@@ -0,0 +1,138 @@
+// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Roslynator.CSharp.Analysis;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class InvalidArgumentNullCheckAnalyzer : BaseDiagnosticAnalyzer
+{
+    private static ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics;
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
+    {
+        get
+        {
+            if (_supportedDiagnostics.IsDefault)
+                Immutable.InterlockedInitialize(ref _supportedDiagnostics, DiagnosticRules.InvalidArgumentNullCheck);
+
+            return _supportedDiagnostics;
+        }
+    }
+
+    public override void Initialize(AnalysisContext context)
+    {
+        base.Initialize(context);
+
+        context.RegisterSyntaxNodeAction(f => AnalyzeMethodDeclaration(f), SyntaxKind.MethodDeclaration);
+        context.RegisterSyntaxNodeAction(f => AnalyzeConstructorDeclaration(f), SyntaxKind.ConstructorDeclaration);
+    }
+
+    private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context)
+    {
+        var methodDeclaration = (MethodDeclarationSyntax)context.Node;
+        AnalyzeParameterList(context, methodDeclaration.ParameterList, methodDeclaration.Body);
+    }
+
+    private static void AnalyzeConstructorDeclaration(SyntaxNodeAnalysisContext context)
+    {
+        var constructorDeclaration = (ConstructorDeclarationSyntax)context.Node;
+        AnalyzeParameterList(context, constructorDeclaration.ParameterList, constructorDeclaration.Body);
+    }
+
+    private static void AnalyzeParameterList(SyntaxNodeAnalysisContext context, ParameterListSyntax parameterList, BlockSyntax body)
+    {
+        if (body is null)
+            return;
+
+        if (parameterList is null)
+            return;
+
+        SeparatedSyntaxList<ParameterSyntax> parameters = parameterList.Parameters;
+
+        if (!parameters.Any())
+            return;
+
+        SyntaxList<StatementSyntax> statements = body.Statements;
+
+        int statementCount = statements.Count;
+
+        if (statementCount == 0)
+            return;
+
+        int lastIndex = -1;
+
+        foreach (ParameterSyntax parameter in parameters)
+        {
+            if (parameter.IsParams())
+                break;
+
+            lastIndex++;
+        }
+
+        if (lastIndex == -1)
+            return;
+
+        foreach (StatementSyntax statement in statements)
+        {
+            ArgumentNullCheckAnalysis nullCheck = ArgumentNullCheckAnalysis.Create(statement, context.SemanticModel, context.CancellationToken);
+
+            if (nullCheck.Success)
+            {
+                ParameterSyntax parameter = FindParameter(nullCheck.Name);
+
+                if (parameter is not null)
+                {
+                    if (parameter.Default?.Value.IsKind(
+                        SyntaxKind.NullLiteralExpression,
+                        SyntaxKind.DefaultLiteralExpression,
+                        SyntaxKind.DefaultExpression) == true
+                        || IsNullableReferenceType(context, parameter))
+                    {
+                        if (statement is IfStatementSyntax ifStatement)
+                        {
+                            context.ReportDiagnostic(DiagnosticRules.InvalidArgumentNullCheck, ifStatement.IfKeyword);
+                        }
+                        else
+                        {
+                            context.ReportDiagnostic(DiagnosticRules.InvalidArgumentNullCheck, statement);
+                        }
+                    }
+                }
+            }
+        }
+
+        bool IsNullableReferenceType(SyntaxNodeAnalysisContext context, ParameterSyntax parameter)
+        {
+            TypeSyntax type = parameter.Type;
+
+            if (type.IsKind(SyntaxKind.NullableType))
+            {
+                ITypeSymbol typeSymbol = context.SemanticModel.GetTypeSymbol(type, context.CancellationToken);
+
+                if (typeSymbol?.IsKind(SymbolKind.ErrorType) == false
+                    && typeSymbol.IsReferenceType)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        ParameterSyntax FindParameter(string name)
+        {
+            for (int i = 0; i <= lastIndex; i++)
+            {
+                if (parameters[i].Identifier.ValueText == name)
+                    return parameters[i];
+            }
+
+            return null;
+        }
+    }
+}

src/Analyzers/CSharp/DiagnosticIdentifiers.Generated.cs

@@ -212,5 +212,6 @@ public static partial class DiagnosticIdentifiers
         public const string FormatDocumentationCommentSummary = "RCS1253";
         public const string NormalizeFormatOfEnumFlagValue = "RCS1254";
         public const string SimplifyArgumentNullCheck = "RCS1255";
+        public const string InvalidArgumentNullCheck = "RCS1256";
     }
 }

src/Analyzers/CSharp/DiagnosticRules.Generated.cs

@@ -2509,5 +2509,17 @@ public static partial class DiagnosticRules
             helpLinkUri:        DiagnosticIdentifiers.SimplifyArgumentNullCheck, 
             customTags:         Array.Empty<string>());
 
+        /// <summary>RCS1256</summary>
+        public static readonly DiagnosticDescriptor InvalidArgumentNullCheck = DiagnosticDescriptorFactory.Create(
+            id:                 DiagnosticIdentifiers.InvalidArgumentNullCheck, 
+            title:              "Invalid argument null check.", 
+            messageFormat:      "Invalid argument null check.", 
+            category:           DiagnosticCategories.Roslynator, 
+            defaultSeverity:    DiagnosticSeverity.Info, 
+            isEnabledByDefault: true, 
+            description:        null, 
+            helpLinkUri:        DiagnosticIdentifiers.InvalidArgumentNullCheck, 
+            customTags:         Array.Empty<string>());
+
     }
 }

src/Common/ArgumentNullCheckAnalysis.cs

@@ -10,14 +10,15 @@ namespace Roslynator.CSharp;
 
 internal readonly struct ArgumentNullCheckAnalysis
 {
-    private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, bool success)
+    private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, bool success)
     {
         Style = style;
+        Name = name;
         Success = success;
     }
 
     public ArgumentNullCheckStyle Style { get; }
-
+    public string Name { get; }
     public bool Success { get; }
 
     public static ArgumentNullCheckAnalysis Create(
@@ -37,6 +38,7 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, bool success)
         if (statement is IfStatementSyntax ifStatement)
         {
             var style = ArgumentNullCheckStyle.None;
+            string identifier = null;
             var success = false;
 
             if (ifStatement.SingleNonBlockStatementOrDefault() is ThrowStatementSyntax throwStatement
@@ -52,22 +54,26 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, bool success)
                 {
                     style = ArgumentNullCheckStyle.IfStatement;
 
-                    if (name is null
-                        || (nullCheck.Expression is IdentifierNameSyntax identifierName
-                            && string.Equals(name, identifierName.Identifier.ValueText, StringComparison.Ordinal)))
+                    if (nullCheck.Expression is IdentifierNameSyntax identifierName)
                     {
-                        if (semanticModel
-                            .GetSymbol(objectCreation, cancellationToken)?
-                            .ContainingType?
-                            .HasMetadataName(MetadataNames.System_ArgumentNullException) == true)
+                        identifier = identifierName.Identifier.ValueText;
+
+                        if (name is null
+                            || string.Equals(name, identifierName.Identifier.ValueText, StringComparison.Ordinal))
                         {
-                            success = true;
+                            if (semanticModel
+                                .GetSymbol(objectCreation, cancellationToken)?
+                                .ContainingType?
+                                .HasMetadataName(MetadataNames.System_ArgumentNullException) == true)
+                            {
+                                success = true;
+                            }
                         }
                     }
                 }
             }
 
-            return new ArgumentNullCheckAnalysis(style, success);
+            return new ArgumentNullCheckAnalysis(style, identifier, success);
         }
         else
         {
@@ -82,6 +88,7 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, bool success)
         CancellationToken cancellationToken)
     {
         var style = ArgumentNullCheckStyle.None;
+        string identifier = null;
         var success = false;
 
         if (statement is ExpressionStatementSyntax expressionStatement)
@@ -97,16 +104,20 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, bool success)
             {
                 style = ArgumentNullCheckStyle.ThrowIfNullMethod;
 
-                if (name is null
-                    || (invocationInfo.Arguments.SingleOrDefault(shouldThrow: false)?.Expression is IdentifierNameSyntax identifierName
-                        && string.Equals(name, identifierName.Identifier.ValueText, StringComparison.Ordinal)))
+                if (invocationInfo.Arguments.SingleOrDefault(shouldThrow: false)?.Expression is IdentifierNameSyntax identifierName)
                 {
-                    success = true;
+                    identifier = identifierName.Identifier.ValueText;
+
+                    if (name is null
+                        || string.Equals(name, identifierName.Identifier.ValueText, StringComparison.Ordinal))
+                    {
+                        success = true;
+                    }
                 }
             }
         }
 
-        return new ArgumentNullCheckAnalysis(style, success);
+        return new ArgumentNullCheckAnalysis(style, identifier, success);
     }
 
     public static bool IsArgumentNullExceptionThrowIfNullCheck(