From 24ad9141a64c3861b9b116bd2486378ddb420212 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Fri, 18 Nov 2022 16:27:54 +0100 Subject: [PATCH 1/5] Improve RCS1227 --- ...lidateArgumentsCorrectlyCodeFixProvider.cs | 5 ++- .../ValidateArgumentsCorrectlyAnalyzer.cs | 10 ++++- src/Common/ArgumentNullCheckAnalysis.cs | 38 +++++++++++++++---- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/ValidateArgumentsCorrectlyCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/ValidateArgumentsCorrectlyCodeFixProvider.cs index 1146c922ad..7af3a79820 100644 --- a/src/Analyzers.CodeFixes/CSharp/CodeFixes/ValidateArgumentsCorrectlyCodeFixProvider.cs +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/ValidateArgumentsCorrectlyCodeFixProvider.cs @@ -40,16 +40,17 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; Diagnostic diagnostic = context.Diagnostics[0]; + Document document = context.Document; CodeAction codeAction = CodeAction.Create( "Validate arguments correctly", - ct => RefactorAsync(context.Document, statement, ct), + ct => AddLocalFunctionWithIteratorAsync(document, statement, ct), GetEquivalenceKey(diagnostic)); context.RegisterCodeFix(codeAction, diagnostic); } - private static async Task RefactorAsync( + private static async Task AddLocalFunctionWithIteratorAsync( Document document, StatementSyntax statement, CancellationToken cancellationToken) diff --git a/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs b/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs index f5b4ee9006..5dff36f1f4 100644 --- a/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs @@ -57,7 +57,8 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) int index = -1; for (int i = 0; i < statementCount; i++) { - if (ArgumentNullCheckAnalysis.IsArgumentNullCheck(statements[i], context.SemanticModel, context.CancellationToken)) + if (IsConditionWithThrow(statements[i]) + || ArgumentNullCheckAnalysis.IsArgumentNullExceptionThrowIfNullCheck(statements[i], context.SemanticModel, context.CancellationToken)) { index++; } @@ -99,5 +100,12 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) DiagnosticRules.ValidateArgumentsCorrectly, Location.Create(body.SyntaxTree, new TextSpan(statements[index + 1].SpanStart, 0))); } + + private static bool IsConditionWithThrow(StatementSyntax statement) + { + return statement is IfStatementSyntax ifStatement + && ifStatement.IsSimpleIf() + && ifStatement.SingleNonBlockStatementOrDefault().IsKind(SyntaxKind.ThrowStatement); + } } } diff --git a/src/Common/ArgumentNullCheckAnalysis.cs b/src/Common/ArgumentNullCheckAnalysis.cs index 845772f4b2..4a6107ff67 100644 --- a/src/Common/ArgumentNullCheckAnalysis.cs +++ b/src/Common/ArgumentNullCheckAnalysis.cs @@ -37,12 +37,12 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo string name, CancellationToken cancellationToken = default) { - var style = ArgumentNullCheckStyle.None; - string identifier = null; - var success = false; - if (statement is IfStatementSyntax ifStatement) { + var style = ArgumentNullCheckStyle.None; + string identifier = null; + var success = false; + if (ifStatement.SingleNonBlockStatementOrDefault() is ThrowStatementSyntax throwStatement && throwStatement.Expression is ObjectCreationExpressionSyntax objectCreation) { @@ -73,11 +73,27 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo } } } - - return new ArgumentNullCheckAnalysis(style, identifier, success); } + + return new ArgumentNullCheckAnalysis(style, identifier, success); } - else if (statement is ExpressionStatementSyntax expressionStatement) + else + { + return CreateFromArgumentNullExceptionThrowIfNullCheck(statement, semanticModel, name, cancellationToken); + } + } + + private static ArgumentNullCheckAnalysis CreateFromArgumentNullExceptionThrowIfNullCheck( + StatementSyntax statement, + SemanticModel semanticModel, + string name, + CancellationToken cancellationToken) + { + var style = ArgumentNullCheckStyle.None; + string identifier = null; + var success = false; + + if (statement is ExpressionStatementSyntax expressionStatement) { SimpleMemberInvocationStatementInfo invocationInfo = SyntaxInfo.SimpleMemberInvocationStatementInfo(expressionStatement); @@ -103,6 +119,14 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo return new ArgumentNullCheckAnalysis(style, identifier, success); } + public static bool IsArgumentNullExceptionThrowIfNullCheck( + StatementSyntax statement, + SemanticModel semanticModel, + CancellationToken cancellationToken = default) + { + return CreateFromArgumentNullExceptionThrowIfNullCheck(statement, semanticModel, null, cancellationToken).Success; + } + public static bool IsArgumentNullCheck( StatementSyntax statement, SemanticModel semanticModel, From 7e3c0a898b45d75c4dd3793ec7b0bcbd5824c95c Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Fri, 18 Nov 2022 16:31:20 +0100 Subject: [PATCH 2/5] changelog --- ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog.md b/ChangeLog.md index d593683915..c615f2de1d 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Disable [RCS1080](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1080.md) by default ([#980](https://github.com/josefpihrt/roslynator/pull/980)). +- Improve analyzer [RCS1227](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1227.md) ([#992](https://github.com/josefpihrt/roslynator/pull/992). ### Fixed From 96092d5adb505454f2ed3ba47d9e589487bdfb82 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Fri, 18 Nov 2022 16:32:15 +0100 Subject: [PATCH 3/5] changelog --- ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index c615f2de1d..8c4a0b2839 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -14,7 +14,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Disable [RCS1080](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1080.md) by default ([#980](https://github.com/josefpihrt/roslynator/pull/980)). -- Improve analyzer [RCS1227](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1227.md) ([#992](https://github.com/josefpihrt/roslynator/pull/992). ### Fixed @@ -22,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix retrieving of trusted platform assemblies - separator differs by OS ([#987](https://github.com/josefpihrt/roslynator/pull/987)). - Fix refactoring ([RR0014](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RR0014.md)) ([#988](https://github.com/josefpihrt/roslynator/pull/988)). - Fix refactoring ([RR0180](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RR0180.md)) ([#988](https://github.com/josefpihrt/roslynator/pull/988)). +- Improve analyzer [RCS1227](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1227.md) ([#992](https://github.com/josefpihrt/roslynator/pull/992). ## [4.1.2] - 2022-10-31 From 29aa44cd489826e40eb2c9e2295445b4657ded63 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Fri, 18 Nov 2022 21:48:02 +0100 Subject: [PATCH 4/5] update --- src/Common/ArgumentNullCheckAnalysis.cs | 40 ++++++++++--------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/src/Common/ArgumentNullCheckAnalysis.cs b/src/Common/ArgumentNullCheckAnalysis.cs index 4a6107ff67..f187f0c8b0 100644 --- a/src/Common/ArgumentNullCheckAnalysis.cs +++ b/src/Common/ArgumentNullCheckAnalysis.cs @@ -10,17 +10,14 @@ namespace Roslynator.CSharp { internal readonly struct ArgumentNullCheckAnalysis { - private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, bool success) + private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, bool success) { Style = style; - Name = name; Success = success; } public ArgumentNullCheckStyle Style { get; } - public string Name { get; } - public bool Success { get; } public static ArgumentNullCheckAnalysis Create( @@ -40,7 +37,6 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo if (statement is IfStatementSyntax ifStatement) { var style = ArgumentNullCheckStyle.None; - string identifier = null; var success = false; if (ifStatement.SingleNonBlockStatementOrDefault() is ThrowStatementSyntax throwStatement @@ -56,26 +52,22 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo { style = ArgumentNullCheckStyle.IfStatement; - if (nullCheck.Expression is IdentifierNameSyntax identifierName) + if (name is null + || (nullCheck.Expression is IdentifierNameSyntax identifierName + && string.Equals(name, identifierName.Identifier.ValueText, StringComparison.Ordinal))) { - identifier = identifierName.Identifier.ValueText; - - if (name is null - || string.Equals(name, identifier, StringComparison.Ordinal)) + if (semanticModel + .GetSymbol(objectCreation, cancellationToken)? + .ContainingType? + .HasMetadataName(MetadataNames.System_ArgumentNullException) == true) { - if (semanticModel - .GetSymbol(objectCreation, cancellationToken)? - .ContainingType? - .HasMetadataName(MetadataNames.System_ArgumentNullException) == true) - { - success = true; - } + success = true; } } } } - return new ArgumentNullCheckAnalysis(style, identifier, success); + return new ArgumentNullCheckAnalysis(style, success); } else { @@ -90,7 +82,6 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo CancellationToken cancellationToken) { var style = ArgumentNullCheckStyle.None; - string identifier = null; var success = false; if (statement is ExpressionStatementSyntax expressionStatement) @@ -106,17 +97,16 @@ private ArgumentNullCheckAnalysis(ArgumentNullCheckStyle style, string name, boo { style = ArgumentNullCheckStyle.ThrowIfNullMethod; - if (invocationInfo.Arguments.SingleOrDefault(shouldThrow: false)?.Expression is IdentifierNameSyntax identifierName) + if (name is null + || (invocationInfo.Arguments.SingleOrDefault(shouldThrow: false)?.Expression is IdentifierNameSyntax identifierName + && string.Equals(name, identifierName.Identifier.ValueText, StringComparison.Ordinal))) { - identifier = identifierName.Identifier.ValueText; - - if (string.Equals(name, identifier, StringComparison.Ordinal)) - success = true; + success = true; } } } - return new ArgumentNullCheckAnalysis(style, identifier, success); + return new ArgumentNullCheckAnalysis(style, success); } public static bool IsArgumentNullExceptionThrowIfNullCheck( From 349e5685f5f0e7d86782eb67b457fbc57090f6d9 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Fri, 18 Nov 2022 21:54:41 +0100 Subject: [PATCH 5/5] changelog --- ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index 8c4a0b2839..f469b88c58 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix retrieving of trusted platform assemblies - separator differs by OS ([#987](https://github.com/josefpihrt/roslynator/pull/987)). - Fix refactoring ([RR0014](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RR0014.md)) ([#988](https://github.com/josefpihrt/roslynator/pull/988)). - Fix refactoring ([RR0180](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RR0180.md)) ([#988](https://github.com/josefpihrt/roslynator/pull/988)). -- Improve analyzer [RCS1227](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1227.md) ([#992](https://github.com/josefpihrt/roslynator/pull/992). +- Recognize `ArgumentNullException.ThrowIfNull` [RCS1227](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1227.md) ([#992](https://github.com/josefpihrt/roslynator/pull/992). ## [4.1.2] - 2022-10-31