Skip to content

Commit 21b5849

Browse files
GrahamTheCoderGrahamTheCoder
committed
Use var pattern to reduce scope of variable - fixes #323 
1 parent 130288f commit 21b5849

4 files changed

Lines changed: 52 additions & 30 deletions

File tree

‎CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -642,13 +642,14 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
642642
{
643643
var vbExpr = node.SelectStatement.Expression;
644644
var vbEquality = CommonConversions.VisualBasicEqualityComparison;
645-
var (csExprToReuse, stmts, csSwitchExpr) = await GetExpressionWithoutSideEffectsAsync(vbExpr, "switchExpr");
645+
646+
var csSwitchExpr = (ExpressionSyntax)await vbExpr.AcceptAsync(_expressionVisitor);
647+
csSwitchExpr = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(vbExpr, csSwitchExpr);
646648
var switchExprTypeInfo = _semanticModel.GetTypeInfo(vbExpr);
647649
var isStringComparison = switchExprTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String || switchExprTypeInfo.ConvertedType?.IsArrayOf(SpecialType.System_Char) == true;
648650
var caseInsensitiveStringComparison = vbEquality.OptionCompareTextCaseInsensitive &&
649651
isStringComparison;
650652
if (isStringComparison) {
651-
csExprToReuse = vbEquality.VbCoerceToNonNullString(vbExpr, csExprToReuse, switchExprTypeInfo);
652653
csSwitchExpr = vbEquality.VbCoerceToNonNullString(vbExpr, csSwitchExpr, switchExprTypeInfo);
653654
}
654655

@@ -668,26 +669,31 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
668669

669670
// Pass both halves in case we can optimize away the check based on the switch expr
670671
var wrapForStringComparison = isStringComparison && (caseInsensitiveStringComparison ||
671-
vbEquality.VbCoerceToNonNullString(vbExpr, csExprToReuse, switchExprTypeInfo, true, s.Value, originalExpressionSyntax, caseTypeInfo, false).rhs != originalExpressionSyntax);
672+
vbEquality.VbCoerceToNonNullString(vbExpr, csSwitchExpr, switchExprTypeInfo, true, s.Value, originalExpressionSyntax, caseTypeInfo, false).rhs != originalExpressionSyntax);
672673

673674
var csExpressionToUse = wrapForStringComparison ? originalExpressionSyntax : correctTypeExpressionSyntax.Expr;
674675

675676
var caseSwitchLabelSyntax = !wrapForStringComparison && correctTypeExpressionSyntax.IsConst && notAlreadyUsed
676677
? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(csExpressionToUse)
677-
: WrapInCasePatternSwitchLabelSyntax(node, s.Value, csExpressionToUse, caseInsensitiveStringComparison);
678+
: WrapInCasePatternSwitchLabelSyntax(node, s.Value, csExpressionToUse);
678679
labels.Add(caseSwitchLabelSyntax);
679680
} else if (c is VBSyntax.ElseCaseClauseSyntax) {
680681
labels.Add(SyntaxFactory.DefaultSwitchLabel());
681682
} else if (c is VBSyntax.RelationalCaseClauseSyntax relational) {
683+
684+
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
685+
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName);
682686
var operatorKind = VBasic.VisualBasicExtensions.Kind(relational);
683687
var relationalValue = (ExpressionSyntax)await relational.Value.AcceptAsync(_expressionVisitor);
684-
var binaryExp = SyntaxFactory.BinaryExpression(operatorKind.ConvertToken(TokenContext.Local), csExprToReuse, relationalValue);
685-
labels.Add(WrapInCasePatternSwitchLabelSyntax(node, relational.Value, binaryExp, caseInsensitiveStringComparison, treatAsBoolean: true));
688+
var binaryExp = SyntaxFactory.BinaryExpression(operatorKind.ConvertToken(TokenContext.Local), csLeft, relationalValue);
689+
labels.Add(VarWhen(varName, binaryExp));
686690
} else if (c is VBSyntax.RangeCaseClauseSyntax range) {
687-
var lowerBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, (ExpressionSyntax)await range.LowerBound.AcceptAsync(_expressionVisitor), csExprToReuse);
688-
var upperBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, csExprToReuse, (ExpressionSyntax)await range.UpperBound.AcceptAsync(_expressionVisitor));
691+
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
692+
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName);
693+
var lowerBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, (ExpressionSyntax)await range.LowerBound.AcceptAsync(_expressionVisitor), csLeft);
694+
var upperBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, csLeft, (ExpressionSyntax)await range.UpperBound.AcceptAsync(_expressionVisitor));
689695
var withinBounds = SyntaxFactory.BinaryExpression(SyntaxKind.LogicalAndExpression, lowerBoundCheck, upperBoundCheck);
690-
labels.Add(WrapInCasePatternSwitchLabelSyntax(node, range.LowerBound, withinBounds, caseInsensitiveStringComparison, treatAsBoolean: true));
696+
labels.Add(VarWhen(varName, withinBounds));
691697
} else throw new NotSupportedException(c.Kind().ToString());
692698
}
693699

@@ -701,7 +707,14 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
701707
}
702708

703709
var switchStatementSyntax = ValidSyntaxFactory.SwitchStatement(csSwitchExpr, sections);
704-
return stmts.Add(switchStatementSyntax);
710+
return SingleStatement(switchStatementSyntax);
711+
}
712+
713+
private static CasePatternSwitchLabelSyntax VarWhen(SyntaxToken varName, ExpressionSyntax binaryExp)
714+
{
715+
var patternMatch = ValidSyntaxFactory.VarPattern(varName);
716+
return SyntaxFactory.CasePatternSwitchLabel(patternMatch,
717+
SyntaxFactory.WhenClause(binaryExp), SyntaxFactory.Token(SyntaxKind.ColonToken));
705718
}
706719

707720
private async Task<(ExpressionSyntax Reusable, SyntaxList<StatementSyntax> Statements, ExpressionSyntax SingleUse)> GetExpressionWithoutSideEffectsAsync(VBSyntax.ExpressionSyntax vbExpr, string variableNameBase, bool forceVariable = false)
@@ -739,7 +752,7 @@ private async Task<bool> IsNeverMutatedAsync(VBSyntax.NameSyntax ns)
739752
return symbol.MatchesKind(SymbolKind.Parameter, SymbolKind.Local) && await CommonConversions.Document.Project.Solution.IsNeverWrittenAsync(symbol, allowedLocation);
740753
}
741754

742-
private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, ExpressionSyntax expression, bool caseInsensitiveTextComparison, bool treatAsBoolean = false)
755+
private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, ExpressionSyntax expression, bool treatAsBoolean = false)
743756
{
744757
var typeInfo = _semanticModel.GetTypeInfo(node.SelectStatement.Expression);
745758

@@ -750,9 +763,7 @@ private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax
750763
SyntaxFactory.DiscardDesignation());
751764
} else {
752765
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
753-
//CodeAnalysis upgrade to 3.0.0 needed for VarPattern. Correct text comes out, but tree is invalid so the tests this will generate "CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code"
754-
patternMatch = SyntaxFactory.DeclarationPattern(
755-
ValidSyntaxFactory.VarType, SyntaxFactory.SingleVariableDesignation(varName));
766+
patternMatch = ValidSyntaxFactory.VarPattern(varName);
756767
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName), csRight = expression;
757768
var caseTypeInfo = _semanticModel.GetTypeInfo(vbCase);
758769
var vbEquality = CommonConversions.VisualBasicEqualityComparison;

‎CodeConverter/CSharp/ValidSyntaxFactory.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ expressionSyntax is IdentifierNameSyntax ||
7373

7474
public static MethodDeclarationSyntax CreateParameterlessMethod(string newMethodName, TypeSyntax type, BlockSyntax body)
7575
{
76-
var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.StaticKeyword));
76+
var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.StaticKeyword));
7777
var typeConstraints = SyntaxFactory.List<TypeParameterConstraintClauseSyntax>();
7878
var parameterList = SyntaxFactory.ParameterList();
7979
var methodAttrs = SyntaxFactory.List<AttributeListSyntax>();
@@ -89,5 +89,15 @@ public static MethodDeclarationSyntax CreateParameterlessMethod(string newMethod
8989
if (arrowExpression != null) methodDecl = methodDecl.WithSemicolonToken(SyntaxFactory.Token(SyntaxKind.SemicolonToken));
9090
return methodDecl;
9191
}
92+
93+
94+
/// <remarks>
95+
/// CodeAnalysis upgrade to 3.0.0 needed for VarPattern. Correct text comes out, but tree is invalid so the tests this will generate "CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code"
96+
/// </remarks>
97+
public static DeclarationPatternSyntax VarPattern(SyntaxToken varName)
98+
{
99+
return SyntaxFactory.DeclarationPattern(
100+
ValidSyntaxFactory.VarType, SyntaxFactory.SingleVariableDesignation(varName));
101+
}
92102
}
93103
}

‎Tests/CSharp/MissingSemanticModelInfo/ExpressionTests.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,7 @@ public enum PositionEnum : int
198198
public PositionEnum PositionEnumFromString(string pS, MissingType missing)
199199
{
200200
var tPos = default(PositionEnum);
201-
var switchExpr = pS.ToUpper();
202-
switch (switchExpr ?? """")
201+
switch (pS.ToUpper() ?? """")
203202
{
204203
case ""NONE"":
205204
case ""0"":

‎Tests/CSharp/StatementTests/StatementTests.cs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,16 +1539,16 @@ public static string TimeAgo(int daysAgo)
15391539
{
15401540
switch (daysAgo)
15411541
{
1542-
case object _ when 0 <= daysAgo && daysAgo <= 3:
1542+
case var @case when 0 <= @case && @case <= 3:
15431543
case 4:
1544-
case object _ when daysAgo >= 5:
1545-
case object _ when daysAgo < 6:
1546-
case object _ when daysAgo <= 7:
1544+
case var case1 when case1 >= 5:
1545+
case var case2 when case2 < 6:
1546+
case var case3 when case3 <= 7:
15471547
{
15481548
return ""this week"";
15491549
}
15501550
1551-
case object _ when daysAgo > 0:
1551+
case var case4 when case4 > 0:
15521552
{
15531553
return daysAgo / 7 + "" weeks ago"";
15541554
}
@@ -1559,7 +1559,9 @@ public static string TimeAgo(int daysAgo)
15591559
}
15601560
}
15611561
}
1562-
}");
1562+
}
1563+
1 target compilation errors:
1564+
CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code");
15631565
}
15641566

15651567
[Fact]
@@ -1584,8 +1586,7 @@ public partial class TestClass
15841586
{
15851587
public static string TimeAgo(string x)
15861588
{
1587-
var switchExpr = Strings.UCase(x);
1588-
switch (switchExpr ?? """")
1589+
switch (Strings.UCase(x) ?? """")
15891590
{
15901591
case var @case when @case == (Strings.UCase(""a"") ?? """"):
15911592
case var case1 when case1 == (Strings.UCase(""b"") ?? """"):
@@ -1704,10 +1705,9 @@ public partial class TestClass2
17041705
public void DoesNotThrow()
17051706
{
17061707
var rand = new Random();
1707-
var switchExpr = rand.Next(8);
1708-
switch (switchExpr)
1708+
switch (rand.Next(8))
17091709
{
1710-
case object _ when switchExpr < 4:
1710+
case var @case when @case < 4:
17111711
{
17121712
break;
17131713
}
@@ -1717,7 +1717,7 @@ public void DoesNotThrow()
17171717
break;
17181718
}
17191719
1720-
case object _ when switchExpr > 4:
1720+
case var case1 when case1 > 4:
17211721
{
17221722
break;
17231723
}
@@ -1729,7 +1729,9 @@ public void DoesNotThrow()
17291729
}
17301730
}
17311731
}
1732-
}");
1732+
}
1733+
1 target compilation errors:
1734+
CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code");
17331735
}
17341736

17351737
[Fact]

0 commit comments

Comments
 (0)