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

New rules: MA0141 and MA0142 to convert null check to pattern matching #647

Merged
merged 1 commit into from
Nov 13, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ If you are already using other analyzers, you can check [which rules are duplica
|[MA0138](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0138.md)|Design|Do not use 'Async' suffix when a method does not return an awaitable type|⚠️|❌|❌|
|[MA0139](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0139.md)|Design|Log Parameter type is not valid|⚠️|✔️|❌|
|[MA0140](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0140.md)|Design|Both if and else branch have identical code|⚠️|✔️|❌|
|[MA0141](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0141.md)|Usage|Use pattern matching instead of inequality operators|ℹ️|❌|✔️|
|[MA0142](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0142.md)|Usage|Use pattern matching instead of equality operators|ℹ️|❌|✔️|

<!-- rules -->

Expand Down
14 changes: 14 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@
|[MA0138](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0138.md)|Design|Do not use 'Async' suffix when a method does not return an awaitable type|<span title='Warning'>⚠️</span>|❌|❌|
|[MA0139](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0139.md)|Design|Log Parameter type is not valid|<span title='Warning'>⚠️</span>|✔️|❌|
|[MA0140](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0140.md)|Design|Both if and else branch have identical code|<span title='Warning'>⚠️</span>|✔️|❌|
|[MA0141](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0141.md)|Usage|Use pattern matching instead of inequality operators|<span title='Info'>ℹ️</span>|❌|✔️|
|[MA0142](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0142.md)|Usage|Use pattern matching instead of equality operators|<span title='Info'>ℹ️</span>|❌|✔️|

|Id|Suppressed rule|Justification|
|--|---------------|-------------|
Expand Down Expand Up @@ -566,6 +568,12 @@ dotnet_diagnostic.MA0139.severity = warning

# MA0140: Both if and else branch have identical code
dotnet_diagnostic.MA0140.severity = warning

# MA0141: Use pattern matching instead of inequality operators
dotnet_diagnostic.MA0141.severity = none

# MA0142: Use pattern matching instead of equality operators
dotnet_diagnostic.MA0142.severity = none
```

# .editorconfig - all rules disabled
Expand Down Expand Up @@ -987,4 +995,10 @@ dotnet_diagnostic.MA0139.severity = none

# MA0140: Both if and else branch have identical code
dotnet_diagnostic.MA0140.severity = none

# MA0141: Use pattern matching instead of inequality operators
dotnet_diagnostic.MA0141.severity = none

# MA0142: Use pattern matching instead of equality operators
dotnet_diagnostic.MA0142.severity = none
```
7 changes: 7 additions & 0 deletions docs/Rules/MA0141.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# MA0141 - Use pattern matching instead of inequality operators

````c#
value != null; // not compliant

value is not null; // ok
````
7 changes: 7 additions & 0 deletions docs/Rules/MA0142.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# MA0142 - Use pattern matching instead of equality operators

````c#
value == null; // not compliant

value is null; // ok
````
2 changes: 1 addition & 1 deletion global.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"sdk": {
"version": "7.0.400",
"rollForward": "feature"
"rollForward": "latestMajor"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System.Collections.Immutable;
using System.Composition;
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;
using Microsoft.CodeAnalysis.Operations;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;

namespace Meziantou.Analyzer.Rules;

[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class UsePatternMatchingForNullCheckFixer : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(RuleIdentifiers.UsePatternMatchingForNullCheck, RuleIdentifiers.UsePatternMatchingForNullEquality);

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 not BinaryExpressionSyntax invocation)
return;

context.RegisterCodeFix(
CodeAction.Create(
"Use pattern matching",
ct => Update(context.Document, invocation, ct),
equivalenceKey: "Use pattern matching"),
context.Diagnostics);
}

private static async Task<Document> Update(Document document, BinaryExpressionSyntax node, CancellationToken cancellationToken)
{
var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
if (editor.SemanticModel.GetOperation(node, cancellationToken) is not IBinaryOperation operation)
return document;

var valueSyntax = IsNull(operation.LeftOperand) ? operation.RightOperand.Syntax : operation.LeftOperand.Syntax;
if (valueSyntax is not ExpressionSyntax expression)
return document;

PatternSyntax constantExpression = ConstantPattern(LiteralExpression(SyntaxKind.NullLiteralExpression));
if (operation.OperatorKind is BinaryOperatorKind.NotEquals)
{
constantExpression = UnaryPattern(constantExpression);
}

var newSyntax = IsPatternExpression(expression, constantExpression);
editor.ReplaceNode(node, newSyntax);
return editor.GetChangedDocument();
}

private static bool IsNull(IOperation operation)
=> operation.UnwrapConversionOperations() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
}
2 changes: 2 additions & 0 deletions src/Meziantou.Analyzer/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ internal static class RuleIdentifiers
public const string MethodsNotReturningAnAwaitableTypeMustNotHaveTheAsyncSuffix = "MA0138";
public const string LoggerParameterType_Serilog = "MA0139";
public const string IfElseBranchesAreIdentical = "MA0140";
public const string UsePatternMatchingForNullCheck = "MA0141";
public const string UsePatternMatchingForNullEquality = "MA0142";

public static string GetHelpUri(string identifier)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Meziantou.Analyzer.Rules;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UsePatternMatchingForNullCheckAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor s_ruleEqual = new(
RuleIdentifiers.UsePatternMatchingForNullEquality,
title: "Use pattern matching instead of equality operators",
messageFormat: "Use pattern matching instead of equality operators",
RuleCategories.Usage,
DiagnosticSeverity.Info,
isEnabledByDefault: false,
description: "",
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UsePatternMatchingForNullEquality));

private static readonly DiagnosticDescriptor s_ruleNotEqual = new(
RuleIdentifiers.UsePatternMatchingForNullCheck,
title: "Use pattern matching instead of inequality operators",
messageFormat: "Use pattern matching instead of inequality operators",
RuleCategories.Usage,
DiagnosticSeverity.Info,
isEnabledByDefault: false,
description: "",
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UsePatternMatchingForNullCheck));

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(s_ruleEqual, s_ruleNotEqual);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.RegisterOperationAction(AnalyzeBinary, OperationKind.Binary);
}

private void AnalyzeBinary(OperationAnalysisContext context)
{
var operation = (IBinaryOperation)context.Operation;
if (operation is { OperatorKind: BinaryOperatorKind.Equals or BinaryOperatorKind.NotEquals, OperatorMethod: null })
{
var leftIfNull = IsNull(operation.LeftOperand);
var rightIfNull = IsNull(operation.RightOperand);
if (leftIfNull ^ rightIfNull)
{
context.ReportDiagnostic(operation.OperatorKind is BinaryOperatorKind.Equals ? s_ruleEqual : s_ruleNotEqual, operation);
}
}
}

private static bool IsNull(IOperation operation)
=> operation.UnwrapConversionOperations() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
using System.Threading.Tasks;
using Meziantou.Analyzer.Rules;
using TestHelper;
using Xunit;

namespace Meziantou.Analyzer.Test.Rules;

public sealed class UsePatternMatchingForNullCheckAnalyzerTests
{
private static ProjectBuilder CreateProjectBuilder()
{
return new ProjectBuilder()
.WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication)
.WithAnalyzer<UsePatternMatchingForNullCheckAnalyzer>()
.WithCodeFixProvider<UsePatternMatchingForNullCheckFixer>();
}

[Fact]
public async Task NullCheckForNullableOfT()
{
await CreateProjectBuilder()
.WithSourceCode("_ = [|(int?)0 == null|];")
.ShouldFixCodeWith("_ = (int?)0 is null;")
.ValidateAsync();
}

[Fact]
public async Task NullCheckForNullableOfT_NotNull()
{
await CreateProjectBuilder()
.WithSourceCode("_ = [|(int?)0 != null|];")
.ShouldFixCodeWith("_ = (int?)0 is not null;")
.ValidateAsync();
}

[Fact]
public async Task NullCheckForObject()
{
await CreateProjectBuilder()
.WithSourceCode("_ = [|new object() == null|];")
.ShouldFixCodeWith("_ = new object() is null;")
.ValidateAsync();
}

[Fact]
public async Task NullCheckForObject_NullFirst()
{
await CreateProjectBuilder()
.WithSourceCode("_ = [|null == new object()|];")
.ShouldFixCodeWith("_ = new object() is null;")
.ValidateAsync();
}

[Fact]
public async Task NullCheckForObject_NotNull_NullFirst()
{
await CreateProjectBuilder()
.WithSourceCode("_ = [|null != new object()|];")
.ShouldFixCodeWith("_ = new object() is not null;")
.ValidateAsync();
}

[Fact]
public async Task NullEqualsNull()
{
// no report as "null is null" is not valid
await CreateProjectBuilder()
.WithSourceCode("_ = null == null;")
.ValidateAsync();
}

[Fact]
public async Task NotNullCheck()
{
// no report as "null is null" is not valid
await CreateProjectBuilder()
.WithSourceCode("_ = new object() == new object();")
.ValidateAsync();
}

[Fact]
public async Task NullCheckForObjectWithCustomOperator()
{
await CreateProjectBuilder()
.WithSourceCode("""
_ = new Sample() == null;

class Sample
{
public static bool operator ==(Sample left, Sample right) => false;
public static bool operator !=(Sample left, Sample right) => false;
}
""")
.ValidateAsync();
}

[Fact]
public async Task NullCheckForNullableOfT_IsNull()
{
await CreateProjectBuilder()
.WithSourceCode(@"_ = (int?)0 is null;")
.ValidateAsync();
}
}