Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyzer: Simplify argument null check (RCS1255) #994

Merged
merged 9 commits into from Nov 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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