diff --git a/README.md b/README.md index 0c34e061..82cf3453 100755 --- a/README.md +++ b/README.md @@ -212,6 +212,7 @@ If you are already using other analyzers, you can check [which rules are duplica |[MA0191](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0191.md)|Design|Do not use the null-forgiving operator|⚠️|❌|❌| |[MA0192](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0192.md)|Usage|Use HasFlag instead of bitwise checks|ℹ️|❌|✔️| |[MA0193](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0193.md)|Usage|Use an overload with a MidpointRounding argument|ℹ️|✔️|✔️| +|[MA0194](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0194.md)|Usage|Merge is expressions on the same value|ℹ️|✔️|✔️| diff --git a/docs/README.md b/docs/README.md index a78ce975..77406476 100755 --- a/docs/README.md +++ b/docs/README.md @@ -192,6 +192,7 @@ |[MA0191](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0191.md)|Design|Do not use the null-forgiving operator|⚠️|❌|❌| |[MA0192](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0192.md)|Usage|Use HasFlag instead of bitwise checks|ℹ️|❌|✔️| |[MA0193](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0193.md)|Usage|Use an overload with a MidpointRounding argument|ℹ️|✔️|✔️| +|[MA0194](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0194.md)|Usage|Merge is expressions on the same value|ℹ️|✔️|✔️| |Id|Suppressed rule|Justification| |--|---------------|-------------| @@ -783,6 +784,9 @@ dotnet_diagnostic.MA0192.severity = none # MA0193: Use an overload with a MidpointRounding argument dotnet_diagnostic.MA0193.severity = suggestion + +# MA0194: Merge is expressions on the same value +dotnet_diagnostic.MA0194.severity = suggestion ``` # .editorconfig - all rules disabled @@ -1360,4 +1364,7 @@ dotnet_diagnostic.MA0192.severity = none # MA0193: Use an overload with a MidpointRounding argument dotnet_diagnostic.MA0193.severity = none + +# MA0194: Merge is expressions on the same value +dotnet_diagnostic.MA0194.severity = none ``` diff --git a/docs/Rules/MA0194.md b/docs/Rules/MA0194.md new file mode 100644 index 00000000..b0477589 --- /dev/null +++ b/docs/Rules/MA0194.md @@ -0,0 +1,14 @@ +# MA0194 - Merge is expressions on the same value + +Sources: [MergeIsPatternChecksAnalyzer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer/Rules/MergeIsPatternChecksAnalyzer.cs), [MergeIsPatternChecksFixer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer.CodeFixers/Rules/MergeIsPatternChecksFixer.cs) + + +````c# +value is 1 || value is 2; // not compliant + +value is 1 or 2; // ok + +value is MyEnum.Value1 && value is not MyEnum.Value2; // not compliant + +value is MyEnum.Value1 and not MyEnum.Value2; // ok +```` diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/MergeIsPatternChecksFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/MergeIsPatternChecksFixer.cs new file mode 100644 index 00000000..0b34641e --- /dev/null +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/MergeIsPatternChecksFixer.cs @@ -0,0 +1,440 @@ +using System.Collections.Immutable; +using System.Composition; +using Meziantou.Analyzer.Internals; +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 MergeIsPatternChecksFixer : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(RuleIdentifiers.MergeIsPatternChecks); + + 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 binaryExpression) + return; + + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (semanticModel is null) + return; + + var expressionToFix = GetContainingLogicalExpression(binaryExpression); + var updatedExpression = RewriteExpression(expressionToFix, semanticModel, context.CancellationToken); + if (AreEquivalent(expressionToFix, updatedExpression)) + return; + + context.RegisterCodeFix( + CodeAction.Create( + "Merge is expressions", + ct => UpdateDocumentAsync(context.Document, binaryExpression, ct), + equivalenceKey: "Merge is expressions"), + context.Diagnostics); + } + + internal static async Task UpdateDocumentAsync(Document document, BinaryExpressionSyntax node, CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + var expressionToFix = GetContainingLogicalExpression(node); + var updatedExpression = RewriteExpression(expressionToFix, editor.SemanticModel, cancellationToken); + if (AreEquivalent(expressionToFix, updatedExpression)) + return document; + + editor.ReplaceNode(expressionToFix, updatedExpression.WithTriviaFrom(expressionToFix)); + return editor.GetChangedDocument(); + } + + private static ExpressionSyntax GetContainingLogicalExpression(BinaryExpressionSyntax binaryExpression) + { + ExpressionSyntax current = binaryExpression; + while (TryGetParentLogicalExpression(current, out var parentBinary)) + { + current = parentBinary; + } + + return current; + } + + private static bool TryGetParentLogicalExpression(ExpressionSyntax expression, out BinaryExpressionSyntax parentBinary) + { + var parent = expression.Parent; + if (parent is ParenthesizedExpressionSyntax parenthesizedExpression) + { + parent = parenthesizedExpression.Parent; + } + + if (parent is BinaryExpressionSyntax binaryExpression && IsLogicalBinary(binaryExpression.Kind())) + { + parentBinary = binaryExpression; + return true; + } + + parentBinary = null!; + return false; + } + + private static ExpressionSyntax RewriteExpression(ExpressionSyntax expression, SemanticModel semanticModel, CancellationToken cancellationToken) + { + if (expression is ParenthesizedExpressionSyntax parenthesizedExpression) + { + var updatedExpression = RewriteExpression(parenthesizedExpression.Expression, semanticModel, cancellationToken); + return AreEquivalent(parenthesizedExpression.Expression, updatedExpression) ? expression : parenthesizedExpression.WithExpression(updatedExpression); + } + + if (expression is BinaryExpressionSyntax binaryExpression && IsLogicalBinary(binaryExpression.Kind())) + { + return RewriteLogicalBinaryExpression(binaryExpression, semanticModel, cancellationToken); + } + + return expression; + } + + private static ExpressionSyntax RewriteLogicalBinaryExpression(BinaryExpressionSyntax rootExpression, SemanticModel semanticModel, CancellationToken cancellationToken) + { + var logicalExpressionKind = rootExpression.Kind(); + var terms = new List(); + FlattenLogicalTerms(rootExpression, logicalExpressionKind, terms); + + var mergeCandidates = new List(); + var updatedTerms = new List(terms.Count); + foreach (var term in terms) + { + if (TryCreateMergeCandidate(term, semanticModel, cancellationToken, out var candidate)) + { + if (mergeCandidates.Count == 0 || AreSameMergeTarget(mergeCandidates[0].Target, candidate.Target)) + { + mergeCandidates.Add(candidate); + } + else + { + FlushCandidates(logicalExpressionKind, mergeCandidates, updatedTerms, semanticModel, cancellationToken); + mergeCandidates.Add(candidate); + } + + continue; + } + + FlushCandidates(logicalExpressionKind, mergeCandidates, updatedTerms, semanticModel, cancellationToken); + updatedTerms.Add(RewriteExpression(term, semanticModel, cancellationToken)); + } + + FlushCandidates(logicalExpressionKind, mergeCandidates, updatedTerms, semanticModel, cancellationToken); + + if (updatedTerms.Count == 0) + return rootExpression; + + var updatedExpression = updatedTerms[0]; + for (var i = 1; i < updatedTerms.Count; i++) + { + updatedExpression = logicalExpressionKind switch + { + SyntaxKind.LogicalAndExpression => BinaryExpression(SyntaxKind.LogicalAndExpression, updatedExpression, updatedTerms[i]), + SyntaxKind.LogicalOrExpression => BinaryExpression(SyntaxKind.LogicalOrExpression, updatedExpression, updatedTerms[i]), + _ => throw new InvalidOperationException("Unexpected logical expression kind"), + }; + } + + return updatedExpression; + } + + private static void FlushCandidates(SyntaxKind logicalExpressionKind, List mergeCandidates, List updatedTerms, SemanticModel semanticModel, CancellationToken cancellationToken) + { + if (mergeCandidates.Count == 0) + return; + + if (mergeCandidates.Count == 1) + { + updatedTerms.Add(RewriteExpression(mergeCandidates[0].TermExpression, semanticModel, cancellationToken)); + } + else if (CanMergeCandidates(logicalExpressionKind, mergeCandidates)) + { + updatedTerms.Add(CreateMergedPatternExpression(logicalExpressionKind, mergeCandidates)); + } + else + { + foreach (var candidate in mergeCandidates) + { + updatedTerms.Add(RewriteExpression(candidate.TermExpression, semanticModel, cancellationToken)); + } + } + + mergeCandidates.Clear(); + } + + private static IsPatternExpressionSyntax CreateMergedPatternExpression(SyntaxKind logicalExpressionKind, List mergeCandidates) + { + var binaryPatternKind = logicalExpressionKind switch + { + SyntaxKind.LogicalAndExpression => SyntaxKind.AndPattern, + SyntaxKind.LogicalOrExpression => SyntaxKind.OrPattern, + _ => throw new InvalidOperationException("Unexpected logical expression kind"), + }; + + var operatorTokenKind = logicalExpressionKind switch + { + SyntaxKind.LogicalAndExpression => SyntaxKind.AndKeyword, + SyntaxKind.LogicalOrExpression => SyntaxKind.OrKeyword, + _ => throw new InvalidOperationException("Unexpected logical expression kind"), + }; + + PatternSyntax mergedPattern = mergeCandidates[0].Pattern; + for (var i = 1; i < mergeCandidates.Count; i++) + { + mergedPattern = BinaryPattern( + binaryPatternKind, + ParenthesizePatternIfNeeded(mergedPattern, binaryPatternKind), + Token(operatorTokenKind), + ParenthesizePatternIfNeeded(mergeCandidates[i].Pattern, binaryPatternKind)); + } + + return IsPatternExpression(mergeCandidates[0].Expression.Parentheses(), mergedPattern); + } + + private static PatternSyntax ParenthesizePatternIfNeeded(PatternSyntax pattern, SyntaxKind parentPatternKind) + { + if (pattern is ParenthesizedPatternSyntax) + return pattern; + + if (pattern is BinaryPatternSyntax binaryPattern && + parentPatternKind is SyntaxKind.AndPattern && + binaryPattern.Kind() is SyntaxKind.OrPattern) + { + return ParenthesizedPattern(pattern); + } + + return pattern; + } + + private static void FlattenLogicalTerms(ExpressionSyntax expression, SyntaxKind logicalExpressionKind, List terms) + { + if (expression is BinaryExpressionSyntax binaryExpression && binaryExpression.IsKind(logicalExpressionKind)) + { + FlattenLogicalTerms(binaryExpression.Left, logicalExpressionKind, terms); + FlattenLogicalTerms(binaryExpression.Right, logicalExpressionKind, terms); + return; + } + + terms.Add(expression); + } + + private static bool TryCreateMergeCandidate(ExpressionSyntax expression, SemanticModel semanticModel, CancellationToken cancellationToken, out MergeCandidate candidate) + { + candidate = default; + + var operation = semanticModel.GetOperation(expression, cancellationToken); + if (operation is null) + return false; + + operation = UnwrapOperation(operation); + if (operation is not IIsPatternOperation isPatternOperation) + return false; + + if (isPatternOperation.Value.Syntax is not ExpressionSyntax valueExpression) + return false; + + if (!TryGetMergeTarget(isPatternOperation.Value, out var mergeTarget)) + return false; + + if (!TryCreatePatternSyntax(isPatternOperation.Pattern, out var patternSyntax)) + return false; + + candidate = new(expression, mergeTarget, UnwrapParentheses(valueExpression), patternSyntax); + return true; + } + + private static IOperation UnwrapOperation(IOperation operation) + { + operation = operation.UnwrapConversionOperations(); + while (operation is IParenthesizedOperation parenthesizedOperation) + { + operation = parenthesizedOperation.Operand.UnwrapConversionOperations(); + } + + return operation; + } + + private static bool TryCreatePatternSyntax(IPatternOperation patternOperation, out PatternSyntax patternSyntax) + { + switch (patternOperation) + { + case IConstantPatternOperation constantPatternOperation: + if (constantPatternOperation.Syntax is PatternSyntax syntaxPattern) + { + patternSyntax = syntaxPattern; + return true; + } + + if (constantPatternOperation.Value?.Syntax is ExpressionSyntax expressionSyntax) + { + patternSyntax = ConstantPattern(expressionSyntax); + return true; + } + + break; + + case INegatedPatternOperation negatedPatternOperation: + if (TryCreatePatternSyntax(negatedPatternOperation.Pattern, out var negatedPatternSyntax)) + { + patternSyntax = UnaryPattern( + negatedPatternSyntax is BinaryPatternSyntax + ? ParenthesizedPattern(negatedPatternSyntax) + : negatedPatternSyntax); + return true; + } + + break; + + case IBinaryPatternOperation binaryPatternOperation: + if (TryCreatePatternSyntax(binaryPatternOperation.LeftPattern, out var leftPatternSyntax) && + TryCreatePatternSyntax(binaryPatternOperation.RightPattern, out var rightPatternSyntax) && + TryGetPatternOperator(binaryPatternOperation.OperatorKind, out var binaryPatternKind, out var operatorTokenKind)) + { + patternSyntax = BinaryPattern( + binaryPatternKind, + ParenthesizePatternIfNeeded(leftPatternSyntax, binaryPatternKind), + Token(operatorTokenKind), + ParenthesizePatternIfNeeded(rightPatternSyntax, binaryPatternKind)); + return true; + } + + break; + } + + patternSyntax = null!; + return false; + } + + private static bool TryGetPatternOperator(BinaryOperatorKind operatorKind, out SyntaxKind binaryPatternKind, out SyntaxKind operatorTokenKind) + { + switch (operatorKind) + { + case BinaryOperatorKind.And: + binaryPatternKind = SyntaxKind.AndPattern; + operatorTokenKind = SyntaxKind.AndKeyword; + return true; + case BinaryOperatorKind.Or: + binaryPatternKind = SyntaxKind.OrPattern; + operatorTokenKind = SyntaxKind.OrKeyword; + return true; + default: + binaryPatternKind = default; + operatorTokenKind = default; + return false; + } + } + + private static bool TryGetMergeTarget(IOperation operation, out MergeTarget mergeTarget) + { + operation = UnwrapOperation(operation); + switch (operation) + { + case ILocalReferenceOperation localReferenceOperation: + mergeTarget = new(localReferenceOperation.Local); + return true; + case IParameterReferenceOperation parameterReferenceOperation: + mergeTarget = new(parameterReferenceOperation.Parameter); + return true; + case IFieldReferenceOperation fieldReferenceOperation when TryGetOptionalMergeTarget(fieldReferenceOperation.Instance, out var fieldInstance): + mergeTarget = new(fieldReferenceOperation.Field, fieldInstance); + return true; + case IPropertyReferenceOperation propertyReferenceOperation when TryGetOptionalMergeTarget(propertyReferenceOperation.Instance, out var propertyInstance): + mergeTarget = new(propertyReferenceOperation.Property, propertyInstance); + return true; + case IEventReferenceOperation eventReferenceOperation when TryGetOptionalMergeTarget(eventReferenceOperation.Instance, out var eventInstance): + mergeTarget = new(eventReferenceOperation.Event, eventInstance); + return true; + case IInstanceReferenceOperation instanceReferenceOperation when instanceReferenceOperation.Type is not null: + mergeTarget = new(instanceReferenceOperation.Type); + return true; + default: + mergeTarget = null!; + return false; + } + } + + private static bool TryGetOptionalMergeTarget(IOperation? operation, out MergeTarget? mergeTarget) + { + if (operation is null) + { + mergeTarget = null; + return true; + } + + if (TryGetMergeTarget(operation, out var target)) + { + mergeTarget = target; + return true; + } + + mergeTarget = null; + return false; + } + + private static bool AreSameMergeTarget(MergeTarget left, MergeTarget right) + { + return SymbolEqualityComparer.Default.Equals(left.Symbol, right.Symbol) && + AreSameOptionalMergeTarget(left.Instance, right.Instance); + } + + private static bool AreSameOptionalMergeTarget(MergeTarget? left, MergeTarget? right) + { + if (left is null) + return right is null; + + if (right is null) + return false; + + return AreSameMergeTarget(left, right); + } + + private static bool CanMergeCandidates(SyntaxKind logicalExpressionKind, List mergeCandidates) + { + if (logicalExpressionKind is not SyntaxKind.LogicalAndExpression) + return true; + + foreach (var candidate in mergeCandidates) + { + if (!IsPositiveConstantPattern(candidate.Pattern)) + return true; + } + + return false; + } + + private static bool IsPositiveConstantPattern(PatternSyntax pattern) + { + return pattern switch + { + ConstantPatternSyntax => true, + ParenthesizedPatternSyntax parenthesizedPattern => IsPositiveConstantPattern(parenthesizedPattern.Pattern), + _ => false, + }; + } + + private static ExpressionSyntax UnwrapParentheses(ExpressionSyntax expression) + { + while (expression is ParenthesizedExpressionSyntax parenthesizedExpression) + { + expression = parenthesizedExpression.Expression; + } + + return expression; + } + + private static bool IsLogicalBinary(SyntaxKind kind) => kind is SyntaxKind.LogicalAndExpression or SyntaxKind.LogicalOrExpression; + + private sealed record class MergeTarget(ISymbol Symbol, MergeTarget? Instance = null); + + private readonly record struct MergeCandidate(ExpressionSyntax TermExpression, MergeTarget Target, ExpressionSyntax Expression, PatternSyntax Pattern); +} diff --git a/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig b/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig index 23767b0f..1ffa4c08 100644 --- a/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig +++ b/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig @@ -574,3 +574,6 @@ dotnet_diagnostic.MA0192.severity = none # MA0193: Use an overload with a MidpointRounding argument dotnet_diagnostic.MA0193.severity = suggestion + +# MA0194: Merge is expressions on the same value +dotnet_diagnostic.MA0194.severity = suggestion diff --git a/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig b/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig index ff3c0443..f406df5f 100644 --- a/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig +++ b/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig @@ -574,3 +574,6 @@ dotnet_diagnostic.MA0192.severity = none # MA0193: Use an overload with a MidpointRounding argument dotnet_diagnostic.MA0193.severity = none + +# MA0194: Merge is expressions on the same value +dotnet_diagnostic.MA0194.severity = none diff --git a/src/Meziantou.Analyzer/RuleIdentifiers.cs b/src/Meziantou.Analyzer/RuleIdentifiers.cs index e99b2981..b0cc2812 100755 --- a/src/Meziantou.Analyzer/RuleIdentifiers.cs +++ b/src/Meziantou.Analyzer/RuleIdentifiers.cs @@ -193,6 +193,7 @@ internal static class RuleIdentifiers public const string DoNotUseNullForgiveness = "MA0191"; public const string UseHasFlagMethod = "MA0192"; public const string UseAnOverloadThatHasMidpointRounding = "MA0193"; + public const string MergeIsPatternChecks = "MA0194"; public static string GetHelpUri(string identifier) { diff --git a/src/Meziantou.Analyzer/Rules/MergeIsPatternChecksAnalyzer.cs b/src/Meziantou.Analyzer/Rules/MergeIsPatternChecksAnalyzer.cs new file mode 100644 index 00000000..355ddf16 --- /dev/null +++ b/src/Meziantou.Analyzer/Rules/MergeIsPatternChecksAnalyzer.cs @@ -0,0 +1,205 @@ +using System.Collections.Immutable; +using Meziantou.Analyzer.Internals; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Meziantou.Analyzer.Rules; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class MergeIsPatternChecksAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor Rule = new( + RuleIdentifiers.MergeIsPatternChecks, + title: "Merge is expressions on the same value", + messageFormat: "Merge is expressions on the same value", + RuleCategories.Usage, + DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: "", + helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.MergeIsPatternChecks)); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(context => + { + if (context.Compilation.GetCSharpLanguageVersion() < LanguageVersion.CSharp9) + return; + + context.RegisterSyntaxNodeAction(AnalyzeBinary, SyntaxKind.LogicalAndExpression, SyntaxKind.LogicalOrExpression); + }); + } + + private static void AnalyzeBinary(SyntaxNodeAnalysisContext context) + { + var operation = context.SemanticModel.GetOperation(context.Node, context.CancellationToken) as IBinaryOperation; + if (operation is null) + return; + + if (operation.OperatorKind is not (BinaryOperatorKind.ConditionalAnd or BinaryOperatorKind.ConditionalOr)) + return; + + if (context.Node is not BinaryExpressionSyntax rootExpression) + return; + + if (rootExpression.Parent is BinaryExpressionSyntax parentExpression && parentExpression.IsKind(rootExpression.Kind())) + return; + + if (!HasMergeableContiguousCandidates(rootExpression, context.SemanticModel, context.CancellationToken)) + return; + + context.ReportDiagnostic(Rule, rootExpression); + } + + private static bool HasMergeableContiguousCandidates(BinaryExpressionSyntax rootExpression, SemanticModel semanticModel, CancellationToken cancellationToken) + { + var terms = new List(); + FlattenLogicalTerms(rootExpression, rootExpression.Kind(), terms); + + var currentGroup = new List(); + foreach (var term in terms) + { + if (TryCreateMergeCandidate(term, semanticModel, cancellationToken, out var candidate)) + { + if (currentGroup.Count == 0 || AreSameMergeTarget(currentGroup[0].Target, candidate.Target)) + { + currentGroup.Add(candidate); + } + else + { + if (currentGroup.Count > 1) + return true; + + currentGroup.Clear(); + currentGroup.Add(candidate); + } + } + else + { + if (currentGroup.Count > 1) + return true; + + currentGroup.Clear(); + } + } + + return currentGroup.Count > 1; + } + + private static void FlattenLogicalTerms(ExpressionSyntax expression, SyntaxKind operatorKind, List terms) + { + if (expression is BinaryExpressionSyntax binaryExpression && binaryExpression.IsKind(operatorKind)) + { + FlattenLogicalTerms(binaryExpression.Left, operatorKind, terms); + FlattenLogicalTerms(binaryExpression.Right, operatorKind, terms); + return; + } + + terms.Add(expression); + } + + private static bool TryCreateMergeCandidate(ExpressionSyntax expression, SemanticModel semanticModel, CancellationToken cancellationToken, out MergeCandidate candidate) + { + candidate = default; + + var operation = semanticModel.GetOperation(expression, cancellationToken); + if (operation is null) + return false; + + operation = UnwrapOperation(operation); + if (operation is not IIsPatternOperation isPatternOperation) + return false; + + if (!TryGetMergeTarget(isPatternOperation.Value, out var mergeTarget)) + return false; + + candidate = new(mergeTarget); + return true; + } + + private static IOperation UnwrapOperation(IOperation operation) + { + operation = operation.UnwrapConversionOperations(); + while (operation is IParenthesizedOperation parenthesizedOperation) + { + operation = parenthesizedOperation.Operand.UnwrapConversionOperations(); + } + + return operation; + } + + private static bool TryGetMergeTarget(IOperation operation, out MergeTarget mergeTarget) + { + operation = UnwrapOperation(operation); + switch (operation) + { + case ILocalReferenceOperation localReferenceOperation: + mergeTarget = new(localReferenceOperation.Local); + return true; + case IParameterReferenceOperation parameterReferenceOperation: + mergeTarget = new(parameterReferenceOperation.Parameter); + return true; + case IFieldReferenceOperation fieldReferenceOperation when TryGetOptionalMergeTarget(fieldReferenceOperation.Instance, out var fieldInstance): + mergeTarget = new(fieldReferenceOperation.Field, fieldInstance); + return true; + case IPropertyReferenceOperation propertyReferenceOperation when TryGetOptionalMergeTarget(propertyReferenceOperation.Instance, out var propertyInstance): + mergeTarget = new(propertyReferenceOperation.Property, propertyInstance); + return true; + case IEventReferenceOperation eventReferenceOperation when TryGetOptionalMergeTarget(eventReferenceOperation.Instance, out var eventInstance): + mergeTarget = new(eventReferenceOperation.Event, eventInstance); + return true; + case IInstanceReferenceOperation instanceReferenceOperation when instanceReferenceOperation.Type is not null: + mergeTarget = new(instanceReferenceOperation.Type); + return true; + default: + mergeTarget = null!; + return false; + } + } + + private static bool TryGetOptionalMergeTarget(IOperation? operation, out MergeTarget? mergeTarget) + { + if (operation is null) + { + mergeTarget = null; + return true; + } + + if (TryGetMergeTarget(operation, out var target)) + { + mergeTarget = target; + return true; + } + + mergeTarget = null; + return false; + } + + private static bool AreSameMergeTarget(MergeTarget left, MergeTarget right) + { + return SymbolEqualityComparer.Default.Equals(left.Symbol, right.Symbol) && + AreSameOptionalMergeTarget(left.Instance, right.Instance); + } + + private static bool AreSameOptionalMergeTarget(MergeTarget? left, MergeTarget? right) + { + if (left is null) + return right is null; + + if (right is null) + return false; + + return AreSameMergeTarget(left, right); + } + + private sealed record class MergeTarget(ISymbol Symbol, MergeTarget? Instance = null); + + private readonly record struct MergeCandidate(MergeTarget Target); +} diff --git a/tests/Meziantou.Analyzer.Test/Rules/MergeIsPatternChecksAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/MergeIsPatternChecksAnalyzerTests.cs new file mode 100644 index 00000000..b1b4253b --- /dev/null +++ b/tests/Meziantou.Analyzer.Test/Rules/MergeIsPatternChecksAnalyzerTests.cs @@ -0,0 +1,293 @@ +using Meziantou.Analyzer.Rules; +using Microsoft.CodeAnalysis; +using TestHelper; + +namespace Meziantou.Analyzer.Test.Rules; + +public sealed class MergeIsPatternChecksAnalyzerTests +{ + private static ProjectBuilder CreateProjectBuilder() + { + return new ProjectBuilder() + .WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication) + .WithAnalyzer() + .WithCodeFixProvider(); + } + + [Fact] + public async Task LogicalOr_ConstantPattern() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = 0; + _ = [|value is 1 || value is 2|]; + """) + .ShouldFixCodeWith(""" + var value = 0; + _ = value is 1 or 2; + """) + .ValidateAsync(); + } + + [Fact] + public async Task LogicalOr_EnumPattern() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = (System.DayOfWeek)0; + _ = [|value is System.DayOfWeek.Monday || value is System.DayOfWeek.Tuesday|]; + """) + .ShouldFixCodeWith(""" + var value = (System.DayOfWeek)0; + _ = value is System.DayOfWeek.Monday or System.DayOfWeek.Tuesday; + """) + .ValidateAsync(); + } + + [Fact] + public async Task LogicalAnd_EnumPattern() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = (System.DayOfWeek)0; + _ = [|value is System.DayOfWeek.Monday && value is System.DayOfWeek.Tuesday|]; + """) + .ValidateAsync(); + } + + [Fact] + public async Task LogicalAnd_NotPattern() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = (System.DayOfWeek)0; + _ = [|value is System.DayOfWeek.Monday && value is not System.DayOfWeek.Tuesday|]; + """) + .ShouldFixCodeWith(""" + var value = (System.DayOfWeek)0; + _ = value is System.DayOfWeek.Monday and not System.DayOfWeek.Tuesday; + """) + .ValidateAsync(); + } + + [Fact] + public async Task LogicalAnd_ParenthesizeOrPattern() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = MyEnum.Value1; + _ = [|value is (MyEnum.Value1 or MyEnum.Value2) && value is not MyEnum.Value2|]; + + enum MyEnum { Value1, Value2 } + """) + .ShouldFixCodeWith(""" + var value = MyEnum.Value1; + _ = value is (MyEnum.Value1 or MyEnum.Value2) and not MyEnum.Value2; + + enum MyEnum { Value1, Value2 } + """) + .ValidateAsync(); + } + + [Fact] + public async Task DifferentExpressions_DoNotReport() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value1 = MyEnum.Value1; + var value2 = MyEnum.Value2; + _ = value1 is MyEnum.Value1 || value2 is MyEnum.Value2; + + enum MyEnum { Value1, Value2 } + """) + .ValidateAsync(); + } + + [Fact] + public async Task AlreadyMerged_DoNotReport() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = MyEnum.Value1; + _ = value is MyEnum.Value1 or MyEnum.Value2; + + enum MyEnum { Value1, Value2 } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Parameter() + { + await CreateProjectBuilder() + .WithSourceCode(""" + static bool M(int value) => [|value is 1 || value is 2|]; + """) + .ShouldFixCodeWith(""" + static bool M(int value) => value is 1 or 2; + """) + .ValidateAsync(); + } + + [Fact] + public async Task LocalVariable() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var value = 0; + _ = [|value is 1 || value is 2|]; + """) + .ShouldFixCodeWith(""" + var value = 0; + _ = value is 1 or 2; + """) + .ValidateAsync(); + } + + [Fact] + public async Task Field_ExplicitAndImplicitThis() + { + await CreateProjectBuilder() + .WithSourceCode(""" + _ = new Sample().M(); + + class Sample + { + private int _value; + public bool M() => [|_value is 1 || this._value is 2|]; + } + """) + .ShouldFixCodeWith(""" + _ = new Sample().M(); + + class Sample + { + private int _value; + public bool M() => _value is 1 or 2; + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Field_NameAndThisFieldName() + { + await CreateProjectBuilder() + .WithSourceCode(""" + _ = new Sample().M(); + + class Sample + { + private int fieldName; + public bool M() => [|fieldName is 1 || this.fieldName is 2|]; + } + """) + .ShouldFixCodeWith(""" + _ = new Sample().M(); + + class Sample + { + private int fieldName; + public bool M() => fieldName is 1 or 2; + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task LocalVariable_HidesField_DoNotReport() + { + await CreateProjectBuilder() + .WithSourceCode(""" + _ = new Sample().M(); + + class Sample + { + private int value; + + public bool M() + { + var value = 0; + return value is 1 || this.value is 2; + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Property_ExplicitAndImplicitThis() + { + await CreateProjectBuilder() + .WithSourceCode(""" + _ = new Sample().M(); + + class Sample + { + private int Value { get; set; } + public bool M() => [|Value is 1 || this.Value is 2|]; + } + """) + .ShouldFixCodeWith(""" + _ = new Sample().M(); + + class Sample + { + private int Value { get; set; } + public bool M() => Value is 1 or 2; + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Property_DifferentInstances_DoNotReport() + { + await CreateProjectBuilder() + .WithSourceCode(""" + var a = new Sample(); + var b = new Sample(); + _ = a.Value is 1 || b.Value is 2; + + class Sample + { + public int Value { get; set; } + } + """) + .ValidateAsync(); + } + +#if CSHARP12_OR_GREATER + [Fact] + public async Task PrimaryConstructorParameter() + { + await CreateProjectBuilder() + .WithLanguageVersion(Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp12) + .WithSourceCode(""" + _ = new Sample(1).M(); + + class Sample(int value) + { + public bool M() => [|value is 1 || value is 2|]; + } + """) + .ShouldFixCodeWith(""" + _ = new Sample(1).M(); + + class Sample(int value) + { + public bool M() => value is 1 or 2; + } + """) + .ValidateAsync(); + } +#endif + + [Fact] + public void Rule_SeverityAndDefault() + { + var rule = new MergeIsPatternChecksAnalyzer().SupportedDiagnostics[0]; + Assert.Equal(DiagnosticSeverity.Info, rule.DefaultSeverity); + Assert.True(rule.IsEnabledByDefault); + } +}