Skip to content

C#: Auto-parenthesize IsPattern with or/and combinators#7196

Merged
knutwannheden merged 1 commit intomainfrom
csharptemplate-should-auto-parenthesize-ispattern-with-or-combinator
Mar 29, 2026
Merged

C#: Auto-parenthesize IsPattern with or/and combinators#7196
knutwannheden merged 1 commit intomainfrom
csharptemplate-should-auto-parenthesize-ispattern-with-or-combinator

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

Motivation

MergeIfWithParentIf merges nested if-statements using CSharpTemplate.Expression($"{_left} && {_right}"). When the outer condition is an IsPattern with an or combinator, the merged result changes semantics because C# && interacts with the pattern combinator: x is A or B && y parses as x is (A or (B && y)) instead of the intended (x is A or B) && y. This also causes a downstream NullReferenceException in CSharpPrinter.VisitIsPattern.

Examples

Before (wrong semantics):

if (existsMode is FtpRemoteExists.Resume or FtpRemoteExists.AddToEnd && fileExists)

After (correct):

if ((existsMode is FtpRemoteExists.Resume or FtpRemoteExists.AddToEnd) && fileExists)

Summary

  • CSharpPrecedences.GetPrecedence is now context-aware for IsPattern: when the pattern contains an or combinator, effective precedence is lowered to 1 (same as ||); and combinator lowers it to 2 (same as &&)
  • CSharpParenthesizeVisitor.NeedsParenthesesInContext no longer wraps CsBinary Or/And pattern combinators inside IsPattern — these are legitimate patterns, not boolean expressions
  • Added ParenthesizeDeep public entry point for recursive parenthesization
  • Clarified CSharpTemplate.Apply comment: inner precedence is handled by ApplySubstitutions, MaybeParenthesize only checks the graft site

Test plan

  • New unit test: IsPattern with or combinator inside Binary(&&) gets parenthesized via MaybeParenthesize
  • New unit test: plain IsPattern (no combinator) inside Binary(&&) is NOT parenthesized
  • New unit test: ParenthesizeDeep recursive walk parenthesizes inner IsPattern with or inside Binary(&&)
  • New unit test: ParenthesizeDeep does NOT parenthesize simple IsPattern inside Binary(&&)
  • All 59 existing precedence and parenthesization tests continue to pass

…ressions

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
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Mar 29, 2026
@knutwannheden knutwannheden merged commit e21cc35 into main Mar 29, 2026
1 check passed
@knutwannheden knutwannheden deleted the csharptemplate-should-auto-parenthesize-ispattern-with-or-combinator branch March 29, 2026 16:28
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant