diff --git a/README.md b/README.md index 680800e21..87c115ad1 100755 --- a/README.md +++ b/README.md @@ -179,6 +179,7 @@ If you are already using other analyzers, you can check [which rules are duplica |[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|ℹ️|❌|❌| |[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|ℹ️|❌|❌| |[MA0163](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0163.md)|Usage|UseShellExecute must be false when redirecting standard input or output|⚠️|✔️|❌| +|[MA0164](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0164.md)|Style|Use parentheses to not pattern clearer|⚠️|✔️|✔️| diff --git a/docs/README.md b/docs/README.md index bbe1cb816..42376e9ab 100755 --- a/docs/README.md +++ b/docs/README.md @@ -163,6 +163,7 @@ |[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|ℹ️|❌|❌| |[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|ℹ️|❌|❌| |[MA0163](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0163.md)|Usage|UseShellExecute must be false when redirecting standard input or output|⚠️|✔️|❌| +|[MA0164](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0164.md)|Style|Use parentheses to not pattern clearer|⚠️|✔️|✔️| |Id|Suppressed rule|Justification| |--|---------------|-------------| @@ -658,6 +659,9 @@ dotnet_diagnostic.MA0162.severity = none # MA0163: UseShellExecute must be false when redirecting standard input or output dotnet_diagnostic.MA0163.severity = warning + +# MA0164: Use parentheses to not pattern clearer +dotnet_diagnostic.MA0164.severity = warning ``` # .editorconfig - all rules disabled @@ -1148,4 +1152,7 @@ dotnet_diagnostic.MA0162.severity = none # MA0163: UseShellExecute must be false when redirecting standard input or output dotnet_diagnostic.MA0163.severity = none + +# MA0164: Use parentheses to not pattern clearer +dotnet_diagnostic.MA0164.severity = none ``` diff --git a/docs/Rules/MA0162.md b/docs/Rules/MA0162.md index 4205057f5..f6bf215a7 100644 --- a/docs/Rules/MA0162.md +++ b/docs/Rules/MA0162.md @@ -20,4 +20,4 @@ Process.Start(new ProcessStartInfo("https://www.meziantou.net/") UseShellExecute = true, }); -```` \ No newline at end of file +```` diff --git a/docs/Rules/MA0163.md b/docs/Rules/MA0163.md index 8ce3b2cec..fdadb2660 100644 --- a/docs/Rules/MA0163.md +++ b/docs/Rules/MA0163.md @@ -40,4 +40,4 @@ Process.Start(new ProcessStartInfo("cmd") UseShellExecute = false, }); -```` \ No newline at end of file +```` diff --git a/docs/Rules/MA0164.md b/docs/Rules/MA0164.md new file mode 100644 index 000000000..12bfeb872 --- /dev/null +++ b/docs/Rules/MA0164.md @@ -0,0 +1,21 @@ +# MA0164 - Use parentheses to not pattern clearer + +`not` patterns are often wrongly used in combination with the `or` operator. This rule suggests using parentheses to make evaluation order clearer. + +````c# +DayOfWeek day = DayOfWeek.Tuesday; + +var isWeekday = day is not DayOfWeek.Saturday or DayOfWeek.Sunday; // wrong +var isWeekday = day is not (DayOfWeek.Saturday or DayOfWeek.Sunday); // ok +var isWeekday = day is not DayOfWeek.Saturday and not DayOfWeek.Sunday; // ok +```` + +````c# +_ = value is not null or ""; // not-compliant + +_ = value is (not null) or ""; // ok +_ = value is not (null or ""); // ok +```` + +> **Warning** +Note that the provided code fix may not always be correct. It adds parenthesis to show the current evaluation order, but this may not be what is expected. It is recommended to review the code after applying the fix. diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/NotPatternShouldBeParenthesizedCodeFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/NotPatternShouldBeParenthesizedCodeFixer.cs new file mode 100644 index 000000000..e1d3325e0 --- /dev/null +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/NotPatternShouldBeParenthesizedCodeFixer.cs @@ -0,0 +1,72 @@ +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +namespace Meziantou.Analyzer.Rules; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public sealed class NotPatternShouldBeParenthesizedCodeFixer : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(RuleIdentifiers.NotPatternShouldBeParenthesized); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true); + if (nodeToFix is null) + return; + + { + var title = "Add parentheses around not"; + var codeAction = CodeAction.Create( + title, + ct => ParenthesizeNotPattern(context.Document, nodeToFix, ct), + equivalenceKey: title); + context.RegisterCodeFix(codeAction, context.Diagnostics); + } + { + var title = "Negate all or patterns"; + var codeAction = CodeAction.Create( + title, + ct => ParenthesizeOrPattern(context.Document, nodeToFix, ct), + equivalenceKey: title); + context.RegisterCodeFix(codeAction, context.Diagnostics); + } + } + + private static async Task ParenthesizeNotPattern(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + editor.ReplaceNode(nodeToFix, SyntaxFactory.ParenthesizedPattern((PatternSyntax)nodeToFix)); + return editor.GetChangedDocument(); + } + + private static async Task ParenthesizeOrPattern(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + + if (nodeToFix is not UnaryPatternSyntax unary) + return document; + + + var root = unary.Ancestors().TakeWhile(IsOrPattern).LastOrDefault(); + if (root is null) + return document; + + editor.ReplaceNode(root, SyntaxFactory.UnaryPattern(SyntaxFactory.Token(SyntaxKind.NotKeyword), SyntaxFactory.ParenthesizedPattern((PatternSyntax)root.ReplaceNode(unary, unary.Pattern)))); + + return editor.GetChangedDocument(); + } + + private static bool IsOrPattern(SyntaxNode? node) => node is BinaryPatternSyntax binaryPatternSyntax && binaryPatternSyntax.OperatorToken.IsKind(SyntaxKind.OrKeyword); +} diff --git a/src/Meziantou.Analyzer/RuleIdentifiers.cs b/src/Meziantou.Analyzer/RuleIdentifiers.cs index 5a64213f5..fcb95b97b 100755 --- a/src/Meziantou.Analyzer/RuleIdentifiers.cs +++ b/src/Meziantou.Analyzer/RuleIdentifiers.cs @@ -166,6 +166,7 @@ internal static class RuleIdentifiers public const string UseShellExecuteMustBeSet = "MA0161"; public const string UseProcessStartOverload = "MA0162"; public const string UseShellExecuteMustBeFalse = "MA0163"; + public const string NotPatternShouldBeParenthesized = "MA0164"; public static string GetHelpUri(string identifier) { diff --git a/src/Meziantou.Analyzer/Rules/NotPatternShouldBeParenthesizedAnalyzer.cs b/src/Meziantou.Analyzer/Rules/NotPatternShouldBeParenthesizedAnalyzer.cs new file mode 100644 index 000000000..2381eee1c --- /dev/null +++ b/src/Meziantou.Analyzer/Rules/NotPatternShouldBeParenthesizedAnalyzer.cs @@ -0,0 +1,48 @@ +#if CSHARP9_OR_GREATER +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Meziantou.Analyzer.Rules; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class NotPatternShouldBeParenthesizedAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor Rule = new( + RuleIdentifiers.NotPatternShouldBeParenthesized, + title: "Use parentheses to not pattern clearer", + messageFormat: "Use parentheses to make it clearer", + RuleCategories.Style, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "", + helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.NotPatternShouldBeParenthesized)); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterSyntaxNodeAction(AnalyzeNotPatternSyntax, SyntaxKind.NotPattern); + } + + private void AnalyzeNotPatternSyntax(SyntaxNodeAnalysisContext context) + { + var node = context.Node; + if (node.Parent is null || node.Parent.IsKind(SyntaxKind.ParenthesizedPattern)) + return; + + if (node.Parent is BinaryPatternSyntax binaryPattern && binaryPattern.OperatorToken.IsKind(SyntaxKind.OrKeyword)) + { + if (binaryPattern.Left == node) + { + context.ReportDiagnostic(Diagnostic.Create(Rule, node.GetLocation())); + } + } + } +} +#endif \ No newline at end of file diff --git a/tests/Meziantou.Analyzer.Test/Rules/NotPatternShouldBeParenthesizedAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/NotPatternShouldBeParenthesizedAnalyzerTests.cs new file mode 100644 index 000000000..bfda10593 --- /dev/null +++ b/tests/Meziantou.Analyzer.Test/Rules/NotPatternShouldBeParenthesizedAnalyzerTests.cs @@ -0,0 +1,111 @@ +using System.Threading.Tasks; +using Meziantou.Analyzer.Rules; +using TestHelper; +using Xunit; + +namespace Meziantou.Analyzer.Test.Rules; +public sealed class NotPatternShouldBeParenthesizedAnalyzerTests +{ + private static ProjectBuilder CreateProjectBuilder() + => new ProjectBuilder() + .WithAnalyzer() + .WithCodeFixProvider() + .WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication); + + [Fact] + public async Task Not_Null() + => await CreateProjectBuilder() + .WithSourceCode(""" + string a = default; + _ = a is not null; + """) + .ValidateAsync(); + + [Fact] + public async Task Not_Null_Or_Empty() + => await CreateProjectBuilder() + .WithSourceCode(""" + string a = default; + _ = a is [|not null|] or ""; + """) + .ShouldFixCodeWith(index: 0, """ + string a = default; + _ = a is (not null) or ""; + """) + .ValidateAsync(); + + [Fact] + public async Task Not_Null_And_Empty() + => await CreateProjectBuilder() + .WithSourceCode(""" + string a = default; + _ = a is not null and ""; + """) + .ValidateAsync(); + + [Fact] + public async Task Not_Or_GreaterThan() + => await CreateProjectBuilder() + .WithSourceCode(""" + int a = default; + _ = a is [|not 1|] or > 2; + """) + .ShouldFixCodeWith(index: 0, """ + int a = default; + _ = a is (not 1) or > 2; + """) + .ValidateAsync(); + + [Fact] + public async Task Parentheses_Not_Or_GreaterThan() + => await CreateProjectBuilder() + .WithSourceCode(""" + int a = 1; + _ = a is (not 1) or > 2; + """) + .ValidateAsync(); + + [Fact] + public async Task GreaterThan_Or_Not() + => await CreateProjectBuilder() + .WithSourceCode(""" + int a = 1; + _ = a is 1 or not (< 0); + """) + .ValidateAsync(); + + [Fact] + public async Task GreaterThan_Or_Not_Or_Not() + => await CreateProjectBuilder() + .WithSourceCode(""" + int a = 1; + _ = a is 1 or not < 0 or not > 1; + """) + .ValidateAsync(); + + [Fact] + public async Task Not_Many_or_Fix1() + => await CreateProjectBuilder() + .WithSourceCode(""" + int a = 1; + _ = a is [|not 1|] or 2 or 3 or 4; + """) + .ShouldFixCodeWith(index: 0, """ + int a = 1; + _ = a is (not 1) or 2 or 3 or 4; + """) + .ValidateAsync(); + + [Fact] + public async Task Not_Many_or_Fix2() + => await CreateProjectBuilder() + .WithSourceCode(""" + int a = 1; + _ = a is [|not 1|] or 2 or 3 or 4; + """) + .ShouldFixCodeWith(index: 1, """ + int a = 1; + _ = a is not (1 or 2 or 3 or 4); + """) + .ValidateAsync(); +}