Do not merge MA0148/MA0149 comparisons when the operand can change between evaluations

docs/Rules/MA0148.md

@@ -13,6 +13,12 @@ value == 0 || value == 1; // not compliant
 value is 0 or 1; // ok
 ````
 
+The code fix only merges the comparisons into a single pattern when the value is a local, a parameter, or a non-volatile field, as merging them evaluates the value only once. Other values, such as method calls or properties, are converted to separate patterns:
+
+````c#
+Next() == 0 || Next() == 1; // fixed as "Next() is 0 || Next() is 1"
+````
+
 Cases that rely on implicit user-defined conversions are ignored because replacing `==` with `is` would be invalid:
 
 ````c#

docs/Rules/MA0149.md

@@ -13,6 +13,12 @@ value != 0 && value != 1; // not compliant
 value is not (0 or 1); // ok
 ````
 
+The code fix only merges the comparisons into a single pattern when the value is a local, a parameter, or a non-volatile field, as merging them evaluates the value only once. Other values, such as method calls or properties, are converted to separate patterns:
+
+````c#
+Next() != 0 && Next() != 1; // fixed as "Next() is not 0 && Next() is not 1"
+````
+
 Cases that rely on implicit user-defined conversions are ignored because replacing `!=` with `is not` would be invalid:
 
 ````c#

src/Meziantou.Analyzer.CodeFixers/Rules/UsePatternMatchingForEqualityComparisonsFixer.cs

@@ -121,7 +121,7 @@ private static ExpressionSyntax RewriteLogicalBinaryExpression(BinaryExpressionS
         {
             if (TryCreateDiscreteComparisonCandidate(term, expectedComparisonOperatorKind, semanticModel, cancellationToken, out var candidate))
             {
-                if (mergeCandidates.Count > 0 && !SyntaxFactory.AreEquivalent(mergeCandidates[0].Expression, candidate.Expression))
+                if (mergeCandidates.Count > 0 && !CanMerge(mergeCandidates[0], candidate))
                 {
                     updatedTerms.Add(CreatePatternExpressionFromCandidates(mergeCandidates, expectedComparisonOperatorKind));
                     mergeCandidates.Clear();
@@ -216,10 +216,40 @@ private static bool TryCreateDiscreteComparisonCandidate(ExpressionSyntax expres
         if (expressionOperation.Syntax is not ExpressionSyntax valueExpression || constantOperation.Syntax is not ExpressionSyntax constantExpression)
             return false;
 
-        candidate = new(valueExpression, ConstantPattern(constantExpression));
+        candidate = new(valueExpression, ConstantPattern(constantExpression), IsStableValue(expressionOperation));
         return true;
     }
 
+    /// <summary>
+    /// Merging the comparisons into a single pattern evaluates the operand once instead of once per comparison,
+    /// so it is only valid when the operands are the same and evaluating them again cannot have a side effect or return another value.
+    /// </summary>
+    private static bool CanMerge(DiscreteComparisonCandidate first, DiscreteComparisonCandidate candidate)
+    {
+        return first.HasStableValue && candidate.HasStableValue && SyntaxFactory.AreEquivalent(first.Expression, candidate.Expression);
+    }
+
+    /// <summary>
+    /// Determines if evaluating the operation has no side effect and returns the same value as the previous evaluation.
+    /// The properties, the indexers and the methods are excluded as they can execute any code.
+    /// </summary>
+    private static bool IsStableValue(IOperation? operation)
+    {
+        if (operation is null)
+            return false;
+
+        if (operation.ConstantValue.HasValue)
+            return true;
+
+        return operation switch
+        {
+            ILocalReferenceOperation or IParameterReferenceOperation or IInstanceReferenceOperation => true,
+            IFieldReferenceOperation fieldReference => !fieldReference.Field.IsVolatile && (fieldReference.Field.IsStatic || IsStableValue(fieldReference.Instance)),
+            IConversionOperation conversion => conversion.OperatorMethod is null && IsStableValue(conversion.Operand),
+            _ => false,
+        };
+    }
+
     private static bool TryCreatePatternExpression(BinaryExpressionSyntax binaryExpression, SemanticModel semanticModel, CancellationToken cancellationToken, out IsPatternExpressionSyntax updatedExpression)
     {
         updatedExpression = null!;
@@ -280,5 +310,5 @@ private static bool TryCreateNullPatternExpression(IBinaryOperation binaryOperat
 
     private static bool IsLogicalBinary(SyntaxKind kind) => kind is SyntaxKind.LogicalAndExpression or SyntaxKind.LogicalOrExpression;
 
-    private readonly record struct DiscreteComparisonCandidate(ExpressionSyntax Expression, PatternSyntax Pattern);
+    private readonly record struct DiscreteComparisonCandidate(ExpressionSyntax Expression, PatternSyntax Pattern, bool HasStableValue);
 }

tests/Meziantou.Analyzer.Test/Rules/UsePatternMatchingForEqualityComparisonsAnalyzerTests.cs

@@ -273,6 +273,104 @@ public Task EqualityComparison_NonContiguousExpressions_DoNotMerge()
         return test.RunAsync();
     }
 
+    [Fact]
+    public Task EqualityComparison_MethodCall_DoNotMerge()
+    {
+        var test = CreateTest();
+        test.TestCode = """
+            _ = {|MA0148:Next() == 0|} || {|MA0148:Next() == 2|};
+
+            static int Next() => 0;
+            """;
+        test.FixedCode = """
+            _ = Next() is 0 || Next() is 2;
+
+            static int Next() => 0;
+            """;
+
+        return test.RunAsync();
+    }
+
+    [Fact]
+    public Task InequalityComparison_MethodCall_DoNotMerge()
+    {
+        var test = CreateTest();
+        test.TestCode = """
+            _ = {|MA0149:Next() != 0|} && {|MA0149:Next() != 2|};
+
+            static int Next() => 0;
+            """;
+        test.FixedCode = """
+            _ = Next() is not 0 && Next() is not 2;
+
+            static int Next() => 0;
+            """;
+
+        return test.RunAsync();
+    }
+
+    [Fact]
+    public Task EqualityComparison_Property_DoNotMerge()
+    {
+        var test = CreateTest();
+        test.TestCode = """
+            _ = {|MA0148:args.Length == 0|} || {|MA0148:args.Length == 1|};
+            """;
+        test.FixedCode = """
+            _ = args.Length is 0 || args.Length is 1;
+            """;
+
+        return test.RunAsync();
+    }
+
+    [Fact]
+    public Task EqualityComparison_Field_MergeConditions()
+    {
+        var test = CreateTest();
+        test.TestCode = """
+            _ = {|MA0148:Sample.Value == 0|} || {|MA0148:Sample.Value == 1|};
+
+            static class Sample
+            {
+                public static int Value;
+            }
+            """;
+        test.FixedCode = """
+            _ = Sample.Value is 0 or 1;
+
+            static class Sample
+            {
+                public static int Value;
+            }
+            """;
+
+        return test.RunAsync();
+    }
+
+    [Fact]
+    public Task EqualityComparison_VolatileField_DoNotMerge()
+    {
+        var test = CreateTest();
+        test.TestCode = """
+            _ = {|MA0148:Sample.Value == 0|} || {|MA0148:Sample.Value == 1|};
+
+            static class Sample
+            {
+                public static volatile int Value;
+            }
+            """;
+        test.FixedCode = """
+            _ = Sample.Value is 0 || Sample.Value is 1;
+
+            static class Sample
+            {
+                public static volatile int Value;
+            }
+            """;
+
+        return test.RunAsync();
+    }
+
     [Fact]
     public Task BatchFix_MergeConditions()
     {