Skip to content

Commit

Permalink
Add MA0164 (#761)
Browse files Browse the repository at this point in the history
  • Loading branch information
meziantou authored Oct 16, 2024
1 parent f844f30 commit 7a29d04
Show file tree
Hide file tree
Showing 9 changed files with 263 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|⚠️|✔️|✔️|

<!-- rules -->

Expand Down
7 changes: 7 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
|[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|<span title='Info'>ℹ️</span>|||
|[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|<span title='Info'>ℹ️</span>|||
|[MA0163](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0163.md)|Usage|UseShellExecute must be false when redirecting standard input or output|<span title='Warning'>⚠️</span>|✔️||
|[MA0164](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0164.md)|Style|Use parentheses to not pattern clearer|<span title='Warning'>⚠️</span>|✔️|✔️|

|Id|Suppressed rule|Justification|
|--|---------------|-------------|
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
```
2 changes: 1 addition & 1 deletion docs/Rules/MA0162.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
UseShellExecute = true,
});

````
````
2 changes: 1 addition & 1 deletion docs/Rules/MA0163.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ Process.Start(new ProcessStartInfo("cmd")
UseShellExecute = false,
});

````
````
21 changes: 21 additions & 0 deletions docs/Rules/MA0164.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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<string> 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<Document> 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<Document> 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);
}
1 change: 1 addition & 0 deletions src/Meziantou.Analyzer/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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<DiagnosticDescriptor> 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
Original file line number Diff line number Diff line change
@@ -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<NotPatternShouldBeParenthesizedAnalyzer>()
.WithCodeFixProvider<NotPatternShouldBeParenthesizedCodeFixer>()
.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();
}

0 comments on commit 7a29d04

Please sign in to comment.