Skip to content

Commit 2e6e4ae

Browse files
knutwannhedenknutwannheden
committed
C#: Move NullSafe from ArrayDimension to ArrayAccess and add template 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.
1 parent 8a1416a commit 2e6e4ae

File tree

5 files changed

+107
-22
lines changed

5 files changed

+107
-22
lines changed

‎rewrite-csharp/csharp/OpenRewrite/CSharp/CSharpParser.cs‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6595,19 +6595,21 @@ private ArrayAccess ParseNullConditionalElementAccess(Space prefix, Expression t
65956595
var closeBracketSpace = ExtractSpaceBefore(argList.CloseBracketToken);
65966596
_cursor = argList.CloseBracketToken.Span.End;
65976597

6598-
// Combine operatorSpace (before ?) with bracketPrefix (before [) into dimension prefix
6599-
// The NullSafe marker on dimension tells printer to print ?[
6600-
var combinedPrefix = CombineSpaces(operatorSpace, bracketPrefix);
6598+
// operatorSpace = space before ?, bracketPrefix = space between ? and [
6599+
// NullSafe.DotPrefix stores space between ? and [ (like space between ? and . in MI)
6600+
var singleDimNs = bracketPrefix.IsEmpty
6601+
? NullSafe.Instance
6602+
: new NullSafe(Guid.NewGuid(), bracketPrefix);
66016603

66026604
return new ArrayAccess(
66036605
Guid.NewGuid(),
66046606
prefix,
6605-
Markers.Empty,
6607+
Markers.Build([singleDimNs]),
66066608
target,
66076609
new ArrayDimension(
66086610
Guid.NewGuid(),
6609-
combinedPrefix,
6610-
Markers.Build([NullSafe.Instance]),
6611+
operatorSpace,
6612+
Markers.Empty,
66116613
new JRightPadded<Expression>(index, closeBracketSpace, Markers.Empty)
66126614
),
66136615
null
@@ -6633,18 +6635,20 @@ private ArrayAccess ParseNullConditionalElementAccess(Space prefix, Expression t
66336635
var currentSpaceBeforeComma = ExtractSpaceBefore(firstSeparator);
66346636
_cursor = firstSeparator.Span.End;
66356637

6636-
var combinedFirstPrefix = CombineSpaces(operatorSpace, firstBracketPrefix);
6638+
var multiDimNs = firstBracketPrefix.IsEmpty
6639+
? NullSafe.Instance
6640+
: new NullSafe(Guid.NewGuid(), firstBracketPrefix);
66376641

6638-
// Innermost ArrayAccess with NullSafe marker on dimension
6642+
// Innermost ArrayAccess with NullSafe marker on the ArrayAccess
66396643
ArrayAccess current = new ArrayAccess(
66406644
Guid.NewGuid(),
66416645
prefix,
6642-
Markers.Empty,
6646+
Markers.Build([multiDimNs]),
66436647
target,
66446648
new ArrayDimension(
66456649
Guid.NewGuid(),
6646-
combinedFirstPrefix,
6647-
Markers.Build([NullSafe.Instance]),
6650+
operatorSpace,
6651+
Markers.Empty,
66486652
new JRightPadded<Expression>(firstIndexExpr, Space.Empty, Markers.Empty)
66496653
),
66506654
null

‎rewrite-csharp/csharp/OpenRewrite/CSharp/CSharpPrinter.cs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,22 @@ public override J VisitArrayAccess(ArrayAccess arrayAccess, PrintOutputCapture<P
329329
// Normal single-dimension access
330330
BeforeSyntax(arrayAccess, p);
331331
Visit(arrayAccess.Indexed, p);
332-
VisitArrayDimension(arrayAccess.Dimension, p);
332+
var nullSafe = arrayAccess.Markers.FindFirst<NullSafe>();
333+
if (nullSafe != null)
334+
{
335+
// Dimension prefix holds space before ?, NullSafe.DotPrefix holds space between ? and [
336+
VisitSpace(arrayAccess.Dimension.Prefix, p);
337+
p.Append('?');
338+
VisitSpace(nullSafe.DotPrefix, p);
339+
p.Append('[');
340+
Visit(arrayAccess.Dimension.Index.Element, p);
341+
VisitSpace(arrayAccess.Dimension.Index.After, p);
342+
p.Append(']');
343+
}
344+
else
345+
{
346+
VisitArrayDimension(arrayAccess.Dimension, p);
347+
}
333348
}
334349
AfterSyntax(arrayAccess, p);
335350
return arrayAccess;
@@ -356,9 +371,13 @@ private void PrintArrayAccessWithoutClosingBracket(ArrayAccess aa, PrintOutputCa
356371
VisitSpace(aa.Prefix, p);
357372
Visit(aa.Indexed, p);
358373
VisitSpace(aa.Dimension.Prefix, p);
359-
// Check for NullSafe marker - if present, print ?[ instead of [
360-
var isNullSafe = aa.Dimension.Markers.FindFirst<NullSafe>() != null;
361-
p.Append(isNullSafe ? "?[" : "[");
374+
var nullSafe = aa.Markers.FindFirst<NullSafe>();
375+
if (nullSafe != null)
376+
{
377+
p.Append('?');
378+
VisitSpace(nullSafe.DotPrefix, p);
379+
}
380+
p.Append('[');
362381
Visit(aa.Dimension.Index.Element, p);
363382
VisitSpace(aa.Dimension.Index.After, p);
364383
// Don't print ] - parent will
@@ -368,9 +387,7 @@ private void PrintArrayAccessWithoutClosingBracket(ArrayAccess aa, PrintOutputCa
368387
public override J VisitArrayDimension(ArrayDimension dimension, PrintOutputCapture<P> p)
369388
{
370389
BeforeSyntax(dimension, p);
371-
// Check for NullSafe marker - if present, print ?[ instead of [
372-
var isNullSafe = dimension.Markers.FindFirst<NullSafe>() != null;
373-
p.Append(isNullSafe ? "?[" : "[");
390+
p.Append('[');
374391
Visit(dimension.Index.Element, p);
375392
VisitSpace(dimension.Index.After, p);
376393
p.Append(']');

‎rewrite-csharp/csharp/OpenRewrite/CSharp/Template/PatternMatchingComparator.cs‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,11 @@ private bool MatchNode(J pattern, J candidate, Cursor cursor)
118118
}
119119

120120
/// <summary>
121-
/// After a successful match of a MethodInvocation or FieldAccess, check if the
122-
/// candidate has a NullSafe marker that the pattern doesn't have. If so, find the
123-
/// capture placeholder in the Select position and record the NullSafe association
124-
/// so the template engine can preserve <c>?.</c> through rewrites.
121+
/// After a successful match of a MethodInvocation, FieldAccess, or ArrayAccess,
122+
/// check if the candidate has a NullSafe marker that the pattern doesn't have.
123+
/// If so, find the capture placeholder in the Select/Target/Indexed position and
124+
/// record the NullSafe association so the template engine can preserve <c>?.</c>
125+
/// and <c>?[</c> through rewrites.
125126
/// </summary>
126127
private void RecordNullSafeForCaptures(J pattern, J candidate)
127128
{
@@ -143,6 +144,15 @@ private void RecordNullSafeForCaptures(J pattern, J candidate)
143144
_nullSafeBindings[captureName] = candNullSafe;
144145
}
145146
}
147+
else if (pattern is ArrayAccess patAa && candidate is ArrayAccess candAa)
148+
{
149+
var candNullSafe = candAa.Markers.FindFirst<NullSafe>();
150+
if (candNullSafe != null && patAa.Markers.FindFirst<NullSafe>() == null)
151+
{
152+
if (FindTargetCaptureName(patAa.Indexed) is { } captureName)
153+
_nullSafeBindings[captureName] = candNullSafe;
154+
}
155+
}
146156
}
147157

148158
/// <summary>

‎rewrite-csharp/csharp/OpenRewrite/CSharp/Template/TemplateEngine.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,26 @@ public override J VisitFieldAccess(FieldAccess fieldAccess, int p)
785785
return fieldAccess;
786786
}
787787

788+
public override J VisitArrayAccess(ArrayAccess arrayAccess, int p)
789+
{
790+
// Check if the indexed expression is a capture placeholder BEFORE substitution
791+
var indexedCaptureName = arrayAccess.Indexed is Identifier indexedId
792+
? Placeholder.FromPlaceholder(indexedId.SimpleName)
793+
: null;
794+
795+
arrayAccess = (ArrayAccess)base.VisitArrayAccess(arrayAccess, p);
796+
797+
// Transfer NullSafe from matched tree when the capture was a null-conditional indexed expr
798+
if (indexedCaptureName != null && arrayAccess.Markers.FindFirst<NullSafe>() == null)
799+
{
800+
var nullSafe = _values.GetNullSafe(indexedCaptureName);
801+
if (nullSafe != null)
802+
arrayAccess = arrayAccess.WithMarkers(arrayAccess.Markers.Add(nullSafe));
803+
}
804+
805+
return arrayAccess;
806+
}
807+
788808
/// <summary>
789809
/// If any argument is a placeholder identifier bound to a variadic capture (list),
790810
/// expand it into the argument list.

‎rewrite-csharp/csharp/OpenRewrite/Tests/Template/RewriteRuleTests.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,40 @@ void M(string? s)
587587
);
588588
}
589589

590+
[Fact]
591+
public void PreservesNullConditionalOnElementAccess()
592+
{
593+
var x = Capture.Expression();
594+
var i = Capture.Expression();
595+
596+
RewriteRun(
597+
spec => spec.SetRecipe(new RewriteRecipe(
598+
CSharpTemplate.Rewrite(
599+
CSharpPattern.Expression($"{x}[{i}]"),
600+
CSharpTemplate.Expression($"{x}[{i}]")))),
601+
CSharp(
602+
"""
603+
class Test
604+
{
605+
void M(int[]? arr)
606+
{
607+
var n = arr?[0];
608+
}
609+
}
610+
""",
611+
"""
612+
class Test
613+
{
614+
void M(int[]? arr)
615+
{
616+
var n = arr?[0];
617+
}
618+
}
619+
"""
620+
)
621+
);
622+
}
623+
590624
[Fact]
591625
public void NullConditionalNotAddedWhenOriginalHasNone()
592626
{

0 commit comments

Comments
 (0)