PR#2733 tweaks

retailcoder · retailcoder · commit 96a16adfdc78 · 2017-03-01T11:09:20.000-05:00
diff --git a/RetailCoder.VBE/Inspections/AssignedByValParameterInspection.cs b/RetailCoder.VBE/Inspections/AssignedByValParameterInspection.cs
@@ -3,7 +3,6 @@
 using Rubberduck.Inspections.Abstract;
 using Rubberduck.Inspections.Resources;
 using Rubberduck.Inspections.Results;
-using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
 using Rubberduck.UI.Refactorings;
@@ -12,7 +11,7 @@ namespace Rubberduck.Inspections
 {
     public sealed class AssignedByValParameterInspection : InspectionBase
     {
-        private IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
+        private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
         public AssignedByValParameterInspection(RubberduckParserState state, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
             : base(state)
         {
diff --git a/RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs b/RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs
@@ -1,6 +1,5 @@
 ﻿using Rubberduck.Inspections.Abstract;
 using System.Linq;
-using Rubberduck.Parsing;
 using Rubberduck.VBEditor;
 using Rubberduck.Inspections.Resources;
 using Rubberduck.Parsing.Grammar;
@@ -18,16 +17,16 @@ public class AssignedByValParameterMakeLocalCopyQuickFix : QuickFixBase
     {
         private readonly Declaration _target;
         private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
+        private readonly IEnumerable<string> _forbiddenNames;
         private string _localCopyVariableName;
-        private string[] _variableNamesAccessibleToProcedureContext;
 
         public AssignedByValParameterMakeLocalCopyQuickFix(Declaration target, QualifiedSelection selection, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
             : base(target.Context, selection, InspectionsUI.AssignedByValParameterMakeLocalCopyQuickFix)
         {
             _target = target;
             _dialogFactory = dialogFactory;
-            _variableNamesAccessibleToProcedureContext = GetVariableNamesAccessibleToProcedureContext(_target.Context.Parent.Parent);
-            SetValidLocalCopyVariableNameSuggestion();
+            _forbiddenNames = GetIdentifierNamesAccessibleToProcedureContext(target.Context.Parent.Parent);
+            _localCopyVariableName = ComputeSuggestedName();
         }
 
         public override bool CanFixInModule { get { return false; } }
@@ -49,10 +48,9 @@ public override void Fix()
 
         private void RequestLocalCopyVariableName()
         {
-            using( var view = _dialogFactory.Create(_target.IdentifierName, _target.DeclarationType.ToString()))
+            using( var view = _dialogFactory.Create(_target.IdentifierName, _target.DeclarationType.ToString(), _forbiddenNames))
             {
                 view.NewName = _localCopyVariableName;
-                view.IdentifierNamesAlreadyDeclared = _variableNamesAccessibleToProcedureContext;
                 view.ShowDialog();
                 IsCancelled = view.DialogResult == DialogResult.Cancel;
                 if (!IsCancelled)
@@ -62,63 +60,66 @@ private void RequestLocalCopyVariableName()
             }
         }
 
-        private void SetValidLocalCopyVariableNameSuggestion()
+        private string ComputeSuggestedName()
         {
-            _localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();
-            if (VariableNameIsValid(_localCopyVariableName)) { return; }
+            var newName = "local" + _target.IdentifierName.CapitalizeFirstLetter();
+            if (VariableNameIsValid(newName))
+            {
+                return newName;
+            }
 
-            //If the initial suggestion is not valid, keep pre-pending x's until it is
-            for ( int attempt = 2; attempt < 10; attempt++) 
+            for ( var attempt = 2; attempt < 10; attempt++)
             {
-                _localCopyVariableName = "x" + _localCopyVariableName;
-                if (VariableNameIsValid(_localCopyVariableName))
+                var result = newName + attempt;
+                if (VariableNameIsValid(result))
                 {
-                    return;
+                    return result;
                 }
             }
-            //if "xxFoo" to "xxxxxxxxxxFoo" isn't unique, give up and go with the original suggestion.
-            //The QuickFix will leave the code as-is unless it receives a name that is free of conflicts
-            _localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();
+            return newName;
         }
 
         private bool VariableNameIsValid(string variableName)
         {
             var validator = new VariableNameValidator(variableName);
             return validator.IsValidName()
-                && !_variableNamesAccessibleToProcedureContext
-                    .Any(name => name.Equals(variableName, System.StringComparison.InvariantCultureIgnoreCase));
+                && !_forbiddenNames.Any(name => name.Equals(variableName, System.StringComparison.InvariantCultureIgnoreCase));
         }
 
         private void ReplaceAssignedByValParameterReferences()
         {
             var module = Selection.QualifiedName.Component.CodeModule;
-            foreach (IdentifierReference identifierReference in _target.References)
+            foreach (var identifierReference in _target.References)
             {
                 module.ReplaceIdentifierReferenceName(identifierReference, _localCopyVariableName);
             }
         }
 
         private void InsertLocalVariableDeclarationAndAssignment()
         {
-            var blocks = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent);
+            var block = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent).FirstOrDefault();
+            if (block == null)
+            {
+                return;
+            }
+
             string[] lines = { BuildLocalCopyDeclaration(), BuildLocalCopyAssignment() };
             var module = Selection.QualifiedName.Component.CodeModule;
-            module.InsertLines(blocks.FirstOrDefault().Start.Line, lines);
+            module.InsertLines(block.Start.Line, lines);
         }
 
         private string BuildLocalCopyDeclaration()
         {
-            return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As 
-                + " " + _target.AsTypeName;
+            return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As + " " + _target.AsTypeName;
         }
 
         private string BuildLocalCopyAssignment()
         {
-            return (SymbolList.ValueTypes.Contains(_target.AsTypeName) ? string.Empty : Tokens.Set + " ") 
+            return (_target.AsTypeDeclaration is ClassModuleDeclaration ? Tokens.Set + " " : string.Empty) 
                 + _localCopyVariableName + " = " + _target.IdentifierName;
         }
 
-        private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleContext)
+        private IEnumerable<string> GetIdentifierNamesAccessibleToProcedureContext(RuleContext ruleContext)
         {
             var allIdentifiers = new HashSet<string>();
 
@@ -137,29 +138,31 @@ private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleCo
             return allIdentifiers.ToArray();
         }
 
-        private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContexts)
+        private IEnumerable<string> GetIdentifierNames(IEnumerable<RuleContext> ruleContexts)
         {
             var identifiers = new HashSet<string>();
-            foreach (RuleContext ruleContext in ruleContexts)
+            foreach (var identifiersForThisContext in ruleContexts.Select(GetIdentifierNames))
             {
-                var identifiersForThisContext = GetIdentifierNames(ruleContext);
                 identifiers.UnionWith(identifiersForThisContext);
             }
             return identifiers;
         }
 
-        private HashSet<string> GetIdentifierNames(RuleContext ruleContext)
+        private static HashSet<string> GetIdentifierNames(RuleContext ruleContext)
         {
+            // note: this looks like something that's already handled somewhere else...
+
             //Recursively work through the tree to get all IdentifierContexts
             var results = new HashSet<string>();
-            var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item);
+            var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item).ToArray();
             var children = GetChildren(ruleContext);
 
-            foreach (IParseTree child in children)
+            foreach (var child in children)
             {
-                if (child is VBAParser.IdentifierContext)
+                var context = child as VBAParser.IdentifierContext;
+                if (context != null)
                 {
-                    var childName = Identifier.GetName((VBAParser.IdentifierContext)child);
+                    var childName = Identifier.GetName(context);
                     if (!tokenValues.Contains(childName))
                     {
                         results.Add(childName);
@@ -176,12 +179,12 @@ private HashSet<string> GetIdentifierNames(RuleContext ruleContext)
             return results;
         }
 
-        private static List<IParseTree> GetChildren(RuleContext ruleCtx)
+        private static IEnumerable<IParseTree> GetChildren(IParseTree tree)
         {
             var result = new List<IParseTree>();
-            for (int index = 0; index < ruleCtx.ChildCount; index++)
+            for (var index = 0; index < tree.ChildCount; index++)
             {
-                result.Add(ruleCtx.GetChild(index));
+                result.Add(tree.GetChild(index));
             }
             return result;
         }
diff --git a/RetailCoder.VBE/Inspections/Results/AssignedByValParameterInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AssignedByValParameterInspectionResult.cs
@@ -11,7 +11,7 @@ namespace Rubberduck.Inspections.Results
     public class AssignedByValParameterInspectionResult : InspectionResultBase
     {
         private IEnumerable<QuickFixBase> _quickFixes;
-        private IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
+        private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
 
         public AssignedByValParameterInspectionResult(IInspection inspection, Declaration target, IAssignedByValParameterQuickFixDialogFactory dialogFactory) 
             : base(inspection, target)
diff --git a/RetailCoder.VBE/Inspections/VariableNameValidator.cs b/RetailCoder.VBE/Inspections/VariableNameValidator.cs
@@ -15,7 +15,8 @@ public VariableNameValidator() { }
         private const int MinVariableNameLength = 3;
 
         /****  Meaningful Name Characteristics  ************/
-        public bool HasVowels
+
+        private bool HasVowels
         {
             get
             {
@@ -25,7 +26,7 @@ public bool HasVowels
             }
         }
 
-        public bool HasConsonants
+        private bool HasConsonants
         {
             get
             {
@@ -35,7 +36,7 @@ public bool HasConsonants
             }
         }
 
-        public bool IsSingleRepeatedLetter
+        private bool IsSingleRepeatedLetter
         {
             get
             {
@@ -45,8 +46,8 @@ public bool IsSingleRepeatedLetter
             }
         }
 
-        public bool IsTooShort { get { return _identifier.Length < MinVariableNameLength; } }
-        public bool EndsWithNumber { get { return char.IsDigit(_identifier.Last()); } }
+        private bool IsTooShort { get { return _identifier.Length < MinVariableNameLength; } }
+        private bool EndsWithNumber { get { return char.IsDigit(_identifier.Last()); } }
 
         /****  Invalid Name Characteristics  ************/
         public bool StartsWithNumber { get { return FirstLetterIsDigit(); } }
diff --git a/RetailCoder.VBE/UI/Refactorings/AssignedByValParameterQuickFixDialog.cs b/RetailCoder.VBE/UI/Refactorings/AssignedByValParameterQuickFixDialog.cs
@@ -1,4 +1,5 @@
 ﻿using System;
+using System.Collections.Generic;
 using System.Windows.Forms;
 using Rubberduck.Inspections;
 using System.Linq;
@@ -7,15 +8,15 @@ namespace Rubberduck.UI.Refactorings
 {
     public partial class AssignedByValParameterQuickFixDialog : Form, IAssignedByValParameterQuickFixDialog
     {
-        private string[] _identifierNamesAlreadyDeclared;
-        private string _identifierName;
+        private readonly string _identifierName;
+        private readonly IEnumerable<string> _forbiddenNames;
 
-        internal AssignedByValParameterQuickFixDialog(string identifierName, string declarationType)
+        internal AssignedByValParameterQuickFixDialog(string identifierName, string declarationType, IEnumerable<string> forbiddenNames)
         {
             InitializeComponent();
             InitializeCaptions(identifierName, declarationType);
            _identifierName = identifierName;
-            _identifierNamesAlreadyDeclared = Enumerable.Empty<string>().ToArray();
+            _forbiddenNames = forbiddenNames;
         }
 
         private void InitializeCaptions(string identifierName, string targetDeclarationType)
@@ -47,17 +48,12 @@ public string NewName
                 SetControlsProperties();
             }
         }
-        public string[] IdentifierNamesAlreadyDeclared
-        {
-            get { return _identifierNamesAlreadyDeclared; }
-            set { _identifierNamesAlreadyDeclared = value; }
-        }
 
         private string GetVariableNameFeedback()
         {
             var validator = new VariableNameValidator(NewName);
 
-            if (UserInputIsBlank())
+            if (string.IsNullOrEmpty(NewName))
             {
                 return string.Empty;
             }
@@ -73,11 +69,11 @@ private string GetVariableNameFeedback()
             {
                 return string.Format(RubberduckUI.AssignedByValDialog_ReservedKeywordFormat, NewName);
             }
-            if (IsByValIdentifier())
+            if (NewName.Equals(_identifierName, StringComparison.OrdinalIgnoreCase))
             {
                 return string.Format(RubberduckUI.AssignedByValDialog_IsByValIdentifierFormat, NewName);
             }
-            if (NewNameAlreadyUsed())
+            if (_forbiddenNames.Any(name => name.Equals(NewName, StringComparison.OrdinalIgnoreCase)))
             {
                 return string.Format(RubberduckUI.AssignedByValDialog_NewNameAlreadyUsedFormat, NewName);
             }
@@ -91,27 +87,10 @@ private string GetVariableNameFeedback()
         private void SetControlsProperties()
         {
             var validator = new VariableNameValidator(NewName);
-            var userInputIsValid = validator.IsValidName() && !NewNameAlreadyUsed();
-            OkButton.Visible = userInputIsValid;
-            OkButton.Enabled = userInputIsValid;
-            InvalidNameValidationIcon.Visible = !userInputIsValid;
-        }
-
-        private bool UserInputIsBlank()
-        {
-            return NewName.Equals(string.Empty);
-        }
-
-        private bool IsByValIdentifier()
-        {
-            return NewName.Equals(_identifierName, StringComparison.OrdinalIgnoreCase);
-        }
-
-        private bool NewNameAlreadyUsed()
-        {
-            //Comparison needs to be case-insensitive, or VBE will often change an existing
-            //same-spelling local variable's casing to conform with the NewName
-            return _identifierNamesAlreadyDeclared.Any(n => n.Equals(NewName, StringComparison.OrdinalIgnoreCase));
+            var isValid = validator.IsValidName() && !_forbiddenNames.Any(name => name.Equals(NewName, StringComparison.OrdinalIgnoreCase));
+            OkButton.Visible = isValid;
+            OkButton.Enabled = isValid;
+            InvalidNameValidationIcon.Visible = !isValid;
         }
     }
 }
diff --git a/RetailCoder.VBE/UI/Refactorings/AssignedByValParameterQuickFixDialogFactory.cs b/RetailCoder.VBE/UI/Refactorings/AssignedByValParameterQuickFixDialogFactory.cs
@@ -1,11 +1,13 @@
 ﻿
+using System.Collections.Generic;
+
 namespace Rubberduck.UI.Refactorings
 {
     public class AssignedByValParameterQuickFixDialogFactory : IAssignedByValParameterQuickFixDialogFactory
     {
-        IAssignedByValParameterQuickFixDialog IAssignedByValParameterQuickFixDialogFactory.Create(string identifier, string identifierType)
+        IAssignedByValParameterQuickFixDialog IAssignedByValParameterQuickFixDialogFactory.Create(string identifier, string identifierType, IEnumerable<string> forbiddenNames)
         {
-            return new AssignedByValParameterQuickFixDialog(identifier, identifierType);
+            return new AssignedByValParameterQuickFixDialog(identifier, identifierType, forbiddenNames);
         }
     }
 }
diff --git a/RetailCoder.VBE/UI/Refactorings/IAssignedByValParameterQuickFixDialog.cs b/RetailCoder.VBE/UI/Refactorings/IAssignedByValParameterQuickFixDialog.cs
@@ -11,6 +11,5 @@ public interface IAssignedByValParameterQuickFixDialog : IDialogView
     {
         DialogResult DialogResult { get;}
         string NewName { get; set; }
-        string[] IdentifierNamesAlreadyDeclared { get; set; }
     }
 }
diff --git a/RetailCoder.VBE/UI/Refactorings/IAssignedByValParameterQuickFixDialogFactory.cs b/RetailCoder.VBE/UI/Refactorings/IAssignedByValParameterQuickFixDialogFactory.cs
@@ -10,6 +10,6 @@ namespace Rubberduck.UI.Refactorings
 {
     public interface IAssignedByValParameterQuickFixDialogFactory
     {
-        IAssignedByValParameterQuickFixDialog Create(string identifier, string identifierType);
+        IAssignedByValParameterQuickFixDialog Create(string identifier, string identifierType, IEnumerable<string> forbiddenNames);
     }
 }
diff --git a/RubberduckTests/Inspections/AssignedByValParameterMakeLocalCopyQuickFixTests.cs b/RubberduckTests/Inspections/AssignedByValParameterMakeLocalCopyQuickFixTests.cs

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,6 @@`
`3`	`3`	`using Rubberduck.Inspections.Abstract;`
`4`	`4`	`using Rubberduck.Inspections.Resources;`
`5`	`5`	`using Rubberduck.Inspections.Results;`
`6`		`-using Rubberduck.Parsing.Grammar;`
`7`	`6`	`using Rubberduck.Parsing.Symbols;`
`8`	`7`	`using Rubberduck.Parsing.VBA;`
`9`	`8`	`using Rubberduck.UI.Refactorings;`
`@@ -12,7 +11,7 @@ namespace Rubberduck.Inspections`
`12`	`11`	`{`
`13`	`12`	`public sealed class AssignedByValParameterInspection : InspectionBase`
`14`	`13`	`{`
`15`		`- private IAssignedByValParameterQuickFixDialogFactory _dialogFactory;`
	`14`	`+ private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;`
`16`	`15`	`public AssignedByValParameterInspection(RubberduckParserState state, IAssignedByValParameterQuickFixDialogFactory dialogFactory)`
`17`	`16`	`: base(state)`
`18`	`17`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,5 @@`
`1`	`1`	`using Rubberduck.Inspections.Abstract;`
`2`	`2`	`using System.Linq;`
`3`		`-using Rubberduck.Parsing;`
`4`	`3`	`using Rubberduck.VBEditor;`
`5`	`4`	`using Rubberduck.Inspections.Resources;`
`6`	`5`	`using Rubberduck.Parsing.Grammar;`
`@@ -18,16 +17,16 @@ public class AssignedByValParameterMakeLocalCopyQuickFix : QuickFixBase`
`18`	`17`	`{`
`19`	`18`	`private readonly Declaration _target;`
`20`	`19`	`private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;`
	`20`	`+ private readonly IEnumerable<string> _forbiddenNames;`
`21`	`21`	`private string _localCopyVariableName;`
`22`		`- private string[] _variableNamesAccessibleToProcedureContext;`
`23`	`22`
`24`	`23`	`public AssignedByValParameterMakeLocalCopyQuickFix(Declaration target, QualifiedSelection selection, IAssignedByValParameterQuickFixDialogFactory dialogFactory)`
`25`	`24`	`: base(target.Context, selection, InspectionsUI.AssignedByValParameterMakeLocalCopyQuickFix)`
`26`	`25`	`{`
`27`	`26`	`_target = target;`
`28`	`27`	`_dialogFactory = dialogFactory;`
`29`		`- _variableNamesAccessibleToProcedureContext = GetVariableNamesAccessibleToProcedureContext(_target.Context.Parent.Parent);`
`30`		`- SetValidLocalCopyVariableNameSuggestion();`
	`28`	`+ _forbiddenNames = GetIdentifierNamesAccessibleToProcedureContext(target.Context.Parent.Parent);`
	`29`	`+ _localCopyVariableName = ComputeSuggestedName();`
`31`	`30`	`}`
`32`	`31`
`33`	`32`	`public override bool CanFixInModule { get { return false; } }`
`@@ -49,10 +48,9 @@ public override void Fix()`
`49`	`48`
`50`	`49`	`private void RequestLocalCopyVariableName()`
`51`	`50`	`{`
`52`		`- using( var view = _dialogFactory.Create(_target.IdentifierName, _target.DeclarationType.ToString()))`
	`51`	`+ using( var view = _dialogFactory.Create(_target.IdentifierName, _target.DeclarationType.ToString(), _forbiddenNames))`
`53`	`52`	`{`
`54`	`53`	`view.NewName = _localCopyVariableName;`
`55`		`- view.IdentifierNamesAlreadyDeclared = _variableNamesAccessibleToProcedureContext;`
`56`	`54`	`view.ShowDialog();`
`57`	`55`	`IsCancelled = view.DialogResult == DialogResult.Cancel;`
`58`	`56`	`if (!IsCancelled)`
`@@ -62,63 +60,66 @@ private void RequestLocalCopyVariableName()`
`62`	`60`	`}`
`63`	`61`	`}`
`64`	`62`
`65`		`- private void SetValidLocalCopyVariableNameSuggestion()`
	`63`	`+ private string ComputeSuggestedName()`
`66`	`64`	`{`
`67`		`- _localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();`
`68`		`- if (VariableNameIsValid(_localCopyVariableName)) { return; }`
	`65`	`+ var newName = "local" + _target.IdentifierName.CapitalizeFirstLetter();`
	`66`	`+ if (VariableNameIsValid(newName))`
	`67`	`+ {`
	`68`	`+ return newName;`
	`69`	`+ }`
`69`	`70`
`70`		`- //If the initial suggestion is not valid, keep pre-pending x's until it is`
`71`		`- for ( int attempt = 2; attempt < 10; attempt++)`
	`71`	`+ for ( var attempt = 2; attempt < 10; attempt++)`
`72`	`72`	`{`
`73`		`- _localCopyVariableName = "x" + _localCopyVariableName;`
`74`		`- if (VariableNameIsValid(_localCopyVariableName))`
	`73`	`+ var result = newName + attempt;`
	`74`	`+ if (VariableNameIsValid(result))`
`75`	`75`	`{`
`76`		`- return;`
	`76`	`+ return result;`
`77`	`77`	`}`
`78`	`78`	`}`
`79`		`- //if "xxFoo" to "xxxxxxxxxxFoo" isn't unique, give up and go with the original suggestion.`
`80`		`- //The QuickFix will leave the code as-is unless it receives a name that is free of conflicts`
`81`		`- _localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();`
	`79`	`+ return newName;`
`82`	`80`	`}`
`83`	`81`
`84`	`82`	`private bool VariableNameIsValid(string variableName)`
`85`	`83`	`{`
`86`	`84`	`var validator = new VariableNameValidator(variableName);`
`87`	`85`	`return validator.IsValidName()`
`88`		`- && !_variableNamesAccessibleToProcedureContext`
`89`		`- .Any(name => name.Equals(variableName, System.StringComparison.InvariantCultureIgnoreCase));`
	`86`	`+ && !_forbiddenNames.Any(name => name.Equals(variableName, System.StringComparison.InvariantCultureIgnoreCase));`
`90`	`87`	`}`
`91`	`88`
`92`	`89`	`private void ReplaceAssignedByValParameterReferences()`
`93`	`90`	`{`
`94`	`91`	`var module = Selection.QualifiedName.Component.CodeModule;`
`95`		`- foreach (IdentifierReference identifierReference in _target.References)`
	`92`	`+ foreach (var identifierReference in _target.References)`
`96`	`93`	`{`
`97`	`94`	`module.ReplaceIdentifierReferenceName(identifierReference, _localCopyVariableName);`
`98`	`95`	`}`
`99`	`96`	`}`
`100`	`97`
`101`	`98`	`private void InsertLocalVariableDeclarationAndAssignment()`
`102`	`99`	`{`
`103`		`- var blocks = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent);`
	`100`	`+ var block = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent).FirstOrDefault();`
	`101`	`+ if (block == null)`
	`102`	`+ {`
	`103`	`+ return;`
	`104`	`+ }`
	`105`	`+`
`104`	`106`	`string[] lines = { BuildLocalCopyDeclaration(), BuildLocalCopyAssignment() };`
`105`	`107`	`var module = Selection.QualifiedName.Component.CodeModule;`
`106`		`- module.InsertLines(blocks.FirstOrDefault().Start.Line, lines);`
	`108`	`+ module.InsertLines(block.Start.Line, lines);`
`107`	`109`	`}`
`108`	`110`
`109`	`111`	`private string BuildLocalCopyDeclaration()`
`110`	`112`	`{`
`111`		`- return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As`
`112`		`- + " " + _target.AsTypeName;`
	`113`	`+ return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As + " " + _target.AsTypeName;`
`113`	`114`	`}`
`114`	`115`
`115`	`116`	`private string BuildLocalCopyAssignment()`
`116`	`117`	`{`
`117`		`- return (SymbolList.ValueTypes.Contains(_target.AsTypeName) ? string.Empty : Tokens.Set + " ")`
	`118`	`+ return (_target.AsTypeDeclaration is ClassModuleDeclaration ? Tokens.Set + " " : string.Empty)`
`118`	`119`	`+ _localCopyVariableName + " = " + _target.IdentifierName;`
`119`	`120`	`}`
`120`	`121`
`121`		`- private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleContext)`
	`122`	`+ private IEnumerable<string> GetIdentifierNamesAccessibleToProcedureContext(RuleContext ruleContext)`
`122`	`123`	`{`
`123`	`124`	`var allIdentifiers = new HashSet<string>();`
`124`	`125`
`@@ -137,29 +138,31 @@ private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleCo`
`137`	`138`	`return allIdentifiers.ToArray();`
`138`	`139`	`}`
`139`	`140`
`140`		`- private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContexts)`
	`141`	`+ private IEnumerable<string> GetIdentifierNames(IEnumerable<RuleContext> ruleContexts)`
`141`	`142`	`{`
`142`	`143`	`var identifiers = new HashSet<string>();`
`143`		`- foreach (RuleContext ruleContext in ruleContexts)`
	`144`	`+ foreach (var identifiersForThisContext in ruleContexts.Select(GetIdentifierNames))`
`144`	`145`	`{`
`145`		`- var identifiersForThisContext = GetIdentifierNames(ruleContext);`
`146`	`146`	`identifiers.UnionWith(identifiersForThisContext);`
`147`	`147`	`}`
`148`	`148`	`return identifiers;`
`149`	`149`	`}`
`150`	`150`
`151`		`- private HashSet<string> GetIdentifierNames(RuleContext ruleContext)`
	`151`	`+ private static HashSet<string> GetIdentifierNames(RuleContext ruleContext)`
`152`	`152`	`{`
	`153`	`+ // note: this looks like something that's already handled somewhere else...`
	`154`	`+`
`153`	`155`	`//Recursively work through the tree to get all IdentifierContexts`
`154`	`156`	`var results = new HashSet<string>();`
`155`		`- var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item);`
	`157`	`+ var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item).ToArray();`
`156`	`158`	`var children = GetChildren(ruleContext);`
`157`	`159`
`158`		`- foreach (IParseTree child in children)`
	`160`	`+ foreach (var child in children)`
`159`	`161`	`{`
`160`		`- if (child is VBAParser.IdentifierContext)`
	`162`	`+ var context = child as VBAParser.IdentifierContext;`
	`163`	`+ if (context != null)`
`161`	`164`	`{`
`162`		`- var childName = Identifier.GetName((VBAParser.IdentifierContext)child);`
	`165`	`+ var childName = Identifier.GetName(context);`
`163`	`166`	`if (!tokenValues.Contains(childName))`
`164`	`167`	`{`
`165`	`168`	`results.Add(childName);`
`@@ -176,12 +179,12 @@ private HashSet<string> GetIdentifierNames(RuleContext ruleContext)`
`176`	`179`	`return results;`
`177`	`180`	`}`
`178`	`181`
`179`		`- private static List<IParseTree> GetChildren(RuleContext ruleCtx)`
	`182`	`+ private static IEnumerable<IParseTree> GetChildren(IParseTree tree)`
`180`	`183`	`{`
`181`	`184`	`var result = new List<IParseTree>();`
`182`		`- for (int index = 0; index < ruleCtx.ChildCount; index++)`
	`185`	`+ for (var index = 0; index < tree.ChildCount; index++)`
`183`	`186`	`{`
`184`		`- result.Add(ruleCtx.GetChild(index));`
	`187`	`+ result.Add(tree.GetChild(index));`
`185`	`188`	`}`
`186`	`189`	`return result;`
`187`	`190`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ namespace Rubberduck.Inspections.Results`
`11`	`11`	`public class AssignedByValParameterInspectionResult : InspectionResultBase`
`12`	`12`	`{`
`13`	`13`	`private IEnumerable<QuickFixBase> _quickFixes;`
`14`		`- private IAssignedByValParameterQuickFixDialogFactory _dialogFactory;`
	`14`	`+ private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;`
`15`	`15`
`16`	`16`	`public AssignedByValParameterInspectionResult(IInspection inspection, Declaration target, IAssignedByValParameterQuickFixDialogFactory dialogFactory)`
`17`	`17`	`: base(inspection, target)`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,8 @@ public VariableNameValidator() { }`
`15`	`15`	`private const int MinVariableNameLength = 3;`
`16`	`16`
`17`	`17`	`/** Meaningful Name Characteristics **********/`
`18`		`- public bool HasVowels`
	`18`	`+`
	`19`	`+ private bool HasVowels`
`19`	`20`	`{`
`20`	`21`	`get`
`21`	`22`	`{`
`@@ -25,7 +26,7 @@ public bool HasVowels`
`25`	`26`	`}`
`26`	`27`	`}`
`27`	`28`
`28`		`- public bool HasConsonants`
	`29`	`+ private bool HasConsonants`
`29`	`30`	`{`
`30`	`31`	`get`
`31`	`32`	`{`
`@@ -35,7 +36,7 @@ public bool HasConsonants`
`35`	`36`	`}`
`36`	`37`	`}`
`37`	`38`
`38`		`- public bool IsSingleRepeatedLetter`
	`39`	`+ private bool IsSingleRepeatedLetter`
`39`	`40`	`{`
`40`	`41`	`get`
`41`	`42`	`{`
`@@ -45,8 +46,8 @@ public bool IsSingleRepeatedLetter`
`45`	`46`	`}`
`46`	`47`	`}`
`47`	`48`
`48`		`- public bool IsTooShort { get { return _identifier.Length < MinVariableNameLength; } }`
`49`		`- public bool EndsWithNumber { get { return char.IsDigit(_identifier.Last()); } }`
	`49`	`+ private bool IsTooShort { get { return _identifier.Length < MinVariableNameLength; } }`
	`50`	`+ private bool EndsWithNumber { get { return char.IsDigit(_identifier.Last()); } }`
`50`	`51`
`51`	`52`	`/** Invalid Name Characteristics **********/`
`52`	`53`	`public bool StartsWithNumber { get { return FirstLetterIsDigit(); } }`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`using System;`
	`2`	`+using System.Collections.Generic;`
`2`	`3`	`using System.Windows.Forms;`
`3`	`4`	`using Rubberduck.Inspections;`
`4`	`5`	`using System.Linq;`
`@@ -7,15 +8,15 @@ namespace Rubberduck.UI.Refactorings`
`7`	`8`	`{`
`8`	`9`	`public partial class AssignedByValParameterQuickFixDialog : Form, IAssignedByValParameterQuickFixDialog`
`9`	`10`	`{`
`10`		`- private string[] _identifierNamesAlreadyDeclared;`
`11`		`- private string _identifierName;`
	`11`	`+ private readonly string _identifierName;`
	`12`	`+ private readonly IEnumerable<string> _forbiddenNames;`
`12`	`13`
`13`		`- internal AssignedByValParameterQuickFixDialog(string identifierName, string declarationType)`
	`14`	`+ internal AssignedByValParameterQuickFixDialog(string identifierName, string declarationType, IEnumerable<string> forbiddenNames)`
`14`	`15`	`{`
`15`	`16`	`InitializeComponent();`
`16`	`17`	`InitializeCaptions(identifierName, declarationType);`
`17`	`18`	`_identifierName = identifierName;`
`18`		`- _identifierNamesAlreadyDeclared = Enumerable.Empty<string>().ToArray();`
	`19`	`+ _forbiddenNames = forbiddenNames;`
`19`	`20`	`}`
`20`	`21`
`21`	`22`	`private void InitializeCaptions(string identifierName, string targetDeclarationType)`
`@@ -47,17 +48,12 @@ public string NewName`
`47`	`48`	`SetControlsProperties();`
`48`	`49`	`}`
`49`	`50`	`}`
`50`		`- public string[] IdentifierNamesAlreadyDeclared`
`51`		`- {`
`52`		`- get { return _identifierNamesAlreadyDeclared; }`
`53`		`- set { _identifierNamesAlreadyDeclared = value; }`
`54`		`- }`
`55`	`51`
`56`	`52`	`private string GetVariableNameFeedback()`
`57`	`53`	`{`
`58`	`54`	`var validator = new VariableNameValidator(NewName);`
`59`	`55`
`60`		`- if (UserInputIsBlank())`
	`56`	`+ if (string.IsNullOrEmpty(NewName))`
`61`	`57`	`{`
`62`	`58`	`return string.Empty;`
`63`	`59`	`}`
`@@ -73,11 +69,11 @@ private string GetVariableNameFeedback()`
`73`	`69`	`{`
`74`	`70`	`return string.Format(RubberduckUI.AssignedByValDialog_ReservedKeywordFormat, NewName);`
`75`	`71`	`}`
`76`		`- if (IsByValIdentifier())`
	`72`	`+ if (NewName.Equals(_identifierName, StringComparison.OrdinalIgnoreCase))`
`77`	`73`	`{`
`78`	`74`	`return string.Format(RubberduckUI.AssignedByValDialog_IsByValIdentifierFormat, NewName);`
`79`	`75`	`}`
`80`		`- if (NewNameAlreadyUsed())`
	`76`	`+ if (_forbiddenNames.Any(name => name.Equals(NewName, StringComparison.OrdinalIgnoreCase)))`
`81`	`77`	`{`
`82`	`78`	`return string.Format(RubberduckUI.AssignedByValDialog_NewNameAlreadyUsedFormat, NewName);`
`83`	`79`	`}`
`@@ -91,27 +87,10 @@ private string GetVariableNameFeedback()`
`91`	`87`	`private void SetControlsProperties()`
`92`	`88`	`{`
`93`	`89`	`var validator = new VariableNameValidator(NewName);`
`94`		`- var userInputIsValid = validator.IsValidName() && !NewNameAlreadyUsed();`
`95`		`- OkButton.Visible = userInputIsValid;`
`96`		`- OkButton.Enabled = userInputIsValid;`
`97`		`- InvalidNameValidationIcon.Visible = !userInputIsValid;`
`98`		`- }`
`99`		`-`
`100`		`- private bool UserInputIsBlank()`
`101`		`- {`
`102`		`- return NewName.Equals(string.Empty);`
`103`		`- }`
`104`		`-`
`105`		`- private bool IsByValIdentifier()`
`106`		`- {`
`107`		`- return NewName.Equals(_identifierName, StringComparison.OrdinalIgnoreCase);`
`108`		`- }`
`109`		`-`
`110`		`- private bool NewNameAlreadyUsed()`
`111`		`- {`
`112`		`- //Comparison needs to be case-insensitive, or VBE will often change an existing`
`113`		`- //same-spelling local variable's casing to conform with the NewName`
`114`		`- return _identifierNamesAlreadyDeclared.Any(n => n.Equals(NewName, StringComparison.OrdinalIgnoreCase));`
	`90`	`+ var isValid = validator.IsValidName() && !_forbiddenNames.Any(name => name.Equals(NewName, StringComparison.OrdinalIgnoreCase));`
	`91`	`+ OkButton.Visible = isValid;`
	`92`	`+ OkButton.Enabled = isValid;`
	`93`	`+ InvalidNameValidationIcon.Visible = !isValid;`
`115`	`94`	`}`
`116`	`95`	`}`
`117`	`96`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,13 @@`
`1`	`1`
	`2`	`+using System.Collections.Generic;`
	`3`	`+`
`2`	`4`	`namespace Rubberduck.UI.Refactorings`
`3`	`5`	`{`
`4`	`6`	`public class AssignedByValParameterQuickFixDialogFactory : IAssignedByValParameterQuickFixDialogFactory`
`5`	`7`	`{`
`6`		`- IAssignedByValParameterQuickFixDialog IAssignedByValParameterQuickFixDialogFactory.Create(string identifier, string identifierType)`
	`8`	`+ IAssignedByValParameterQuickFixDialog IAssignedByValParameterQuickFixDialogFactory.Create(string identifier, string identifierType, IEnumerable<string> forbiddenNames)`
`7`	`9`	`{`
`8`		`- return new AssignedByValParameterQuickFixDialog(identifier, identifierType);`
	`10`	`+ return new AssignedByValParameterQuickFixDialog(identifier, identifierType, forbiddenNames);`
`9`	`11`	`}`
`10`	`12`	`}`
`11`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,5 @@ public interface IAssignedByValParameterQuickFixDialog : IDialogView`
`11`	`11`	`{`
`12`	`12`	`DialogResult DialogResult { get;}`
`13`	`13`	`string NewName { get; set; }`
`14`		`- string[] IdentifierNamesAlreadyDeclared { get; set; }`
`15`	`14`	`}`
`16`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,6 @@ namespace Rubberduck.UI.Refactorings`
`10`	`10`	`{`
`11`	`11`	`public interface IAssignedByValParameterQuickFixDialogFactory`
`12`	`12`	`{`
`13`		`- IAssignedByValParameterQuickFixDialog Create(string identifier, string identifierType);`
	`13`	`+ IAssignedByValParameterQuickFixDialog Create(string identifier, string identifierType, IEnumerable<string> forbiddenNames);`
`14`	`14`	`}`
`15`	`15`	`}`