Skip to content

Commit

Permalink
Analyzer: Simplify argument null check (RCS1255) (#994)
Browse files Browse the repository at this point in the history
  • Loading branch information
josefpihrt committed Nov 19, 2022
1 parent fba159b commit bae4d29
Show file tree
Hide file tree
Showing 18 changed files with 303 additions and 14 deletions.
1 change: 1 addition & 0 deletions .editorconfig
Expand Up @@ -138,6 +138,7 @@ dotnet_diagnostic.RCS1250.severity = suggestion
dotnet_diagnostic.RCS1252.severity = suggestion
dotnet_diagnostic.RCS1253.severity = suggestion
dotnet_diagnostic.RCS1254.severity = suggestion
dotnet_diagnostic.RCS1255.severity = suggestion

dotnet_diagnostic.IDE0007.severity = none
dotnet_diagnostic.IDE0007WithoutSuggestion.severity = none
Expand Down
4 changes: 4 additions & 0 deletions ChangeLog.md
Expand Up @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add Arm64 VS 2022 extension support ([#990](https://github.com/JosefPihrt/Roslynator/pull/990) by @snickler).
- Add analyzer "Add/remove blank line after file scoped namespace declaration" [RCS0060](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS0060.md) ([#993](https://github.com/josefpihrt/roslynator/pull/993).
- Required option: `roslynator_blank_line_after_file_scoped_namespace_declaration = true|false`
- Not enabled by default
- 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

### Changed

Expand Down
Expand Up @@ -16,6 +16,9 @@
using Roslynator.CSharp.Analysis.If;
using Roslynator.CSharp.Refactorings;
using Roslynator.CSharp.Refactorings.ReduceIfNesting;
using Roslynator.CSharp.Syntax;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
using static Roslynator.CSharp.CSharpFactory;

namespace Roslynator.CSharp.CodeFixes;

Expand All @@ -33,7 +36,8 @@ public override ImmutableArray<string> FixableDiagnosticIds
DiagnosticIdentifiers.ConvertIfToReturnStatement,
DiagnosticIdentifiers.ConvertIfToAssignment,
DiagnosticIdentifiers.ReduceIfNesting,
DiagnosticIdentifiers.UseExceptionFilter);
DiagnosticIdentifiers.UseExceptionFilter,
DiagnosticIdentifiers.SimplifyArgumentNullCheck);
}
}

Expand Down Expand Up @@ -110,6 +114,22 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
},
GetEquivalenceKey(diagnostic));

context.RegisterCodeFix(codeAction, diagnostic);
break;
}
case DiagnosticIdentifiers.SimplifyArgumentNullCheck:
{
CodeAction codeAction = CodeAction.Create(
"Call ArgumentNullException.ThrowIfNull",
ct =>
{
return CallArgumentNullExceptionThrowIfNullAsync(
context.Document,
ifStatement,
cancellationToken: ct);
},
GetEquivalenceKey(diagnostic));

context.RegisterCodeFix(codeAction, diagnostic);
break;
}
Expand Down Expand Up @@ -156,7 +176,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
CatchClauseSyntax newCatchClause = catchClause.Update(
catchKeyword: catchClause.CatchKeyword,
declaration: catchClause.Declaration,
filter: SyntaxFactory.CatchFilterClause(filterExpression.WalkDownParentheses()),
filter: CatchFilterClause(filterExpression.WalkDownParentheses()),
block: catchClause.Block.WithStatements(newStatements));

newCatchClause = newCatchClause.WithFormatterAnnotation();
Expand Down Expand Up @@ -189,7 +209,7 @@ SyntaxList<StatementSyntax> ReplaceStatement(StatementSyntax statement)
if (!right.IsKind(SyntaxKind.LogicalAndExpression))
right = right.Parenthesize();

BinaryExpressionSyntax newCondition = CSharpFactory.LogicalAndExpression(left, right);
BinaryExpressionSyntax newCondition = LogicalAndExpression(left, right);

IfStatementSyntax newNode = GetNewIfStatement(ifStatement, nestedIf)
.WithCondition(newCondition)
Expand All @@ -216,4 +236,21 @@ private static IfStatementSyntax GetNewIfStatement(IfStatementSyntax ifStatement
return ifStatement.ReplaceNode(ifStatement.Statement, ifStatement2.Statement);
}
}

private Task<Document> CallArgumentNullExceptionThrowIfNullAsync(
Document document,
IfStatementSyntax ifStatement,
CancellationToken cancellationToken)
{
NullCheckExpressionInfo nullCheck = SyntaxInfo.NullCheckExpressionInfo(ifStatement.Condition);

ExpressionStatementSyntax newStatement = ExpressionStatement(
SimpleMemberInvocationExpression(
ParseExpression("global::System.ArgumentNullException").WithSimplifierAnnotation(),
IdentifierName("ThrowIfNull"),
Argument(nullCheck.Expression.WithoutTrivia())))
.WithTriviaFrom(ifStatement);

return document.ReplaceNodeAsync(ifStatement, newStatement, cancellationToken);
}
}
17 changes: 17 additions & 0 deletions src/Analyzers/Analyzers.xml
Expand Up @@ -5741,4 +5741,21 @@ void M()
<Option Key="enum_flag_value_style" IsRequired="true" />
</ConfigOptions>
</Analyzer>
<Analyzer Identifier="SimplifyArgumentNullCheck">
<Id>RCS1255</Id>
<Title>Simplify argument null check.</Title>
<Category>General</Category>
<DefaultSeverity>Info</DefaultSeverity>
<IsEnabledByDefault>false</IsEnabledByDefault>
<Summary>Use `ArgumentNullException.ThrowIfNull` instead of `if` null check.</Summary>
<Samples>
<Sample>
<Before><![CDATA[if (arg is null) // [|Id|]
{
throw new ArgumentNullException(nameof(arg));
}]]></Before>
<After><![CDATA[ArgumentNullException.ThrowIfNull(arg);]]></After>
</Sample>
</Samples>
</Analyzer>
</Analyzers>
99 changes: 99 additions & 0 deletions src/Analyzers/CSharp/Analysis/SimplifyArgumentNullCheckAnalyzer.cs
@@ -0,0 +1,99 @@
// 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.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Roslynator.CSharp.Syntax;

namespace Roslynator.CSharp.Analysis;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class SimplifyArgumentNullCheckAnalyzer : BaseDiagnosticAnalyzer
{
private static ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics;

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
{
get
{
if (_supportedDiagnostics.IsDefault)
Immutable.InterlockedInitialize(ref _supportedDiagnostics, DiagnosticRules.SimplifyArgumentNullCheck);

return _supportedDiagnostics;
}
}

public override void Initialize(AnalysisContext context)
{
base.Initialize(context);

context.RegisterSyntaxNodeAction(f => AnalyzeIfStatement(f), SyntaxKind.IfStatement);
}

private static void AnalyzeIfStatement(SyntaxNodeAnalysisContext context)
{
var ifStatement = (IfStatementSyntax)context.Node;

if (!ifStatement.IsSimpleIf())
return;

NullCheckExpressionInfo nullCheck = SyntaxInfo.NullCheckExpressionInfo(
ifStatement.Condition,
NullCheckStyles.EqualsToNull | NullCheckStyles.IsNull);

if (!nullCheck.Success)
return;

if (nullCheck.Expression is not IdentifierNameSyntax identifierName)
return;

if (ifStatement.SingleNonBlockStatementOrDefault() is not ThrowStatementSyntax throwStatement)
return;

if (throwStatement.Expression is not ObjectCreationExpressionSyntax objectCreation)
return;

ExpressionSyntax expression = objectCreation.ArgumentList?.Arguments.SingleOrDefault(shouldThrow: false)?.Expression;

if (expression is null)
return;

if (ifStatement.ContainsUnbalancedIfElseDirectives())
return;

INamedTypeSymbol containingTypeSymbol = context.SemanticModel
.GetSymbol(objectCreation, context.CancellationToken)?
.ContainingType;

if (containingTypeSymbol?.HasMetadataName(MetadataNames.System_ArgumentNullException) != true)
return;

if (!containingTypeSymbol.GetMembers("ThrowIfNull").Any(f => f.IsKind(SymbolKind.Method)))
return;

if (expression.IsKind(SyntaxKind.StringLiteralExpression))
{
var literal = (LiteralExpressionSyntax)expression;

if (string.Equals(identifierName.Identifier.ValueText, literal.Token.ValueText))
ReportDiagnostic(context, ifStatement);
}
else if (expression is InvocationExpressionSyntax invocationExpression)
{
if (CSharpUtility.IsNameOfExpression(invocationExpression, context.SemanticModel, context.CancellationToken)
&& invocationExpression.ArgumentList.Arguments.FirstOrDefault().Expression is IdentifierNameSyntax identifierName2
&& string.Equals(identifierName.Identifier.ValueText, identifierName2.Identifier.ValueText))
{
ReportDiagnostic(context, ifStatement);
}
}
}

private static void ReportDiagnostic(SyntaxNodeAnalysisContext context, IfStatementSyntax ifStatement)
{
context.ReportDiagnostic(DiagnosticRules.SimplifyArgumentNullCheck, ifStatement.IfKeyword);
}
}
1 change: 1 addition & 0 deletions src/Analyzers/CSharp/DiagnosticIdentifiers.Generated.cs
Expand Up @@ -211,5 +211,6 @@ public static partial class DiagnosticIdentifiers
public const string NormalizeUsageOfInfiniteLoop = "RCS1252";
public const string FormatDocumentationCommentSummary = "RCS1253";
public const string NormalizeFormatOfEnumFlagValue = "RCS1254";
public const string SimplifyArgumentNullCheck = "RCS1255";
}
}
14 changes: 13 additions & 1 deletion src/Analyzers/CSharp/DiagnosticRules.Generated.cs
Expand Up @@ -778,7 +778,7 @@ public static partial class DiagnosticRules
messageFormat: "Use '{0}' property instead of 'Any' method.",
category: DiagnosticCategories.Roslynator,
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true,
isEnabledByDefault: false,
description: null,
helpLinkUri: DiagnosticIdentifiers.UseCountOrLengthPropertyInsteadOfAnyMethod,
customTags: Array.Empty<string>());
Expand Down Expand Up @@ -2497,5 +2497,17 @@ public static partial class DiagnosticRules
helpLinkUri: DiagnosticIdentifiers.NormalizeFormatOfEnumFlagValue,
customTags: Array.Empty<string>());

/// <summary>RCS1255</summary>
public static readonly DiagnosticDescriptor SimplifyArgumentNullCheck = DiagnosticDescriptorFactory.Create(
id: DiagnosticIdentifiers.SimplifyArgumentNullCheck,
title: "Simplify argument null check.",
messageFormat: "Simplify argument null check.",
category: DiagnosticCategories.Roslynator,
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: false,
description: null,
helpLinkUri: DiagnosticIdentifiers.SimplifyArgumentNullCheck,
customTags: Array.Empty<string>());

}
}
2 changes: 1 addition & 1 deletion src/Tests/Analyzers.Tests/Analyzers.Tests.csproj
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>net6.0</TargetFramework>
</PropertyGroup>

<PropertyGroup>
Expand Down
118 changes: 118 additions & 0 deletions src/Tests/Analyzers.Tests/RCS1255SimplifyArgumentNullCheckTests.cs
@@ -0,0 +1,118 @@
// 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.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Roslynator.CSharp.CodeFixes;
using Roslynator.Testing.CSharp;
using Xunit;

namespace Roslynator.CSharp.Analysis.Tests;

public class RCS1255SimplifyArgumentNullCheckTests : AbstractCSharpDiagnosticVerifier<SimplifyArgumentNullCheckAnalyzer, IfStatementCodeFixProvider>
{
public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.SimplifyArgumentNullCheck;

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.SimplifyArgumentNullCheck)]
public async Task Test_IfStatement_Block_Nameof()
{
await VerifyDiagnosticAndFixAsync(@"
using System;
class C
{
C(string x)
{
[|if|] (x is null)
{
throw new ArgumentNullException(nameof(x));
}
}
}
", @"
using System;
class C
{
C(string x)
{
ArgumentNullException.ThrowIfNull(x);
}
}
");
}

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.SimplifyArgumentNullCheck)]
public async Task Test_IfStatement_Block_Literal()
{
await VerifyDiagnosticAndFixAsync(@"
using System;
class C
{
C(string x)
{
[|if|] (x is null)
{
throw new ArgumentNullException(""x"");
}
}
}
", @"
using System;
class C
{
C(string x)
{
ArgumentNullException.ThrowIfNull(x);
}
}
");
}

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.SimplifyArgumentNullCheck)]
public async Task Test_IfStatement_Embedded_Nameof()
{
await VerifyDiagnosticAndFixAsync(@"
using System;
class C
{
C(string x)
{
[|if|] (x is null)
throw new ArgumentNullException(nameof(x));
}
}
", @"
using System;
class C
{
C(string x)
{
ArgumentNullException.ThrowIfNull(x);
}
}
");
}

[Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.SimplifyArgumentNullCheck)]
public async Task TestNoDiagnostic_TwoArguments()
{
await VerifyNoDiagnosticAsync(@"
using System;
class C
{
C(string x)
{
if (x is null)
{
throw new ArgumentNullException(nameof(x), ""message"");
}
}
}
");
}
}
2 changes: 1 addition & 1 deletion src/Tests/CSharp.Tests/CSharp.Tests.csproj
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>net6.0</TargetFramework>
</PropertyGroup>

<PropertyGroup>
Expand Down

0 comments on commit bae4d29

Please sign in to comment.