Neaten up case when variable is only used within loop

Author: GrahamTheCoder

CodeConverter/CSharp/CommonConversions.cs

@@ -58,7 +58,7 @@ public CommonConversions(Document document, SemanticModel semanticModel,
         }
 
         public async Task<(IReadOnlyCollection<(VariableDeclarationSyntax Decl, ITypeSymbol Type)> Variables, IReadOnlyCollection<CSharpSyntaxNode> Methods)> SplitVariableDeclarations(
-            VariableDeclaratorSyntax declarator, bool preferExplicitType = false)
+            VariableDeclaratorSyntax declarator, HashSet<ILocalSymbol> symbolsToSkip = null, bool preferExplicitType = false)
         {
             var vbInitValue = GetInitializerToConvert(declarator);
             var initializerOrMethodDecl = await vbInitValue.AcceptAsync(TriviaConvertingExpressionVisitor);
@@ -81,6 +81,7 @@ public CommonConversions(Document document, SemanticModel semanticModel,
             foreach (var name in declarator.Names) {
 
                 var declaredSymbol = _semanticModel.GetDeclaredSymbol(name);
+                if (symbolsToSkip?.Contains(declaredSymbol) == true) continue;
                 var declaredSymbolType = declaredSymbol.GetSymbolType();
                 var equalsValueClauseSyntax = await ConvertEqualsValueClauseSyntax(declarator, name, vbInitValue, declaredSymbolType, declaredSymbol, initializerOrMethodDecl);
                 var v = SyntaxFactory.VariableDeclarator(ConvertIdentifier(name.Identifier), null, equalsValueClauseSyntax);

CodeConverter/CSharp/DeclarationNodeVisitor.cs

@@ -49,7 +49,7 @@ internal class DeclarationNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSh
         private string _topAncestorNamespace;
 
         private CommonConversions CommonConversions { get; }
-        private Func<VisualBasicSyntaxNode, bool, IdentifierNameSyntax, VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>> _createMethodBodyVisitor { get; }
+        private Func<VisualBasicSyntaxNode, bool, IdentifierNameSyntax, Task<VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>>> _createMethodBodyVisitorAsync { get; }
 
         public DeclarationNodeVisitor(Document document, Compilation compilation, SemanticModel semanticModel,
             CSharpCompilation csCompilation, SyntaxGenerator csSyntaxGenerator)
@@ -66,13 +66,13 @@ public DeclarationNodeVisitor(Document document, Compilation compilation, Semant
             _additionalInitializers = new AdditionalInitializers();
             var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, _additionalLocals, csCompilation, _methodsWithHandles, CommonConversions, _extraUsingDirectives);
             _triviaConvertingExpressionVisitor = expressionNodeVisitor.TriviaConvertingExpressionVisitor;
-            _createMethodBodyVisitor = expressionNodeVisitor.CreateMethodBodyVisitor;
+            _createMethodBodyVisitorAsync = expressionNodeVisitor.CreateMethodBodyVisitor;
             CommonConversions.TriviaConvertingExpressionVisitor = _triviaConvertingExpressionVisitor;
         }
 
-        public VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>> CreateMethodBodyVisitor(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
+        public async Task<VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>> CreateMethodBodyVisitor(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
         {
-            return _createMethodBodyVisitor(node, isIterator, csReturnVariable);
+            return await _createMethodBodyVisitorAsync(node, isIterator, csReturnVariable);
         }
 
         public override async Task<CSharpSyntaxNode> DefaultVisit(SyntaxNode node)
@@ -809,7 +809,7 @@ public override async Task<CSharpSyntaxNode> VisitAccessorBlock(VBSyntax.Accesso
             Microsoft.CodeAnalysis.CSharp.SyntaxKind blockKind;
             bool isIterator = node.IsIterator();
             var csReturnVariableOrNull = CommonConversions.GetRetVariableNameOrNull(node);
-            var convertedStatements = await ConvertStatements(node.Statements, CreateMethodBodyVisitor(node, isIterator));
+            var convertedStatements = await ConvertStatements(node.Statements, await CreateMethodBodyVisitor(node, isIterator));
             var body = WithImplicitReturnStatements(node, convertedStatements, csReturnVariableOrNull);
             var attributes = await CommonConversions.ConvertAttributes(node.AccessorStatement.AttributeLists);
             var modifiers = CommonConversions.ConvertModifiers(node, node.AccessorStatement.Modifiers, TokenContext.Local);
@@ -889,7 +889,7 @@ public override async Task<CSharpSyntaxNode> VisitMethodBlock(VBSyntax.MethodBlo
                 return methodBlock;
             }
             var csReturnVariableOrNull = CommonConversions.GetRetVariableNameOrNull(node);
-            var visualBasicSyntaxVisitor = CreateMethodBodyVisitor(node, node.IsIterator(), csReturnVariableOrNull);
+            var visualBasicSyntaxVisitor = await CreateMethodBodyVisitor(node, node.IsIterator(), csReturnVariableOrNull);
             var convertedStatements = await ConvertStatements(node.Statements, visualBasicSyntaxVisitor);
 
             if (node.SubOrFunctionStatement.Identifier.Text == "InitializeComponent" && node.SubOrFunctionStatement.IsKind(VBasic.SyntaxKind.SubStatement) && declaredSymbol.ContainingType.IsDesignerGeneratedTypeWithInitializeComponent(_compilation)) {
@@ -1140,7 +1140,7 @@ public override async Task<CSharpSyntaxNode> VisitOperatorStatement(VBSyntax.Ope
             var attributeList = SyntaxFactory.List(attributes);
             var returnType = (TypeSyntax) await (node.AsClause?.Type).AcceptAsync(_triviaConvertingExpressionVisitor) ?? SyntaxFactory.PredefinedType(SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.VoidKeyword));
             var parameterList = (ParameterListSyntax) await node.ParameterList.AcceptAsync(_triviaConvertingExpressionVisitor);
-            var methodBodyVisitor = CreateMethodBodyVisitor(node);
+            var methodBodyVisitor = await CreateMethodBodyVisitor(node);
             var body = SyntaxFactory.Block(await containingBlock.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor)));
             var modifiers = CommonConversions.ConvertModifiers(node, node.Modifiers, GetMemberContext(node));
 
@@ -1181,7 +1181,7 @@ public override async Task<CSharpSyntaxNode> VisitConstructorBlock(VBSyntax.Cons
                 ctorCall = null;
             }
 
-            var methodBodyVisitor = CreateMethodBodyVisitor(node);
+            var methodBodyVisitor = await CreateMethodBodyVisitor(node);
             return SyntaxFactory.ConstructorDeclaration(
                 SyntaxFactory.List(attributes),
                 modifiers, 

CodeConverter/CSharp/ExpressionNodeVisitor.cs

@@ -179,7 +179,7 @@ public override async Task<CSharpSyntaxNode> VisitCatchBlock(VBasic.Syntax.Catch
             }
 
             var filter = (CatchFilterClauseSyntax) await stmt.WhenClause.AcceptAsync(TriviaConvertingExpressionVisitor);
-            var methodBodyVisitor = CreateMethodBodyVisitor(node); //Probably should actually be using the existing method body visitor in order to get variable name generation correct
+            var methodBodyVisitor = await CreateMethodBodyVisitor(node); //Probably should actually be using the existing method body visitor in order to get variable name generation correct
             var stmts = await node.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor));
             return SyntaxFactory.CatchClause(
                 catcher,
@@ -202,7 +202,7 @@ public override async Task<CSharpSyntaxNode> VisitCatchFilterClause(VBasic.Synta
 
         public override async Task<CSharpSyntaxNode> VisitFinallyBlock(VBasic.Syntax.FinallyBlockSyntax node)
         {
-            var methodBodyVisitor = CreateMethodBodyVisitor(node); //Probably should actually be using the existing method body visitor in order to get variable name generation correct
+            var methodBodyVisitor = await CreateMethodBodyVisitor(node); //Probably should actually be using the existing method body visitor in order to get variable name generation correct
             var stmts = await node.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor));
             return SyntaxFactory.FinallyClause(SyntaxFactory.Block(stmts));
         }
@@ -864,7 +864,7 @@ public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBa
         {
             IReadOnlyCollection<StatementSyntax> convertedStatements;
             if (node.Body is VBasic.Syntax.StatementSyntax statement) {
-                convertedStatements = await statement.Accept(CreateMethodBodyVisitor(node));
+                convertedStatements = await statement.Accept(await CreateMethodBodyVisitor(node));
             } else {
                 var csNode = await node.Body.AcceptAsync(TriviaConvertingExpressionVisitor);
                 convertedStatements = new[] { SyntaxFactory.ExpressionStatement((ExpressionSyntax)csNode)};
@@ -875,7 +875,7 @@ public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBa
 
         public override async Task<CSharpSyntaxNode> VisitMultiLineLambdaExpression(VBasic.Syntax.MultiLineLambdaExpressionSyntax node)
         {
-            var methodBodyVisitor = CreateMethodBodyVisitor(node);
+            var methodBodyVisitor = await CreateMethodBodyVisitor(node);
             var body = await node.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor));
             var param = (ParameterListSyntax) await node.SubOrFunctionHeader.ParameterList.AcceptAsync(TriviaConvertingExpressionVisitor);
             return await _lambdaConverter.Convert(node, param, body.ToList());
@@ -1170,12 +1170,9 @@ public override async Task<CSharpSyntaxNode> VisitTypeArgumentList(VBasic.Syntax
             return SyntaxFactory.TypeArgumentList(SyntaxFactory.SeparatedList(args));
         }
 
-        public VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>> CreateMethodBodyVisitor(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
+        public async Task<VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>> CreateMethodBodyVisitor(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
         {
-            var methodBodyVisitor = new MethodBodyExecutableStatementVisitor(node, _semanticModel, TriviaConvertingExpressionVisitor, CommonConversions, _withBlockLhs, _extraUsingDirectives, _additionalLocals, _methodsWithHandles) {
-                IsIterator = isIterator,
-                ReturnVariable = csReturnVariable,
-            };
+            var methodBodyVisitor = await MethodBodyExecutableStatementVisitor.CreateAsync(node, _semanticModel, TriviaConvertingExpressionVisitor, CommonConversions, _withBlockLhs, _extraUsingDirectives, _additionalLocals, _methodsWithHandles, isIterator, csReturnVariable);
             return methodBodyVisitor.CommentConvertingVisitor;
         }
 

CodeConverter/CSharp/Extensions.cs

@@ -0,0 +1,75 @@
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace ICSharpCode.CodeConverter.CSharp
+{
+    internal static class Extensions
+    {
+        /// <summary>
+        /// Returns the single statement in a block if it has no nested statements.
+        /// If it has nested statements, and the surrounding block was removed, it could be ambiguous,
+        /// e.g. if (...) { if (...) return null; } else return "";
+        /// Unbundling the middle if statement would bind the else to it, rather than the outer if statement
+        /// </summary>
+        public static StatementSyntax UnpackNonNestedBlock(this BlockSyntax block)
+        {
+            return block.Statements.Count == 1 && !block.ContainsNestedStatements() ? block.Statements[0] : block;
+        }
+
+        /// <summary>
+        /// Returns the single statement in a block
+        /// </summary>
+        public static bool TryUnpackSingleStatement(this IReadOnlyCollection<StatementSyntax> statements, out StatementSyntax singleStatement)
+        {
+            singleStatement = statements.Count == 1 ? statements.Single() : null;
+            if (singleStatement is BlockSyntax block && TryUnpackSingleStatement(block.Statements, out var s)) {
+                singleStatement = s;
+            }
+
+            return singleStatement != null;
+        }
+
+        /// <summary>
+        /// Returns the single expression in a statement
+        /// </summary>
+        public static bool TryUnpackSingleExpressionFromStatement(this StatementSyntax statement, out ExpressionSyntax singleExpression)
+        {
+            switch(statement){
+                case BlockSyntax blockSyntax:
+                    singleExpression = null;
+                    return TryUnpackSingleStatement(blockSyntax.Statements, out var nestedStmt) &&
+                           TryUnpackSingleExpressionFromStatement(nestedStmt, out singleExpression);
+                case ExpressionStatementSyntax expressionStatementSyntax:
+                    singleExpression = expressionStatementSyntax.Expression;
+                    return singleExpression != null;
+                case ReturnStatementSyntax returnStatementSyntax:
+                    singleExpression = returnStatementSyntax.Expression;
+                    return singleExpression != null;
+                default:
+                    singleExpression = null;
+                    return false;
+            }
+        }
+
+        /// <summary>
+        /// Only use this over <see cref="UnpackNonNestedBlock"/> in special cases where it will display more neatly and where you're sure nested statements don't introduce ambiguity
+        /// </summary>
+        public static StatementSyntax UnpackPossiblyNestedBlock(this BlockSyntax block)
+        {
+            SyntaxList<StatementSyntax> statementSyntaxs = block.Statements;
+            return statementSyntaxs.Count == 1 ? statementSyntaxs[0] : block;
+        }
+
+        private static bool ContainsNestedStatements(this BlockSyntax block)
+        {
+            return block.Statements.Any(HasDescendantCSharpStatement);
+        }
+
+        private static bool HasDescendantCSharpStatement(this StatementSyntax c)
+        {
+            return c.DescendantNodes().OfType<StatementSyntax>().Any();
+        }
+    }
+}

CodeConverter/CSharp/LocalVariableAnalyzer.cs

@@ -0,0 +1,31 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using ICSharpCode.CodeConverter.Shared;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;
+using ICSharpCode.CodeConverter.Util.FromRoslyn;
+using Microsoft.CodeAnalysis.VisualBasic;
+using Microsoft.CodeAnalysis.VisualBasic.Syntax;
+
+namespace ICSharpCode.CodeConverter.CSharp
+{
+    internal static class LocalVariableAnalyzer
+    {
+        public static async Task<HashSet<ILocalSymbol>> GetDescendantsToInlineInLoopAsync(this Solution solution, SemanticModel semanticModel, VisualBasicSyntaxNode methodNode)
+        {
+            var forEachControlVariables = await methodNode.DescendantNodes().OfType<VBSyntax.ForEachBlockSyntax>().SelectAsync(forEach => GetLoopVariablesToInlineAsync(solution, semanticModel, forEach));
+            return new HashSet<ILocalSymbol>(forEachControlVariables.Where(f => f != null), SymbolEquivalenceComparer.Instance);
+        }
+
+        private static async Task<ILocalSymbol> GetLoopVariablesToInlineAsync(Solution solution, SemanticModel semanticModel, ForEachBlockSyntax block)
+        {
+            if (semanticModel.GetSymbolInfo(block.ForEachStatement.ControlVariable).Symbol is ILocalSymbol varSymbol) {
+                var usagesOutsideLoop = await solution.GetUsagesAsync(varSymbol, block.GetLocation());
+                if (!usagesOutsideLoop.Any()) return varSymbol;
+            }
+            return null;
+        }
+    }
+}