Addressed PR initial comments except Dialog dependency removal

BZngr · BZngr · commit b151934e7bb3 · 2017-02-23T21:02:21.000-08:00
diff --git a/RetailCoder.VBE/Common/CodeModuleExtensions.cs b/RetailCoder.VBE/Common/CodeModuleExtensions.cs
@@ -4,20 +4,31 @@
 
 namespace Rubberduck.Common
 {
-    public static class ICodeModuleExtensions
+    public static class CodeModuleExtensions
     {
         public static void ReplaceToken(this ICodeModule module, IToken token, string replacement)
         {
             var original = module.GetLines(token.Line, 1);
             var result = ReplaceStringAtIndex(original, token.Text, replacement, token.Column);
             module.ReplaceLine(token.Line, result);
         }
+
         public static void ReplaceIdentifierReferenceName(this ICodeModule module, IdentifierReference identifierReference, string replacement)
         {
             var original = module.GetLines(identifierReference.Selection.StartLine, 1);
             var result = ReplaceStringAtIndex(original, identifierReference.IdentifierName, replacement, identifierReference.Context.Start.Column);
             module.ReplaceLine(identifierReference.Selection.StartLine, result);
         }
+
+        public static void InsertLines(this ICodeModule module, int startLine, string[] lines)
+        {
+            int lineNumber = startLine;
+            for ( int idx = 0; idx < lines.Length; idx++ )
+            {
+                module.InsertLines(lineNumber, lines[idx]);
+                lineNumber++;
+            }
+        }
         private static string ReplaceStringAtIndex(string original, string toReplace, string replacement, int startIndex)
         {
             var stopIndex = startIndex + toReplace.Length - 1;
diff --git a/RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs b/RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs
@@ -94,15 +94,17 @@ private void ReplaceAssignedByValParameterReferences()
                 module.ReplaceIdentifierReferenceName(identifierReference, _localCopyVariableName);
             }
         }
+
         private void InsertLocalVariableDeclarationAndAssignment()
         {
             var blocks = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent);
             var firstBlockLineNumber =  blocks.FirstOrDefault().Start.Line;
 
             var module = Selection.QualifiedName.Component.CodeModule;
-            module.InsertLines(firstBlockLineNumber++, BuildLocalCopyDeclaration());
-            module.InsertLines(firstBlockLineNumber, BuildLocalCopyAssignment());
+            string[] lines = { BuildLocalCopyDeclaration(), BuildLocalCopyAssignment() };
+            module.InsertLines(firstBlockLineNumber, lines);
         }
+
         private string BuildLocalCopyDeclaration()
         {
             return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As 
@@ -113,14 +115,14 @@ private string BuildLocalCopyAssignment()
             return (SymbolList.ValueTypes.Contains(_target.AsTypeName) ? string.Empty : Tokens.Set + " ") 
                 + _localCopyVariableName + " = " + _target.IdentifierName;
         }
+
         private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleContext)
         {
             var allIdentifiers = new HashSet<string>();
 
             var blocks = QuickFixHelper.GetBlockStmtContextsForContext(ruleContext);
 
             var blockStmtIdentifiers = GetIdentifierNames(blocks);
-
             allIdentifiers.UnionWith(blockStmtIdentifiers);
 
             var args = QuickFixHelper.GetArgContextsForContext(ruleContext);
@@ -132,6 +134,7 @@ private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleCo
 
             return allIdentifiers.ToArray();
         }
+
         private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContexts)
         {
             var identifiers = new HashSet<string>();
@@ -142,6 +145,7 @@ private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContex
             }
             return identifiers;
         }
+
         private HashSet<string> GetIdentifierNames(RuleContext ruleContext)
         {
             //Recursively work through the tree to get all IdentifierContexts
@@ -169,6 +173,7 @@ private HashSet<string> GetIdentifierNames(RuleContext ruleContext)
             }
             return results;
         }
+
         private static List<IParseTree> GetChildren(RuleContext ruleCtx)
         {
             var result = new List<IParseTree>();
diff --git a/RetailCoder.VBE/Inspections/QuickFixes/PassParameterByReferenceQuickFix.cs b/RetailCoder.VBE/Inspections/QuickFixes/PassParameterByReferenceQuickFix.cs
@@ -31,7 +31,6 @@ public override void Fix()
                     .Equals(_target.IdentifierName));
 
             module.ReplaceToken(argContext.BYVAL().Symbol,Tokens.ByRef);
-            
         }
     }
 }
diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.de.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.de.resx
@@ -594,4 +594,7 @@ Falls der Parameter 'null' sein kann, bitte dieses Auftreten ignorieren. 'null'
   <data name="AggregateInspectionResultFormat" xml:space="preserve">
     <value>{0} ({1} Ergebnisse)</value>
   </data>
+  <data name="AssignedByValParameterMakeLocalCopyQuickFix" xml:space="preserve">
+    <value>Create and use a local copy of the parameter</value>
+  </data>
 </root>
diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.fr.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.fr.resx
@@ -604,4 +604,7 @@ Si le paramètre peut être nul, ignorer ce résultat; passer une valeur nulle 
   <data name="AggregateInspectionResultFormat" xml:space="preserve">
     <value>{0} ({1} résultats)</value>
   </data>
+  <data name="AssignedByValParameterMakeLocalCopyQuickFix" xml:space="preserve">
+    <value>Create and use a local copy of the parameter</value>
+  </data>
 </root>
diff --git a/RetailCoder.VBE/Rubberduck.csproj b/RetailCoder.VBE/Rubberduck.csproj
@@ -325,7 +325,7 @@
     <Compile Include="Common\Hotkeys\Hotkey.cs" />
     <Compile Include="Common\IAttachable.cs" />
     <Compile Include="Common\Hotkeys\IHotkey.cs" />
-    <Compile Include="Common\ICodeModuleExtensions.cs" />
+    <Compile Include="Common\CodeModuleExtensions.cs" />
     <Compile Include="Common\IOperatingSystem.cs" />
     <Compile Include="Common\IRubberduckHooks.cs" />
     <Compile Include="Common\LogLevelHelper.cs" />
diff --git a/RubberduckTests/Inspections/AssignedByValParameterInspectionTests.cs b/RubberduckTests/Inspections/AssignedByValParameterInspectionTests.cs
@@ -140,6 +140,7 @@ Public Sub Foo(ByVal arg1 As String)
             var quickFixResult = ApplyIgnoreOnceQuickFixToCodeFragment(inputCode);
             Assert.AreEqual(expectedCode, quickFixResult); 
         }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void InspectionType()
@@ -158,7 +159,6 @@ public void InspectionName()
             Assert.AreEqual(inspectionName, inspection.Name);
         }
 
-
         private void AssertVbaFragmentYieldsExpectedInspectionResultCount(string inputCode, int expectedCount)
         {
             var inspectionResults = GetInspectionResults(inputCode);
diff --git a/RubberduckTests/Inspections/AssignedByValParameterMakeLocalCopyQuickFixTests.cs b/RubberduckTests/Inspections/AssignedByValParameterMakeLocalCopyQuickFixTests.cs
@@ -466,6 +466,7 @@ private string GetModuleContent(Mock<IVBE> vbe)
             var module = project.VBComponents[0].CodeModule;
             return module.Content();
         }
+
         private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe)
         {
             var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
@@ -475,6 +476,7 @@ private string GetModuleContent(Mock<IVBE> vbe)
             var inspection = new AssignedByValParameterInspection(parser.State);
             return inspection.GetInspectionResults();
         }
+
         private Mock<IVBE> BuildMockVBEStandardModuleForVBAFragment(string inputCode)
         {
             var builder = new MockVbeBuilder();
diff --git a/RubberduckTests/Inspections/PassParameterByReferenceQuickFixTests.cs b/RubberduckTests/Inspections/PassParameterByReferenceQuickFixTests.cs
@@ -161,7 +161,6 @@ End Sub
 ";
             quickFixResult = ApplyPassParameterByReferenceQuickFixToVBAFragment(inputCode);
             Assert.AreEqual(expectedCode, quickFixResult);
-
         }
         [TestMethod]
         [TestCategory("Inspections")]
@@ -197,6 +196,7 @@ private string GetModuleContent(Mock<IVBE> vbe)
             var module = project.VBComponents[0].CodeModule;
             return module.Content();
         }
+
         private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe)
         {
             var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
@@ -206,6 +206,7 @@ private string GetModuleContent(Mock<IVBE> vbe)
             var inspection = new AssignedByValParameterInspection(parser.State);
             return inspection.GetInspectionResults();
         }
+
         private Mock<IVBE> BuildMockVBEStandardModuleForVBAFragment(string inputCode)
         {
             var builder = new MockVbeBuilder();

Original file line number	Diff line number	Diff line change
`@@ -4,20 +4,31 @@`
`4`	`4`
`5`	`5`	`namespace Rubberduck.Common`
`6`	`6`	`{`
`7`		`- public static class ICodeModuleExtensions`
	`7`	`+ public static class CodeModuleExtensions`
`8`	`8`	`{`
`9`	`9`	`public static void ReplaceToken(this ICodeModule module, IToken token, string replacement)`
`10`	`10`	`{`
`11`	`11`	`var original = module.GetLines(token.Line, 1);`
`12`	`12`	`var result = ReplaceStringAtIndex(original, token.Text, replacement, token.Column);`
`13`	`13`	`module.ReplaceLine(token.Line, result);`
`14`	`14`	`}`
	`15`	`+`
`15`	`16`	`public static void ReplaceIdentifierReferenceName(this ICodeModule module, IdentifierReference identifierReference, string replacement)`
`16`	`17`	`{`
`17`	`18`	`var original = module.GetLines(identifierReference.Selection.StartLine, 1);`
`18`	`19`	`var result = ReplaceStringAtIndex(original, identifierReference.IdentifierName, replacement, identifierReference.Context.Start.Column);`
`19`	`20`	`module.ReplaceLine(identifierReference.Selection.StartLine, result);`
`20`	`21`	`}`
	`22`	`+`
	`23`	`+ public static void InsertLines(this ICodeModule module, int startLine, string[] lines)`
	`24`	`+ {`
	`25`	`+ int lineNumber = startLine;`
	`26`	`+ for ( int idx = 0; idx < lines.Length; idx++ )`
	`27`	`+ {`
	`28`	`+ module.InsertLines(lineNumber, lines[idx]);`
	`29`	`+ lineNumber++;`
	`30`	`+ }`
	`31`	`+ }`
`21`	`32`	`private static string ReplaceStringAtIndex(string original, string toReplace, string replacement, int startIndex)`
`22`	`33`	`{`
`23`	`34`	`var stopIndex = startIndex + toReplace.Length - 1;`
Original file line number	Diff line number	Diff line change
`@@ -94,15 +94,17 @@ private void ReplaceAssignedByValParameterReferences()`
`94`	`94`	`module.ReplaceIdentifierReferenceName(identifierReference, _localCopyVariableName);`
`95`	`95`	`}`
`96`	`96`	`}`
	`97`	`+`
`97`	`98`	`private void InsertLocalVariableDeclarationAndAssignment()`
`98`	`99`	`{`
`99`	`100`	`var blocks = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent);`
`100`	`101`	`var firstBlockLineNumber = blocks.FirstOrDefault().Start.Line;`
`101`	`102`
`102`	`103`	`var module = Selection.QualifiedName.Component.CodeModule;`
`103`		`- module.InsertLines(firstBlockLineNumber++, BuildLocalCopyDeclaration());`
`104`		`- module.InsertLines(firstBlockLineNumber, BuildLocalCopyAssignment());`
	`104`	`+ string[] lines = { BuildLocalCopyDeclaration(), BuildLocalCopyAssignment() };`
	`105`	`+ module.InsertLines(firstBlockLineNumber, lines);`
`105`	`106`	`}`
	`107`	`+`
`106`	`108`	`private string BuildLocalCopyDeclaration()`
`107`	`109`	`{`
`108`	`110`	`return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As`
`@@ -113,14 +115,14 @@ private string BuildLocalCopyAssignment()`
`113`	`115`	`return (SymbolList.ValueTypes.Contains(_target.AsTypeName) ? string.Empty : Tokens.Set + " ")`
`114`	`116`	`+ _localCopyVariableName + " = " + _target.IdentifierName;`
`115`	`117`	`}`
	`118`	`+`
`116`	`119`	`private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleContext)`
`117`	`120`	`{`
`118`	`121`	`var allIdentifiers = new HashSet<string>();`
`119`	`122`
`120`	`123`	`var blocks = QuickFixHelper.GetBlockStmtContextsForContext(ruleContext);`
`121`	`124`
`122`	`125`	`var blockStmtIdentifiers = GetIdentifierNames(blocks);`
`123`		`-`
`124`	`126`	`allIdentifiers.UnionWith(blockStmtIdentifiers);`
`125`	`127`
`126`	`128`	`var args = QuickFixHelper.GetArgContextsForContext(ruleContext);`
`@@ -132,6 +134,7 @@ private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleCo`
`132`	`134`
`133`	`135`	`return allIdentifiers.ToArray();`
`134`	`136`	`}`
	`137`	`+`
`135`	`138`	`private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContexts)`
`136`	`139`	`{`
`137`	`140`	`var identifiers = new HashSet<string>();`
`@@ -142,6 +145,7 @@ private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContex`
`142`	`145`	`}`
`143`	`146`	`return identifiers;`
`144`	`147`	`}`
	`148`	`+`
`145`	`149`	`private HashSet<string> GetIdentifierNames(RuleContext ruleContext)`
`146`	`150`	`{`
`147`	`151`	`//Recursively work through the tree to get all IdentifierContexts`
`@@ -169,6 +173,7 @@ private HashSet<string> GetIdentifierNames(RuleContext ruleContext)`
`169`	`173`	`}`
`170`	`174`	`return results;`
`171`	`175`	`}`
	`176`	`+`
`172`	`177`	`private static List<IParseTree> GetChildren(RuleContext ruleCtx)`
`173`	`178`	`{`
`174`	`179`	`var result = new List<IParseTree>();`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,6 @@ public override void Fix()`
`31`	`31`	`.Equals(_target.IdentifierName));`
`32`	`32`
`33`	`33`	`module.ReplaceToken(argContext.BYVAL().Symbol,Tokens.ByRef);`
`34`		`-`
`35`	`34`	`}`
`36`	`35`	`}`
`37`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -140,6 +140,7 @@ Public Sub Foo(ByVal arg1 As String)`
`140`	`140`	`var quickFixResult = ApplyIgnoreOnceQuickFixToCodeFragment(inputCode);`
`141`	`141`	`Assert.AreEqual(expectedCode, quickFixResult);`
`142`	`142`	`}`
	`143`	`+`
`143`	`144`	`[TestMethod]`
`144`	`145`	`[TestCategory("Inspections")]`
`145`	`146`	`public void InspectionType()`
`@@ -158,7 +159,6 @@ public void InspectionName()`
`158`	`159`	`Assert.AreEqual(inspectionName, inspection.Name);`
`159`	`160`	`}`
`160`	`161`
`161`		`-`
`162`	`162`	`private void AssertVbaFragmentYieldsExpectedInspectionResultCount(string inputCode, int expectedCount)`
`163`	`163`	`{`
`164`	`164`	`var inspectionResults = GetInspectionResults(inputCode);`
Original file line number	Diff line number	Diff line change
`@@ -466,6 +466,7 @@ private string GetModuleContent(Mock<IVBE> vbe)`
`466`	`466`	`var module = project.VBComponents[0].CodeModule;`
`467`	`467`	`return module.Content();`
`468`	`468`	`}`
	`469`	`+`
`469`	`470`	`private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe)`
`470`	`471`	`{`
`471`	`472`	`var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));`
`@@ -475,6 +476,7 @@ private string GetModuleContent(Mock<IVBE> vbe)`
`475`	`476`	`var inspection = new AssignedByValParameterInspection(parser.State);`
`476`	`477`	`return inspection.GetInspectionResults();`
`477`	`478`	`}`
	`479`	`+`
`478`	`480`	`private Mock<IVBE> BuildMockVBEStandardModuleForVBAFragment(string inputCode)`
`479`	`481`	`{`
`480`	`482`	`var builder = new MockVbeBuilder();`
Original file line number	Diff line number	Diff line change
`@@ -161,7 +161,6 @@ End Sub`
`161`	`161`	`";`
`162`	`162`	`quickFixResult = ApplyPassParameterByReferenceQuickFixToVBAFragment(inputCode);`
`163`	`163`	`Assert.AreEqual(expectedCode, quickFixResult);`
`164`		`-`
`165`	`164`	`}`
`166`	`165`	`[TestMethod]`
`167`	`166`	`[TestCategory("Inspections")]`
`@@ -197,6 +196,7 @@ private string GetModuleContent(Mock<IVBE> vbe)`
`197`	`196`	`var module = project.VBComponents[0].CodeModule;`
`198`	`197`	`return module.Content();`
`199`	`198`	`}`
	`199`	`+`
`200`	`200`	`private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe)`
`201`	`201`	`{`
`202`	`202`	`var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));`
`@@ -206,6 +206,7 @@ private string GetModuleContent(Mock<IVBE> vbe)`
`206`	`206`	`var inspection = new AssignedByValParameterInspection(parser.State);`
`207`	`207`	`return inspection.GetInspectionResults();`
`208`	`208`	`}`
	`209`	`+`
`209`	`210`	`private Mock<IVBE> BuildMockVBEStandardModuleForVBAFragment(string inputCode)`
`210`	`211`	`{`
`211`	`212`	`var builder = new MockVbeBuilder();`