Skip to content

C#: Preserve NullSafe marker through CSharpTemplate.Rewrite()#7140

Merged
knutwannheden merged 2 commits intomainfrom
csharp-preserve-nullsafe
Mar 26, 2026
Merged

C#: Preserve NullSafe marker through CSharpTemplate.Rewrite()#7140
knutwannheden merged 2 commits intomainfrom
csharp-preserve-nullsafe

Conversation

@knutwannheden
Copy link
Contributor

Motivation

When CSharpTemplate.Rewrite() transforms a chained method call that has a ?. (NullSafe marker) on an intermediate call, the marker was lost in the output because the template builds a fresh tree. For example, dict["key"]?.Where(x => x > 0).First() rewritten to dict["key"]?.First(x => x > 0) would lose the ?., producing dict["key"].First(x => x > 0) instead.

Two root causes: (1) the NullSafe marker lived on the Name Identifier rather than the MethodInvocation/FieldAccess node, making it invisible to structural comparison and unintuitive for recipe authors; (2) the pattern matcher and template engine had no mechanism to preserve it.

Summary

  • Move NullSafe marker from mi.Name.Markers to mi.Markers (and fa.Markers for FieldAccess) across parser and printer — this is the semantically correct location since ?. describes the access operation, not the name
  • Asymmetric pattern matching — a pattern without ?. matches both . and ?. access (lenient), but a pattern with ?. only matches ?. (strict). This avoids forcing recipe authors to duplicate patterns
  • NullSafe preservation in templates — when a capture was the select of a null-conditional MI/FA, the template engine transfers the NullSafe marker to the output MI/FA

Test plan

  • 1205 tree round-trip tests pass (parser/printer refactor is correct)
  • 176 pattern match + template tests pass (no regressions)
  • New test: PreservesNullConditionalInRewrite{x}.Where({pred}).First(){x}.First({pred}) preserves ?.
  • New test: PreservesNullConditionalOnFieldAccess{x}.Length{x}.Count preserves ?.
  • New test: NullConditionalNotAddedWhenOriginalHasNone — no spurious ?. added
  • Updated test: RegularDotPatternMatchesNullConditionalAccess — pattern without ?. now matches ?.
  • Existing test: NullConditionalPatternDoesNotMatchRegularDotAccess — pattern with ?. still rejects .
  • Full suite: 1726/1726 tests pass

Move the NullSafe marker from the Name Identifier to the
MethodInvocation/FieldAccess node where it semantically belongs,
and teach the pattern matcher and template engine to preserve it
through rewrites.

- Parser: place NullSafe on MI/FA Markers instead of Name Markers
- Printer: check mi.Markers instead of mi.Name.Markers
- Pattern matching: asymmetric NullSafe check — pattern without ?.
  matches both . and ?. access, but pattern with ?. only matches ?.
- Template substitution: when a capture was the select of a NullSafe
  MI/FA, transfer the marker to the template output MI/FA
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Mar 25, 2026
@knutwannheden knutwannheden changed the title C#: Preserve NullSafe marker through CSharpTemplate.Rewrite() C#: Preserve NullSafe marker through CSharpTemplate.Rewrite() Mar 25, 2026
@knutwannheden knutwannheden marked this pull request as ready for review March 25, 2026 12:47
… preservation

Move the NullSafe marker for ?[ from ArrayDimension.Markers to
ArrayAccess.Markers for consistency with MethodInvocation and
FieldAccess. Split the combined space into Dimension prefix (space
before ?) and NullSafe.DotPrefix (space between ? and [).

Add NullSafe recording and transfer for ArrayAccess in the pattern
matcher and template engine.
@knutwannheden knutwannheden force-pushed the csharp-preserve-nullsafe branch from 2e6e4ae to 86736ab Compare March 25, 2026 16:19
@knutwannheden knutwannheden merged commit 004769a into main Mar 26, 2026
1 check passed
@knutwannheden knutwannheden deleted the csharp-preserve-nullsafe branch March 26, 2026 08:53
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant