C#: Auto-parenthesize IsPattern with or/and combinators in binary expressions (#7196)

When CSharpTemplate substitutes an IsPattern containing an `or` or `and`
pattern combinator into a Binary(&&) expression, the result needs
parentheses to preserve operator precedence. Without them, C# parses
`x is A or B && y` as `x is (A or (B && y))` instead of
`(x is A or B) && y`.

- Make GetPrecedence context-aware for IsPattern: `or` combinator lowers
  effective precedence to 1 (same as ||), `and` to 2 (same as &&)
- Fix NeedsParenthesesInContext to not wrap CsBinary Or/And pattern
  combinators inside IsPattern — they are legitimate patterns, not
  boolean expressions
- Add ParenthesizeDeep public entry point for recursive parenthesization
- Clarify CSharpTemplate.Apply comment: inner precedence is handled by
  ApplySubstitutions, MaybeParenthesize only checks the graft site

Author: knutwannheden

rewrite-csharp/csharp/OpenRewrite/CSharp/CSharpParenthesizeVisitor.cs

@@ -53,6 +53,16 @@ or TypeCast or CsBinary or CsUnary or IsPattern or RangeExpression
 
         return newTree;
     }
+
+    /// <summary>
+    /// Recursively walks <paramref name="tree"/> and adds parentheses wherever operator
+    /// precedence requires them within the tree itself (not considering the graft site).
+    /// Call this on template results before <see cref="MaybeParenthesize"/>.
+    /// </summary>
+    public static J ParenthesizeDeep(J tree)
+    {
+        return new CSharpParenthesizeVisitor<int>(true).Visit(tree, 0)!;
+    }
 }
 
 /// <summary>
@@ -239,6 +249,11 @@ private bool NeedsParenthesesInContext(Expression expr)
 
         if (parent is IsPattern)
         {
+            // CsBinary with Or/And inside IsPattern is a pattern combinator, not a
+            // boolean expression — it must NOT be wrapped in parentheses.
+            if (expr is CsBinary csb && csb.Operator.Element is CsBinary.OperatorType.Or or CsBinary.OperatorType.And)
+                return false;
+
             return expr is Binary or CsBinary or Ternary or Assignment or AssignmentOperation or Lambda;
         }
 

rewrite-csharp/csharp/OpenRewrite/CSharp/CSharpPrecedences.cs

@@ -48,7 +48,7 @@ internal static class CSharpPrecedences
         Unary => 13,
         CsUnary => 13,
         TypeCast => 13,
-        IsPattern => 7,
+        IsPattern ip => GetIsPatternPrecedence(ip),
         RangeExpression => 12,
         SwitchExpression => 11,
         WithExpression => 11,
@@ -80,6 +80,25 @@ public static Parentheses<Expression> Parenthesize(Expression expr)
             new JRightPadded<Expression>(J.SetPrefix(expr, Space.Empty), Space.Empty, Markers.Empty));
     }
 
+    /// <summary>
+    /// IsPattern with pattern combinators (or/and) has lower effective precedence
+    /// than a plain is-type check, because the combinator keywords extend the pattern
+    /// in a way that can be ambiguous when nested inside binary expressions like &amp;&amp;.
+    /// </summary>
+    private static int GetIsPatternPrecedence(IsPattern ip)
+    {
+        if (ip.Pattern.Element is CsBinary csb)
+        {
+            return csb.Operator.Element switch
+            {
+                CsBinary.OperatorType.Or => 1,  // same as ||
+                CsBinary.OperatorType.And => 2, // same as &&
+                _ => 7
+            };
+        }
+        return 7;
+    }
+
     public static bool IsAssociative(Expression expr) => expr switch
     {
         Binary b => b.Operator.Element is

rewrite-csharp/csharp/OpenRewrite/CSharp/Template/CSharpTemplate.cs

@@ -204,7 +204,9 @@ public J GetTree()
             tree = TemplateEngine.ApplySubstitutions(tree, values);
         }
 
-        // Phase 1.5: auto-parenthesization after substitution
+        // Phase 1.5: check if the result needs wrapping when grafted into the cursor position.
+        // Inner precedence is already handled by ApplySubstitutions (which runs a recursive
+        // CSharpParenthesizeVisitor after substitution); this only checks the root vs. cursor parent.
         if (tree is Expression expr && cursorJ != null)
         {
             tree = CSharpParenthesizeVisitor.MaybeParenthesize(expr, cursor);

rewrite-csharp/csharp/OpenRewrite/Tests/CSharp/CSharpParenthesizeVisitorTests.cs

@@ -156,4 +156,60 @@ public void MaybeParenthesize_TernaryInsideTernary_AddsParens()
 
         Assert.IsType<Parentheses<Expression>>(result);
     }
+
+    [Fact]
+    public void MaybeParenthesize_IsPatternWithOrCombinatorInsideLogicalAnd_AddsParens()
+    {
+        // placeholder && flag => (m is A or B) && flag — or combinator needs parens inside &&
+        var placeholder = MakeId("placeholder");
+        var parent = MakeBinary(Binary.OperatorType.And, placeholder, MakeId("flag"));
+        var cursor = new Cursor(new Cursor(null, "root"), parent);
+        cursor = new Cursor(cursor, placeholder);
+
+        var orPattern = MakeCsBinary(CsBinary.OperatorType.Or, MakeId("A"), MakeId("B"));
+        var newExpr = MakeIsPattern(MakeId("m"), orPattern);
+        var result = CSharpParenthesizeVisitor.MaybeParenthesize(newExpr, cursor);
+
+        Assert.IsType<Parentheses<Expression>>(result);
+    }
+
+    [Fact]
+    public void MaybeParenthesize_IsPatternWithoutCombinatorInsideLogicalAnd_NoParens()
+    {
+        // placeholder && flag => m is A && flag — simple is pattern doesn't need parens inside &&
+        var placeholder = MakeId("placeholder");
+        var parent = MakeBinary(Binary.OperatorType.And, placeholder, MakeId("flag"));
+        var cursor = new Cursor(new Cursor(null, "root"), parent);
+        cursor = new Cursor(cursor, placeholder);
+
+        var newExpr = MakeIsPattern(MakeId("m"), MakeConstantPattern(MakeId("A")));
+        var result = CSharpParenthesizeVisitor.MaybeParenthesize(newExpr, cursor);
+
+        Assert.IsNotType<Parentheses<Expression>>(result);
+    }
+
+    [Fact]
+    public void ParenthesizeDeep_IsPatternWithOrInsideBinaryAnd_AddsParens()
+    {
+        // (m is A or B) && flag — recursive walk should parenthesize inner IsPattern
+        var orPattern = MakeCsBinary(CsBinary.OperatorType.Or, MakeId("A"), MakeId("B"));
+        var isExpr = MakeIsPattern(MakeId("m"), orPattern);
+        var tree = MakeBinary(Binary.OperatorType.And, isExpr, MakeId("flag"));
+
+        var result = (Binary)CSharpParenthesizeVisitor.ParenthesizeDeep(tree);
+
+        Assert.IsType<Parentheses<Expression>>(result.Left);
+    }
+
+    [Fact]
+    public void ParenthesizeDeep_SimpleIsPatternInsideBinaryAnd_NoParens()
+    {
+        // m is A && flag — no inner parens needed
+        var isExpr = MakeIsPattern(MakeId("m"), MakeConstantPattern(MakeId("A")));
+        var tree = MakeBinary(Binary.OperatorType.And, isExpr, MakeId("flag"));
+
+        var result = (Binary)CSharpParenthesizeVisitor.ParenthesizeDeep(tree);
+
+        Assert.IsType<IsPattern>(result.Left);
+    }
 }

rewrite-csharp/csharp/OpenRewrite/Tests/CSharp/TestHelpers.cs

@@ -74,4 +74,11 @@ public static Assignment MakeAssignment(Expression variable, Expression value) =
     public static Parentheses<Expression> MakeParens(Expression expr) =>
         new(Guid.NewGuid(), Space.Empty, Markers.Empty,
             new JRightPadded<Expression>(expr, Space.Empty, Markers.Empty));
+
+    public static IsPattern MakeIsPattern(Expression expression, Pattern pattern) =>
+        new(Guid.NewGuid(), Space.Empty, Markers.Empty, expression,
+            new JLeftPadded<Pattern>(Space.Empty, pattern));
+
+    public static ConstantPattern MakeConstantPattern(Expression value) =>
+        new(Guid.NewGuid(), Space.Empty, Markers.Empty, value);
 }