Fix bug in Pass Parameter by Value quick fix

Author: Hosch250

RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs

@@ -49,7 +49,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
                 && ((VBAParser.ArgContext)declaration.Context).BYVAL() == null
                 && !IsUsedAsByRefParam(declarations, declaration)
                 && !declaration.References.Any(reference => reference.IsAssignment))
-                .Select(issue => new ParameterCanBeByValInspectionResult(this, issue, ((dynamic)issue.Context).unrestrictedIdentifier(), issue.QualifiedName));
+                .Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName));
 
             return issues;
         }

RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs

@@ -36,14 +36,13 @@ public PassParameterByValueQuickFix(ParserRuleContext context, QualifiedSelectio
 
         public override void Fix()
         {
-            var parameter = Context.Parent.GetText();
-            var newContent = string.Concat(Tokens.ByVal, " ", parameter.Replace(Tokens.ByRef, string.Empty).Trim());
             var selection = Selection.Selection;
+            var selectionLength = ((VBAParser.ArgContext) Context).BYREF() == null ? 0 : 6;
 
             var module = Selection.QualifiedName.Component.CodeModule;
-            var lines = module.Lines[selection.StartLine, selection.LineCount];
+            var lines = module.Lines[selection.StartLine, 1];
 
-            var result = lines.Replace(parameter, newContent);
+            var result = lines.Remove(selection.StartColumn - 1, selectionLength).Insert(selection.StartColumn - 1, Tokens.ByVal + ' ');
             module.ReplaceLine(selection.StartLine, result);
         }
     }

RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs

@@ -192,6 +192,63 @@ public void ParameterCanByByVal_ReturnsResult_SomeParams()
             Assert.AreEqual(1, inspectionResults.Count());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void GivenArrayParameter_ReturnsNoResult()
+        {
+            const string inputCode =
+@"Sub Foo(ByRef arg1() As Variant)
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            VBComponent component;
+            var vbe = builder.BuildFromSingleStandardModule(inputCode, out component);
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
+
+            parser.Parse(new CancellationTokenSource());
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ParameterCanBeByValInspection(parser.State);
+
+            var results = inspection.GetInspectionResults().ToList();
+
+            Assert.AreEqual(0, results.Count);
+        }
+
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ParameterCanByByVal_ReturnsResult_QuickFixWorks_SubNameStartsWithParamName()
+        {
+            const string inputCode =
+@"Sub foo(f)
+End Sub";
+
+            const string expectedCode =
+@"Sub foo(ByVal f)
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            VBComponent component;
+            var vbe = builder.BuildFromSingleStandardModule(inputCode, out component);
+            var project = vbe.Object.VBProjects.Item(0);
+            var module = project.VBComponents.Item(0).CodeModule;
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
+
+            parser.Parse(new CancellationTokenSource());
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ParameterCanBeByValInspection(parser.State);
+            inspection.GetInspectionResults().First().QuickFixes.First().Fix();
+
+            Assert.AreEqual(expectedCode, module.Lines());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ParameterCanByByVal_ReturnsResult_QuickFixWorks_PassedByUnspecified()
@@ -256,16 +313,24 @@ public void ParameterCanByByVal_ReturnsResult_QuickFixWorks_PassedByRef()
 
         [TestMethod]
         [TestCategory("Inspections")]
-        public void GivenArrayParameter_ReturnsNoResult()
+        public void ParameterCanByByVal_ReturnsResult_QuickFixWorks_PassedByUnspecified_MultilineParameter()
         {
             const string inputCode =
-@"Sub Foo(ByRef arg1() As Variant)
+@"Sub Foo( _
+arg1 As String)
+End Sub";
+
+            const string expectedCode =
+@"Sub Foo( _
+ByVal arg1 As String)
 End Sub";
 
             //Arrange
             var builder = new MockVbeBuilder();
             VBComponent component;
             var vbe = builder.BuildFromSingleStandardModule(inputCode, out component);
+            var project = vbe.Object.VBProjects.Item(0);
+            var module = project.VBComponents.Item(0).CodeModule;
             var mockHost = new Mock<IHostApplication>();
             mockHost.SetupAllProperties();
             var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
@@ -274,10 +339,42 @@ public void GivenArrayParameter_ReturnsNoResult()
             if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
 
             var inspection = new ParameterCanBeByValInspection(parser.State);
-            
-            var results = inspection.GetInspectionResults().ToList();
+            inspection.GetInspectionResults().First().QuickFixes.First().Fix();
 
-            Assert.AreEqual(0, results.Count);
+            Assert.AreEqual(expectedCode, module.Lines());
+        }
+
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ParameterCanByByVal_ReturnsResult_QuickFixWorks_PassedByRef_MultilineParameter()
+        {
+            const string inputCode =
+@"Sub Foo(ByRef _
+arg1 As String)
+End Sub";
+
+            const string expectedCode =
+@"Sub Foo(ByVal _
+arg1 As String)
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            VBComponent component;
+            var vbe = builder.BuildFromSingleStandardModule(inputCode, out component);
+            var project = vbe.Object.VBProjects.Item(0);
+            var module = project.VBComponents.Item(0).CodeModule;
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
+
+            parser.Parse(new CancellationTokenSource());
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ParameterCanBeByValInspection(parser.State);
+            inspection.GetInspectionResults().First().QuickFixes.First().Fix();
+
+            Assert.AreEqual(expectedCode, module.Lines());
         }
 
         [TestMethod]