Merge pull request #4022 from tommy9/MoveCloserWithLineNumbers

Stop removing new line when removing an expression with a line number
rubberduck-vba · May 22, 2018 · 864bc3c · 864bc3c
2 parents 024eecd + 21c4299
commit 864bc3c
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 13 deletions.
diff --git a/Rubberduck.Parsing/Rewriter/RewriterInfo/VariableRewriterInfoFinder.cs b/Rubberduck.Parsing/Rewriter/RewriterInfo/VariableRewriterInfoFinder.cs
@@ -58,13 +58,14 @@ private static RewriterInfo GetRewriterInfo(VBAParser.VariableSubStmtContext var
             int count, int itemIndex, IReadOnlyList<VBAParser.VariableSubStmtContext> items)
         {
             var mainBlockStmt = (VBAParser.MainBlockStmtContext)variables.Parent.Parent;
+            var blockStmt = (VBAParser.BlockStmtContext)mainBlockStmt.Parent;
             var startIndex = mainBlockStmt.Start.TokenIndex;
             if (count == 1)
             {
                 int stopIndex = variables.Stop.TokenIndex + 1; // also remove trailing newlines?
 
                 var containingBlock = (VBAParser.BlockContext)mainBlockStmt.Parent.Parent;
-                var blockStmtIndex = containingBlock.children.IndexOf(mainBlockStmt.Parent);
+                var blockStmtIndex = containingBlock.children.IndexOf(blockStmt);
                 // a few things can happen here
                 if (blockStmtIndex == containingBlock.ChildCount)
                 {
@@ -80,27 +81,28 @@ private static RewriterInfo GetRewriterInfo(VBAParser.VariableSubStmtContext var
                     {
                         stopIndex = eol.commentOrAnnotation().Start.TokenIndex - 1;
                     }
+                    else if (blockStmtIndex + 2 >= containingBlock.ChildCount)
+                    {
+                        // remove until the end of the EOS
+                        stopIndex = eol.Stop.TokenIndex;
+                    }
+                    else if (blockStmt.statementLabelDefinition() != null)
+                    {
+                        // special case where line has statement label or line number
+                        // don't want to remove trailing newline or can have label/number apply to next line where it isn't necessarily valid
+                        stopIndex = eol.Stop.TokenIndex - 1;
+                    }
                     else
                     {
-                        // remove until the end of the EOS or continue to the start of the following
-                        if (blockStmtIndex + 2 >= containingBlock.ChildCount)
-                        {
-                            stopIndex = eol.Stop.TokenIndex;
-                        }
-                        else
-                        {
-                            stopIndex = containingBlock.GetChild<ParserRuleContext>(blockStmtIndex + 2).Start.TokenIndex - 1;
-                        }
+                        // remove up to the start of the following blockStmt (offset index by 2 because blockStmts alternate with endOfStatements)
+                        stopIndex = containingBlock.GetChild<ParserRuleContext>(blockStmtIndex + 2).Start.TokenIndex - 1;
                     }
 
                 }
 
                 return new RewriterInfo(startIndex, stopIndex);
             }
 
-            var blockStmt = (VBAParser.BlockStmtContext)mainBlockStmt.Parent;
-            var block = (VBAParser.BlockContext)blockStmt.Parent;
-            var statements = block.blockStmt();
             return GetRewriterInfoForTargetRemovedFromListStmt(target.Start, itemIndex, items);
         }
     }

diff --git a/RubberduckTests/Refactoring/MoveCloserToUsageTests.cs b/RubberduckTests/Refactoring/MoveCloserToUsageTests.cs
@@ -206,6 +206,43 @@ public void MoveCloserToUsageRefactoring_Variable()
             }
         }
 
+        [Test]
+        [Category("Refactorings")]
+        [Category("Move Closer")]
+        public void MoveCloserToUsageRefactoring_VariableWithLineNumbers()
+        {
+            //Input
+            const string inputCode =
+                @"Private Sub Foo()
+1   Dim bar As Boolean
+2   Dim bat As Integer
+3   bar = True
+End Sub";
+            var selection = new Selection(4, 6, 4, 8);
+
+            //Expectation
+            const string expectedCode =
+                @"Private Sub Foo()
+1   
+2   Dim bat As Integer
+Dim bar As Boolean
+3   bar = True
+End Sub";
+
+            var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component, selection);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+
+                var qualifiedSelection = new QualifiedSelection(new QualifiedModuleName(component), selection);
+
+                var refactoring = new MoveCloserToUsageRefactoring(vbe.Object, state, null);
+                refactoring.Refactor(qualifiedSelection);
+
+                var rewriter = state.GetRewriter(component);
+                Assert.AreEqual(expectedCode, rewriter.GetText());
+            }
+        }
+
         [Test]
         [Category("Refactorings")]
         [Category("Move Closer")]