Skip to content

Commit 27c556e

Browse files
knutwannhedenknutwannheden
authored
C#: Replace ExceptionFilteredTry with WhenClause on catch variable initializer (#7171)
The ExceptionFilteredTry wrapper stored when-filters in a parallel list with null entries positionally aligned to catch clauses. This violated the convention of no null elements in LST lists and caused misalignment when catch clauses were added or removed. Replace with a Cs.WhenClause Expression stored directly on the NamedVariable initializer of each catch parameter. For catch clauses without a named variable, a NamedVariable with an empty name is created to hold the WhenClause. This eliminates the wrapper, restores normal J.Try data flow, and makes each catch clause self-contained.
1 parent 215d13c commit 27c556e

12 files changed

Lines changed: 186 additions & 222 deletions

File tree

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

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4169,14 +4169,12 @@ public override J VisitTryStatement(TryStatementSyntax node)
41694169

41704170
// Parse catch clauses
41714171
var catches = new List<Try.Catch>();
4172-
var catchFilters = new List<JLeftPadded<ControlParentheses<Expression>>?>();
4173-
bool hasAnyFilter = false;
41744172
foreach (var catchClause in node.Catches)
41754173
{
41764174
var catchPrefix = ExtractSpaceBefore(catchClause.CatchKeyword);
41774175
_cursor = catchClause.CatchKeyword.Span.End;
41784176

4179-
ControlParentheses<VariableDeclarations>? parameter = null;
4177+
ControlParentheses<VariableDeclarations> parameter;
41804178
if (catchClause.Declaration != null)
41814179
{
41824180
var parenPrefix = ExtractSpaceBefore(catchClause.Declaration.OpenParenToken);
@@ -4193,19 +4191,38 @@ public override J VisitTryStatement(TryStatementSyntax node)
41934191
exName = new Identifier(Guid.NewGuid(), exNamePrefix, Markers.Empty, [], catchClause.Declaration.Identifier.Text, catchVarType?.Type, catchVarType);
41944192
}
41954193

4194+
var closeParenPrefix = ExtractSpaceBefore(catchClause.Declaration.CloseParenToken);
4195+
_cursor = catchClause.Declaration.CloseParenToken.Span.End;
4196+
4197+
// Parse when clause if present — stored on the NamedVariable initializer
4198+
JLeftPadded<Expression>? whenInitializer = catchClause.Filter != null
4199+
? ParseWhenClause(catchClause.Filter)
4200+
: null;
4201+
4202+
IList<JRightPadded<NamedVariable>> variables;
4203+
if (exName != null)
4204+
{
4205+
variables = [new JRightPadded<NamedVariable>(new NamedVariable(Guid.NewGuid(), Space.Empty, Markers.Empty, exName, [], whenInitializer, _typeMapping?.VariableType(catchClause.Declaration!)), Space.Empty, Markers.Empty)];
4206+
}
4207+
else if (whenInitializer != null)
4208+
{
4209+
// Type-only catch with when clause: create a NamedVariable with empty name to hold the when clause
4210+
var emptyName = new Identifier(Guid.NewGuid(), Space.Empty, Markers.Empty, [], "", null, null);
4211+
variables = [new JRightPadded<NamedVariable>(new NamedVariable(Guid.NewGuid(), Space.Empty, Markers.Empty, emptyName, [], whenInitializer, null), Space.Empty, Markers.Empty)];
4212+
}
4213+
else
4214+
{
4215+
variables = [];
4216+
}
4217+
41964218
var varDecl = new VariableDeclarations(
41974219
Guid.NewGuid(),
41984220
Space.Empty,
41994221
Markers.Empty,
42004222
[], [], typeExpr, null, [],
4201-
exName != null
4202-
? [new JRightPadded<NamedVariable>(new NamedVariable(Guid.NewGuid(), Space.Empty, Markers.Empty, exName, [], null, _typeMapping?.VariableType(catchClause.Declaration!)), Space.Empty, Markers.Empty)]
4203-
: []
4223+
variables
42044224
);
42054225

4206-
var closeParenPrefix = ExtractSpaceBefore(catchClause.Declaration.CloseParenToken);
4207-
_cursor = catchClause.Declaration.CloseParenToken.Span.End;
4208-
42094226
parameter = new ControlParentheses<VariableDeclarations>(
42104227
Guid.NewGuid(),
42114228
parenPrefix,
@@ -4216,8 +4233,24 @@ public override J VisitTryStatement(TryStatementSyntax node)
42164233
else
42174234
{
42184235
// Catch-all without declaration
4236+
// Parse when clause if present
4237+
JLeftPadded<Expression>? whenInitializer = catchClause.Filter != null
4238+
? ParseWhenClause(catchClause.Filter)
4239+
: null;
4240+
4241+
IList<JRightPadded<NamedVariable>> variables;
4242+
if (whenInitializer != null)
4243+
{
4244+
// Bare catch with when clause: create a NamedVariable with empty name to hold the when clause
4245+
var emptyName = new Identifier(Guid.NewGuid(), Space.Empty, Markers.Empty, [], "", null, null);
4246+
variables = [new JRightPadded<NamedVariable>(new NamedVariable(Guid.NewGuid(), Space.Empty, Markers.Empty, emptyName, [], whenInitializer, null), Space.Empty, Markers.Empty)];
4247+
}
4248+
else
4249+
{
4250+
variables = [];
4251+
}
42194252
var emptyVarDecl = new VariableDeclarations(
4220-
Guid.NewGuid(), Space.Empty, Markers.Empty, [], [], null, null, [], []
4253+
Guid.NewGuid(), Space.Empty, Markers.Empty, [], [], null, null, [], variables
42214254
);
42224255
parameter = new ControlParentheses<VariableDeclarations>(
42234256
Guid.NewGuid(),
@@ -4227,25 +4260,6 @@ public override J VisitTryStatement(TryStatementSyntax node)
42274260
);
42284261
}
42294262

4230-
// Parse catch filter (when clause) if present
4231-
JLeftPadded<ControlParentheses<Expression>>? catchFilter = null;
4232-
if (catchClause.Filter != null)
4233-
{
4234-
var whenPrefix = ExtractSpaceBefore(catchClause.Filter.WhenKeyword);
4235-
_cursor = catchClause.Filter.WhenKeyword.Span.End;
4236-
var openParenPrefix = ExtractSpaceBefore(catchClause.Filter.OpenParenToken);
4237-
_cursor = catchClause.Filter.OpenParenToken.Span.End;
4238-
var filterExpr = (Expression)Visit(catchClause.Filter.FilterExpression)!;
4239-
var closeParenPrefix = ExtractSpaceBefore(catchClause.Filter.CloseParenToken);
4240-
_cursor = catchClause.Filter.CloseParenToken.Span.End;
4241-
var controlParens = new ControlParentheses<Expression>(
4242-
Guid.NewGuid(), openParenPrefix, Markers.Empty,
4243-
new JRightPadded<Expression>(filterExpr, closeParenPrefix, Markers.Empty));
4244-
catchFilter = new JLeftPadded<ControlParentheses<Expression>>(whenPrefix, controlParens);
4245-
hasAnyFilter = true;
4246-
}
4247-
catchFilters.Add(catchFilter);
4248-
42494263
var catchBody = (Block)VisitBlock(catchClause.Block);
42504264

42514265
catches.Add(new Try.Catch(
@@ -4267,28 +4281,31 @@ public override J VisitTryStatement(TryStatementSyntax node)
42674281
finallyBlock = new JLeftPadded<Block>(finallyPrefix, finallyBody);
42684282
}
42694283

4270-
var tryStmt = new Try(
4284+
return new Try(
42714285
Guid.NewGuid(),
4272-
hasAnyFilter ? Space.Empty : prefix,
4286+
prefix,
42734287
Markers.Empty,
42744288
null,
42754289
body,
42764290
catches,
42774291
finallyBlock
42784292
);
4293+
}
42794294

4280-
if (hasAnyFilter)
4281-
{
4282-
return new ExceptionFilteredTry(
4283-
Guid.NewGuid(),
4284-
prefix,
4285-
Markers.Empty,
4286-
tryStmt,
4287-
catchFilters
4288-
);
4289-
}
4290-
4291-
return tryStmt;
4295+
private JLeftPadded<Expression> ParseWhenClause(CatchFilterClauseSyntax filter)
4296+
{
4297+
var whenPrefix = ExtractSpaceBefore(filter.WhenKeyword);
4298+
_cursor = filter.WhenKeyword.Span.End;
4299+
var openParenPrefix = ExtractSpaceBefore(filter.OpenParenToken);
4300+
_cursor = filter.OpenParenToken.Span.End;
4301+
var filterExpr = (Expression)Visit(filter.FilterExpression)!;
4302+
var closeParenPrefix = ExtractSpaceBefore(filter.CloseParenToken);
4303+
_cursor = filter.CloseParenToken.Span.End;
4304+
var controlParens = new ControlParentheses<Expression>(
4305+
Guid.NewGuid(), openParenPrefix, Markers.Empty,
4306+
new JRightPadded<Expression>(filterExpr, closeParenPrefix, Markers.Empty));
4307+
var whenClause = new WhenClause(Guid.NewGuid(), Space.Empty, Markers.Empty, controlParens);
4308+
return new JLeftPadded<Expression>(whenPrefix, whenClause);
42924309
}
42934310

42944311
public override J VisitThrowStatement(ThrowStatementSyntax node)

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

Lines changed: 50 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1773,19 +1773,7 @@ public override J VisitTry(Try tr, PrintOutputCapture<P> p)
17731773

17741774
foreach (var c in tr.Catches)
17751775
{
1776-
VisitSpace(c.Prefix, p);
1777-
p.Append("catch");
1778-
1779-
if (c.Parameter.Tree.Element.TypeExpression != null || c.Parameter.Tree.Element.Variables.Count > 0)
1780-
{
1781-
VisitSpace(c.Parameter.Prefix, p);
1782-
p.Append('(');
1783-
VisitVariableDeclarationsWithoutSemicolon(c.Parameter.Tree.Element, p);
1784-
VisitSpace(c.Parameter.Tree.After, p);
1785-
p.Append(')');
1786-
}
1787-
1788-
VisitBlock(c.Body, p);
1776+
VisitCatchClause(c, p);
17891777
}
17901778

17911779
if (tr.Finally != null)
@@ -3524,60 +3512,70 @@ public override J VisitTypeWithArguments(TypeWithArguments twa, PrintOutputCaptu
35243512
return twa;
35253513
}
35263514

3527-
public override J VisitExceptionFilteredTry(ExceptionFilteredTry eft, PrintOutputCapture<P> p)
3515+
private void VisitCatchClause(Try.Catch catchClause, PrintOutputCapture<P> p)
35283516
{
3529-
BeforeSyntax(eft, p);
3530-
3531-
// Print 'try' keyword and body via the inner Try's prefix
3532-
var innerTry = eft.Try;
3533-
VisitSpace(innerTry.Prefix, p);
3534-
p.Append("try");
3517+
VisitSpace(catchClause.Prefix, p);
3518+
p.Append("catch");
35353519

3536-
Visit(innerTry.Body, p);
3520+
var varDecl = catchClause.Parameter.Tree.Element;
3521+
bool hasDeclaration = varDecl.TypeExpression != null;
35373522

3538-
// Print catch clauses with filters
3539-
for (int i = 0; i < innerTry.Catches.Count; i++)
3523+
// Find the WhenClause if present (stored on the NamedVariable initializer)
3524+
JLeftPadded<Expression>? whenInitializer = null;
3525+
if (varDecl.Variables.Count > 0)
35403526
{
3541-
var catchClause = innerTry.Catches[i];
3542-
VisitSpace(catchClause.Prefix, p);
3543-
p.Append("catch");
3544-
3545-
// Print catch parameter manually (ControlParentheses<VariableDeclarations>
3546-
// is not handled by VisitControlParentheses which only takes Expression)
3547-
if (catchClause.Parameter.Tree.Element.TypeExpression != null || catchClause.Parameter.Tree.Element.Variables.Count > 0)
3527+
var namedVar = varDecl.Variables[0].Element;
3528+
if (namedVar.Initializer?.Element is WhenClause)
35483529
{
3549-
VisitSpace(catchClause.Parameter.Prefix, p);
3550-
p.Append('(');
3551-
VisitVariableDeclarationsWithoutSemicolon(catchClause.Parameter.Tree.Element, p);
3552-
VisitSpace(catchClause.Parameter.Tree.After, p);
3553-
p.Append(')');
3530+
whenInitializer = namedVar.Initializer;
35543531
}
3532+
}
3533+
3534+
if (hasDeclaration)
3535+
{
3536+
VisitSpace(catchClause.Parameter.Prefix, p);
3537+
p.Append('(');
35553538

3556-
// Print filter if present: when (expr)
3557-
if (i < eft.CatchFilters.Count && eft.CatchFilters[i] is { } filter)
3539+
// Print type
3540+
Visit(varDecl.TypeExpression, p);
3541+
3542+
// Print variable name if present (skip empty names used as when-clause holders)
3543+
if (varDecl.Variables.Count > 0)
35583544
{
3559-
VisitSpace(filter.Before, p);
3560-
p.Append("when");
3561-
VisitSpace(filter.Element.Prefix, p);
3562-
p.Append('(');
3563-
Visit(filter.Element.Tree.Element, p);
3564-
VisitSpace(filter.Element.Tree.After, p);
3565-
p.Append(')');
3545+
var namedVar = varDecl.Variables[0].Element;
3546+
if (namedVar.Name.SimpleName.Length > 0)
3547+
{
3548+
VisitSpace(namedVar.Prefix, p);
3549+
VisitSpace(namedVar.Name.Prefix, p);
3550+
p.Append(namedVar.Name.SimpleName);
3551+
}
35663552
}
35673553

3568-
Visit(catchClause.Body, p);
3554+
VisitSpace(catchClause.Parameter.Tree.After, p);
3555+
p.Append(')');
35693556
}
35703557

3571-
// Print finally if present
3572-
if (innerTry.Finally != null)
3558+
// Print when clause if present
3559+
if (whenInitializer != null)
35733560
{
3574-
VisitSpace(innerTry.Finally.Before, p);
3575-
p.Append("finally");
3576-
Visit(innerTry.Finally.Element, p);
3561+
VisitSpace(whenInitializer.Before, p);
3562+
p.Append("when");
3563+
Visit(whenInitializer.Element, p);
35773564
}
35783565

3579-
AfterSyntax(eft, p);
3580-
return eft;
3566+
VisitBlock(catchClause.Body, p);
3567+
}
3568+
3569+
public override J VisitWhenClause(WhenClause whenClause, PrintOutputCapture<P> p)
3570+
{
3571+
BeforeSyntax(whenClause, p);
3572+
VisitSpace(whenClause.Condition.Prefix, p);
3573+
p.Append('(');
3574+
Visit(whenClause.Condition.Tree.Element, p);
3575+
VisitSpace(whenClause.Condition.Tree.After, p);
3576+
p.Append(')');
3577+
AfterSyntax(whenClause, p);
3578+
return whenClause;
35813579
}
35823580

35833581
public override J VisitExplicitInterfaceMember(ExplicitInterfaceMember eim, PrintOutputCapture<P> p)

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

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public class CSharpVisitor<P> : JavaVisitor<P>
126126
FunctionPointerType fpt => VisitFunctionPointerType(fpt, p),
127127
TypeWithArguments twa => VisitTypeWithArguments(twa, p),
128128
ExplicitInterfaceMember eim => VisitExplicitInterfaceMember(eim, p),
129-
ExceptionFilteredTry eft => VisitExceptionFilteredTry(eft, p),
129+
WhenClause wc => VisitWhenClause(wc, p),
130130
// LINQ
131131
QueryExpression qe => VisitQueryExpression(qe, p),
132132
QueryBody qb => VisitQueryBody(qb, p),
@@ -1469,36 +1469,19 @@ public virtual J VisitTypeWithArguments(TypeWithArguments twa, P p)
14691469
.WithArguments(VisitContainer(node.Arguments, p)!);
14701470
}
14711471

1472-
public virtual J VisitExceptionFilteredTry(ExceptionFilteredTry eft, P p)
1472+
public virtual J VisitWhenClause(WhenClause whenClause, P p)
14731473
{
1474-
eft = eft
1475-
.WithPrefix(VisitSpace(eft.Prefix, p))
1476-
.WithMarkers(VisitMarkers(eft.Markers, p));
1474+
whenClause = whenClause
1475+
.WithPrefix(VisitSpace(whenClause.Prefix, p))
1476+
.WithMarkers(VisitMarkers(whenClause.Markers, p));
14771477

1478-
var stmtResult = VisitStatement(eft, p);
1479-
if (stmtResult is not ExceptionFilteredTry node) return stmtResult;
1478+
var exprResult = VisitExpression(whenClause, p);
1479+
if (exprResult is not WhenClause node) return exprResult;
14801480

14811481
node = node
1482-
.WithTry((Try)Visit(node.Try, p)!);
1482+
.WithCondition((ControlParentheses<Expression>)Visit(node.Condition, p)!);
14831483

1484-
// Keep manual loop for CatchFilters since the list element type is nullable (JLeftPadded<...>?)
1485-
var newFilters = new List<JLeftPadded<ControlParentheses<Expression>>?>(node.CatchFilters.Count);
1486-
bool filtersChanged = false;
1487-
foreach (var filter in node.CatchFilters)
1488-
{
1489-
if (filter != null)
1490-
{
1491-
var visited = VisitLeftPadded(filter, p);
1492-
if (!ReferenceEquals(visited, filter)) filtersChanged = true;
1493-
newFilters.Add(visited);
1494-
}
1495-
else
1496-
{
1497-
newFilters.Add(null);
1498-
}
1499-
}
1500-
1501-
return node.WithCatchFilters(filtersChanged ? newFilters : node.CatchFilters);
1484+
return node;
15021485
}
15031486

15041487
public virtual J VisitExplicitInterfaceMember(ExplicitInterfaceMember eim, P p)

0 commit comments

Comments
 (0)
⚡