Skip to content

Commit

Permalink
Add analyzer "Invalid argument null check" (RCS1256) (dotnet#888)
Browse files Browse the repository at this point in the history
  • Loading branch information
josefpihrt authored and JochemHarmes committed Oct 30, 2023
1 parent 0a4ddba commit 94f4f1b
Show file tree
Hide file tree
Showing 8 changed files with 446 additions and 17 deletions.
4 changes: 4 additions & 0 deletions ChangeLog.md
Expand Up @@ -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

Expand Down
@@ -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);
}
}
13 changes: 12 additions & 1 deletion src/Analyzers/Analyzers.xml
Expand Up @@ -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>
Expand All @@ -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>
138 changes: 138 additions & 0 deletions 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;
}
}
}
1 change: 1 addition & 0 deletions src/Analyzers/CSharp/DiagnosticIdentifiers.Generated.cs
Expand Up @@ -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";
}
}
12 changes: 12 additions & 0 deletions src/Analyzers/CSharp/DiagnosticRules.Generated.cs
Expand Up @@ -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>());

}
}
43 changes: 27 additions & 16 deletions src/Common/ArgumentNullCheckAnalysis.cs
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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
{
Expand All @@ -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)
Expand All @@ -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(
Expand Down

0 comments on commit 94f4f1b

Please sign in to comment.