Colon seperated statement handling and whitespace improvements

Author: tommy9

Rubberduck.Refactorings/ExtractMethod/ExtractMethodModel.cs

@@ -83,13 +83,19 @@ private void Setup()
             //List of "inbound" variables. Parent procedure parameters + explicit dims which get referenced inside the selection.
             //TODO - add case where reference earlier in the same line as where the selection starts (unusual but could exist if using colons to separate multiple statements)
             var inboundParameters = sourceMethodParameters.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)));
-            var inboundLocalVariables = declarationsInParentMethod.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
-                                                                              d.References.Any(r => r.Selection.EndLine < QualifiedSelection.Selection.StartLine));
+            var inboundLocalVariables = declarationsInParentMethod
+                .Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
+                       (d.References.Any(r => r.Selection.EndLine < QualifiedSelection.Selection.StartLine ||
+                                        (r.Selection.EndLine == QualifiedSelection.Selection.StartLine &&
+                                         r.Selection.EndColumn < QualifiedSelection.Selection.StartColumn))));
             var inboundVariables = inboundParameters.Concat(inboundLocalVariables);
 
             //List of "outbound" variables. Any variables assigned a value in the selection which are then referenced after the selection
             var outboundVariables = sourceMethodParameters.Concat(declarationsInParentMethod)
-                .Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) && d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine));
+                .Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
+                                (d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine ||
+                                                 (r.Selection.StartLine == QualifiedSelection.Selection.EndLine &&
+                                                  r.Selection.StartColumn > QualifiedSelection.Selection.EndColumn))));
 
             SetUpParameters(inboundVariables, outboundVariables);
 
@@ -103,8 +109,12 @@ private void Setup()
             // - where declaration is before the selection but only references are inside the selection
             _declarationsToMoveIn = declarationsInParentMethod.Except(declarationsInSelection)
                                     .Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
-                                           !d.References.Any(r => r.Selection.EndLine < QualifiedSelection.Selection.StartLine) &&
-                                           !d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine));
+                                           !d.References.Any(r => r.Selection.EndLine < QualifiedSelection.Selection.StartLine ||
+                                                            (r.Selection.EndLine == QualifiedSelection.Selection.StartLine &&
+                                                             r.Selection.EndColumn < QualifiedSelection.Selection.StartColumn)) &&
+                                           !d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine ||
+                                                            (r.Selection.StartLine == QualifiedSelection.Selection.EndLine &&
+                                                             r.Selection.StartColumn > QualifiedSelection.Selection.EndColumn)));
         }
 
         private IOrderedEnumerable<Declaration> GetDeclarationsInSelection(QualifiedSelection qualifiedSelection)
@@ -157,6 +167,7 @@ private void SetUpParameters(IEnumerable<Declaration> inboundVariables, IEnumera
         }
 
         public string SelectedCode { get; private set; }
+        private int SelectionIndentation;
 
         //Code excluding declarations that are to be moved out of the selection
         public string SelectedCodeToExtract 
@@ -170,10 +181,21 @@ public string SelectedCodeToExtract
                 //TODO - create way to map selection to the string or confirm that commented out code above works (including whitespace)
 
                 var targetMethodSelection = TargetMethod.Selection;
-                var selectionCode = targetMethodCode.Skip(QualifiedSelection.Selection.StartLine - targetMethodSelection.StartLine)
-                                                    .Take(QualifiedSelection.Selection.EndLine - QualifiedSelection.Selection.StartLine + 1)
-                                                    .ToList();
                 var selectionToExtract = QualifiedSelection.Selection;
+                var selectionCode = targetMethodCode.Skip(selectionToExtract.StartLine - targetMethodSelection.StartLine)
+                                                    .Take(selectionToExtract.EndLine - selectionToExtract.StartLine + 1)
+                                                    .ToList();
+                SelectionIndentation = selectionCode[0].Length - selectionCode[0].TrimStart(' ').Length;
+                //Handle first and last lines of selection.
+                //As a start, detect if code on those lines that is not in the selection and throw error
+                if (selectionToExtract.StartColumn > 1)
+                {
+                    selectionCode[0] = selectionCode[0].Substring(selectionToExtract.StartColumn - 1);
+                }
+                if (selectionToExtract.EndColumn < targetMethodCode[selectionToExtract.EndLine - selectionToExtract.StartLine].Length)
+                {
+                    selectionCode[selectionCode.Count - 1] = selectionCode[selectionCode.Count - 1].Substring(0, selectionToExtract.EndColumn - 1);
+                }
 
                 foreach (var decl in _declarationsToMoveOut.Except(_declarationsToMoveOut.Where(d => d.IdentifierName == ReturnParameter.Name)))
                 {
@@ -207,17 +229,6 @@ public string SelectedCodeToExtract
                     selectionCode.RemoveAt(startLine - selectionToExtract.StartLine);
                 }
 
-                //TODO - Handle first and last lines of selection. Could be affected by previous moves, so need better approach using parse tree.
-                //As a start, detect if code on those lines that is not in the selection and throw error
-                //if (selectionToExtract.StartColumn > 1)
-                //{
-                //    selectionCode[0] = selectionCode[0].Substring(selectionToExtract.StartColumn - 1);
-                //}
-                //if (selectionToExtract.EndColumn < targetMethodCode[selectionToExtract.EndLine - selectionToExtract.StartLine].Length)
-                //{
-                //    selectionCode[selectionCode.Count - 1] = selectionCode[selectionCode.Count - 1].Substring(0, selectionToExtract.EndColumn);
-                //}
-
                 //var selectionsToMoveOut = (from dec in _declarationsToMoveOut 
                 //                           orderby dec.Selection.StartLine, dec.Selection.StartColumn 
                 //                           select dec.Selection);
@@ -234,16 +245,7 @@ public string ReplacementCode
             get
             {
                 var strings = new List<string> { string.Empty };
-                string indentation;
-                if (SelectedContexts.First().GetType() == typeof(VBAParser.BlockStmtContext))
-                {
-                    indentation = FrontPadding((VBAParser.BlockStmtContext)SelectedContexts.First());
-                }
-                else
-                {
-                    var enclosingStatementContext = SelectedContexts.First().GetAncestor<VBAParser.BlockStmtContext>();
-                    indentation = FrontPadding(enclosingStatementContext);                    
-                }
+                var indentation = new string(' ', SelectionIndentation);
 
                 foreach (var dec in _declarationsToMoveOut)
                 {
@@ -259,7 +261,7 @@ public string ReplacementCode
                 }
 
                 // Make call to new method
-                var argList = string.Join(",", from p in ArgumentsToPass select p.Name);
+                var argList = string.Join(", ", from p in ArgumentsToPass select p.Name);
 
                 if (ReturnParameter == ExtractMethodParameter.None)
                 {
@@ -318,20 +320,6 @@ public string NewMethodCode
             }
         }
 
-        private string FrontPadding(VBAParser.BlockStmtContext context)
-        {
-            //TODO - starts from block statement but that could be following another statement e.g. a = 1 : b = 2, so better to find whole line
-            var paddingChars = context.Start.Column;
-            if (paddingChars > 0)
-            {
-                return new string(' ', paddingChars);
-            }
-            else
-            {
-                return string.Empty;
-            }
-        }
-
         private static bool IsStatic(Declaration declaration)
         {
             var ctxt = declaration.Context.GetAncestor<VBAParser.VariableStmtContext>();

RubberduckTests/Refactoring/ExtractMethod/ExtractMethodModelTests.cs

@@ -451,9 +451,69 @@ End If
             });
         }
 
+        [Test(Description = "Handle indenting and splitting of code when two statements in one colon-separated line")]
+        [Category("Refactorings")]
+        [Category("ExtractMethod")]
+        public void FindCorrectIndentingForReplacementCode()
+        {
+            var inputCode = @"
+Sub Test
+    Dim a As Integer: Dim b As Integer
+    a = 10
+    b = 12
+    Debug.Print a + b
+End Sub";
+            var selection = new Selection(3, 23, 6, 22);
+            var expectedNewMethodCode = @"
+Private Sub NewMethod()
+    Dim a As Integer
+    Dim b As Integer
+    a = 10
+    b = 12
+    Debug.Print a + b
+End Sub";
+            var expectedReplacementCode = @"
+    NewMethod";
+            var model = InitialModel(inputCode, selection, true);
+            var newMethodCode = model.NewMethodCode;
+
+            Assert.AreEqual(expectedNewMethodCode, newMethodCode);
+            var replacementCode = model.ReplacementCode;
+            Assert.AreEqual(expectedReplacementCode, replacementCode);
+        }
+
+
+
+        [Test(Description = "Two references to a variable in the same line, only one extracted")]
+        [Category("Refactorings")]
+        [Category("ExtractMethod")]
+        public void SplitOutLogicalStatementFromMultistatementLine()
+        {
+            var inputCode = @"
+Sub splitter()
+    Dim a As Integer
+    Dim b As Integer
+    b = 10: b = b + 10
+    a = b
+    Debug.Print a
+End Sub";
+            var selection = new Selection(5, 13, 6, 10);
+            var expectedNewMethodCode = @"
+Private Sub NewMethod(ByVal b As Integer, ByRef a As Integer)
+    b = b + 10
+    a = b
+End Sub";
+            var expectedReplacementCode = @"
+    NewMethod b, a";
+            var model = InitialModel(inputCode, selection, true);
+            var newMethodCode = model.NewMethodCode;
+
+            Assert.AreEqual(expectedNewMethodCode, newMethodCode);
+            var replacementCode = model.ReplacementCode;
+            Assert.AreEqual(expectedReplacementCode, replacementCode);
+        }
 
         //TODO - add Dim a As New Object with declaration copied/split
-        //TODO - add changes to original function return value (or raise error if too complex)
         //TODO - exception testing to cover all exception types (and message???)
 
         protected override IRefactoring TestRefactoring(