From c0745d88381e4397e1f3d636544495b5a759f770 Mon Sep 17 00:00:00 2001 From: GrahamTheCoder Date: Tue, 13 Feb 2024 01:16:10 +0000 Subject: [PATCH] Don't try to ref foreach, Me or Using identifiers - fixes #1052 --- .../CSharp/CachedReflectedDelegates.cs | 6 ++ .../MethodBodyExecutableStatementVisitor.cs | 12 +-- .../StatementTests/MethodStatementTests.cs | 86 +++++++++++++++++++ 3 files changed, 99 insertions(+), 5 deletions(-) diff --git a/CodeConverter/CSharp/CachedReflectedDelegates.cs b/CodeConverter/CSharp/CachedReflectedDelegates.cs index b2adc8f63..94a73018f 100644 --- a/CodeConverter/CSharp/CachedReflectedDelegates.cs +++ b/CodeConverter/CSharp/CachedReflectedDelegates.cs @@ -25,6 +25,12 @@ public static ISymbol GetAssociatedField(this IPropertySymbol declaredSymbol) => public static SyntaxTree GetEmbeddedSyntaxTree(this Location loc) => GetCachedReflectedPropertyDelegate(loc, "PossiblyEmbeddedOrMySourceTree", ref _possiblyEmbeddedOrMySourceTree); private static Func _possiblyEmbeddedOrMySourceTree; + + public static bool GetIsUsing(this ILocalSymbol l) => + GetCachedReflectedPropertyDelegate(l, "IsUsing", ref _isUsing); + private static Func _isUsing; + + /// Unfortunately the roslyn UnassignedVariablesWalker and all useful collections created from it are internal only /// Other attempts using DataFlowsIn on each reference showed that "DataFlowsIn" even from an uninitialized variable (at least in the case of ints) diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs index 9e36972c2..79adfdc80 100644 --- a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs +++ b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs @@ -870,7 +870,10 @@ private static CasePatternSwitchLabelSyntax VarWhen(SyntaxToken varName, Express SyntaxList stmts = SyntaxFactory.List(); ExpressionSyntax exprWithoutSideEffects; ExpressionSyntax reusableExprWithoutSideEffects; - if (!await CanEvaluateMultipleTimesAsync(vbExpr)) { + if (IsReusableReadOnlyLocalKind(_semanticModel.GetSymbolInfo(vbExpr).Symbol) || await CanEvaluateMultipleTimesAsync(vbExpr)) { + exprWithoutSideEffects = expr; + reusableExprWithoutSideEffects = expr.WithoutSourceMapping(); + } else { TypeSyntax forceType = null; if (_semanticModel.GetOperation(vbExpr.SkipIntoParens()).IsAssignableExpression()) { forceType = SyntaxFactory.RefType(ValidSyntaxFactory.VarType); @@ -880,14 +883,13 @@ private static CasePatternSwitchLabelSyntax VarWhen(SyntaxToken varName, Express var (stmt, id) = CreateLocalVariableWithUniqueName(vbExpr, variableNameBase, expr, forceType); stmts = stmts.Add(stmt); reusableExprWithoutSideEffects = exprWithoutSideEffects = id; - } else { - exprWithoutSideEffects = expr; - reusableExprWithoutSideEffects = expr.WithoutSourceMapping(); } return (reusableExprWithoutSideEffects, stmts, exprWithoutSideEffects); } + private static bool IsReusableReadOnlyLocalKind(ISymbol symbol) => symbol is ILocalSymbol ls && (VBasic.VisualBasicExtensions.IsForEach(ls) || ls.GetIsUsing()); + private (StatementSyntax Declaration, IdentifierNameSyntax Reference) CreateLocalVariableWithUniqueName(VBSyntax.ExpressionSyntax vbExpr, string variableNameBase, ExpressionSyntax expr, TypeSyntax forceType = null) { var contextNode = vbExpr.GetAncestor() ?? (VBasic.VisualBasicSyntaxNode) vbExpr.Parent; @@ -903,7 +905,7 @@ private static CasePatternSwitchLabelSyntax VarWhen(SyntaxToken varName, Express private async Task CanEvaluateMultipleTimesAsync(VBSyntax.ExpressionSyntax vbExpr) { - return _semanticModel.GetConstantValue(vbExpr).HasValue || vbExpr.SkipIntoParens() is VBSyntax.NameSyntax ns && await IsNeverMutatedAsync(ns); + return _semanticModel.GetConstantValue(vbExpr).HasValue || vbExpr.IsKind(VBasic.SyntaxKind.MeExpression) || vbExpr.SkipIntoParens() is VBSyntax.NameSyntax ns && await IsNeverMutatedAsync(ns); } private async Task IsNeverMutatedAsync(VBSyntax.NameSyntax ns) diff --git a/Tests/CSharp/StatementTests/MethodStatementTests.cs b/Tests/CSharp/StatementTests/MethodStatementTests.cs index e2a4ee27b..b2f39e433 100644 --- a/Tests/CSharp/StatementTests/MethodStatementTests.cs +++ b/Tests/CSharp/StatementTests/MethodStatementTests.cs @@ -530,6 +530,92 @@ public partial struct SomeStruct }"); } + [Fact] + public async Task WithBlockMeClassAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Public Class TestWithMe + Private _x As Integer + Sub S() + With Me + ._x = 1 + ._x = 2 + End With + End Sub +End Class", @" +public partial class TestWithMe +{ + private int _x; + public void S() + { + _x = 1; + _x = 2; + } +}"); + } + + [Fact] + public async Task WithBlockMeStructAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Public Structure TestWithMe + Private _x As Integer + Sub S() + With Me + ._x = 1 + ._x = 2 + End With + End Sub +End Structure", @" +public partial struct TestWithMe +{ + private int _x; + public void S() + { + _x = 1; + _x = 2; + } +}"); + } + + [Fact] + public async Task WithBlockForEachAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Imports System.Collections.Generic + +Public Class TestWithForEachClass + Private _x As Integer + + Public Shared Sub Main() + Dim x = New List(Of TestWithForEachClass)() + For Each y In x + With y + ._x = 1 + System.Console.Write(._x) + End With + y = Nothing + Next + End Sub +End Class", @"using System; +using System.Collections.Generic; + +public partial class TestWithForEachClass +{ + private int _x; + + public static void Main() + { + var x = new List(); + foreach (var y in x) + { + y._x = 1; + Console.Write(y._x); + y = (TestWithForEachClass)null; + } + } +} +1 target compilation errors: +CS1656: Cannot assign to 'y' because it is a 'foreach iteration variable'"); + } + [Fact] public async Task NestedWithBlockAsync() {