Merge pull request #2896 from Hosch250/Issue2884

Rewrite Encapsulate Field

RetailCoder.VBE/Inspections/Abstract/InspectionBase.cs

@@ -104,7 +104,7 @@ protected bool IsIgnoringInspectionResultFor(IVBComponent component, int line)
             }
 
             // VBE 1-based indexing
-            for (var i = line - 1; i >= 1; i--)
+            for (var i = line; i >= 1; i--)
             {
                 var annotation = annotations.SingleOrDefault(a => a.QualifiedSelection.Selection.StartLine == i) as IgnoreAnnotation;
                 if (annotation != null && annotation.InspectionNames.Contains(AnnotationName))

RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs

@@ -103,7 +103,7 @@ private void InsertLocalVariableDeclarationAndAssignment(IModuleRewriter rewrite
                 + (_target.AsTypeDeclaration is ClassModuleDeclaration ? Tokens.Set + " " : string.Empty)
                 + localIdentifier + " = " + _target.IdentifierName;
 
-            rewriter.Insert(((ParserRuleContext)_target.Context.Parent).Stop.TokenIndex + 1, "\r\n" + content);
+            rewriter.InsertBefore(((ParserRuleContext)_target.Context.Parent).Stop.TokenIndex + 1, "\r\n" + content);
         }
     }
 }

RetailCoder.VBE/Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using Rubberduck.Parsing.PostProcessing;
 using Rubberduck.Parsing.Symbols;
@@ -15,6 +16,8 @@ public class EncapsulateFieldRefactoring : IRefactoring
         private readonly IRefactoringPresenterFactory<IEncapsulateFieldPresenter> _factory;
         private EncapsulateFieldModel _model;
 
+        private readonly HashSet<IModuleRewriter> _referenceRewriters = new HashSet<IModuleRewriter>();
+
         public EncapsulateFieldRefactoring(IVBE vbe, IIndenter indenter, IRefactoringPresenterFactory<IEncapsulateFieldPresenter> factory)
         {
             _vbe = vbe;
@@ -35,6 +38,10 @@ public void Refactor()
             AddProperty(rewriter);
 
             rewriter.Rewrite();
+            foreach (var referenceRewriter in _referenceRewriters)
+            {
+                referenceRewriter.Rewrite();
+            }
         }
 
         public void Refactor(QualifiedSelection target)
@@ -58,46 +65,54 @@ private void AddProperty(IModuleRewriter rewriter)
 
             var members = _model.State.DeclarationFinder
                 .Members(_model.TargetDeclaration.QualifiedName.QualifiedModuleName)
-                .OrderBy(declaration => declaration.QualifiedSelection)
-                .ToArray();
+                .OrderBy(declaration => declaration.QualifiedSelection);
 
-            var property = Environment.NewLine + Environment.NewLine + GetPropertyText() + Environment.NewLine;
+            var fields = members.Where(d => d.DeclarationType == DeclarationType.Variable && !d.ParentScopeDeclaration.DeclarationType.HasFlag(DeclarationType.Member)).ToList();
 
-            var lastMember = members.LastOrDefault(m => m.DeclarationType.HasFlag(DeclarationType.Member));
-            if (lastMember == null)
+            var property = Environment.NewLine + Environment.NewLine + GetPropertyText();
+            if (members.Any(m => m.DeclarationType.HasFlag(DeclarationType.Member)))
             {
-                rewriter.Insert(1, property);
+                property += Environment.NewLine;
+            }
+
+            if (_model.TargetDeclaration.Accessibility != Accessibility.Private)
+            {
+                var newField = "Private " + _model.TargetDeclaration.IdentifierName + " As " + _model.TargetDeclaration.AsTypeName;
+                if (fields.Count > 1)
+                {
+                    newField = Environment.NewLine + newField;
+                }
+
+                property = newField + property;
+            }
+
+            if (_model.TargetDeclaration.Accessibility == Accessibility.Private || fields.Count > 1)
+            {
+                rewriter.InsertAfter(fields.Last().Context.Stop.TokenIndex, property);
             }
             else
             {
-                rewriter.Insert(lastMember.Context.Stop.TokenIndex, property);
+                rewriter.InsertBefore(0, property);
             }
         }
 
         private void UpdateReferences()
         {
             foreach (var reference in _model.TargetDeclaration.References)
             {
-                var module = reference.QualifiedModuleName.Component.CodeModule;
-                var oldLine = module.GetLines(reference.Selection.StartLine, 1);
-                oldLine = oldLine.Remove(reference.Selection.StartColumn - 1, reference.Selection.EndColumn - reference.Selection.StartColumn);
-                var newLine = oldLine.Insert(reference.Selection.StartColumn - 1, _model.PropertyName);
+                var rewriter = _model.State.GetRewriter(reference.QualifiedModuleName);
+                rewriter.Replace(reference.Context, _model.PropertyName);
 
-                module.ReplaceLine(reference.Selection.StartLine, newLine);
+                _referenceRewriters.Add(rewriter);
             }
         }
 
         private void SetFieldToPrivate(IModuleRewriter rewriter)
         {
-            var target = _model.TargetDeclaration;
-            if (target.Accessibility == Accessibility.Private)
+            if (_model.TargetDeclaration.Accessibility != Accessibility.Private)
             {
-                return;
+                rewriter.Remove(_model.TargetDeclaration);
             }
-
-            var newField = "Private " + _model.TargetDeclaration.IdentifierName + " As " + _model.TargetDeclaration.AsTypeName + Environment.NewLine;
-
-            rewriter.Replace(target, newField);
         }
 
         private string GetPropertyText()

RetailCoder.VBE/Refactorings/IntroduceField/IntroduceFieldRefactoring.cs

@@ -95,6 +95,8 @@ private void PromoteVariable(IModuleRewriter rewriter, Declaration target)
                     pane.Selection = oldSelection.Value.Selection;
                 }
             }
+
+            rewriter.Rewrite();
         }
 
         private void AddField(IModuleRewriter rewriter, Declaration target)
@@ -105,7 +107,7 @@ private void AddField(IModuleRewriter rewriter, Declaration target)
                 .OrderByDescending(item => item.Selection);
 
             var firstMember = members.FirstOrDefault();
-            rewriter.Insert(firstMember?.Context.Start.TokenIndex ?? 0, content);
+            rewriter.InsertBefore(firstMember?.Context.Start.TokenIndex ?? 0, content);
         }
     }
 }

RetailCoder.VBE/Refactorings/IntroduceParameter/IntroduceParameterRefactoring.cs

@@ -3,6 +3,7 @@
 using System.Windows.Forms;
 using Rubberduck.Common;
 using Rubberduck.Parsing.Grammar;
+using Rubberduck.Parsing.PostProcessing;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
 using Rubberduck.UI;
@@ -18,6 +19,8 @@ public class IntroduceParameterRefactoring : IRefactoring
         private readonly IList<Declaration> _declarations;
         private readonly IMessageBox _messageBox;
 
+        private readonly HashSet<IModuleRewriter> _rewriters = new HashSet<IModuleRewriter>();
+
         private static readonly DeclarationType[] ValidDeclarationTypes =
         {
             DeclarationType.Function,
@@ -94,6 +97,7 @@ private void PromoteVariable(Declaration target)
             }
 
             var rewriter = _state.GetRewriter(target);
+            _rewriters.Add(rewriter);
 
             QualifiedSelection? oldSelection = null;
             var pane = _vbe.ActiveCodePane;
@@ -110,6 +114,11 @@ private void PromoteVariable(Declaration target)
             {
                 pane.Selection = oldSelection.Value.Selection;
             }
+
+            foreach (var tokenRewriter in _rewriters)
+            {
+                tokenRewriter.Rewrite();
+            }
         }
 
         private bool PromptIfMethodImplementsInterface(Declaration targetVariable)
@@ -188,23 +197,24 @@ private void UpdateSignature(Declaration targetMethod, Declaration targetVariabl
         private void AddParameter(Declaration targetMethod, Declaration targetVariable, VBAParser.ArgListContext paramList)
         {
             var rewriter = _state.GetRewriter(targetMethod);
+            _rewriters.Add(rewriter);
 
             var argList = paramList.arg();
             var newParameter = Tokens.ByVal + " " + targetVariable.IdentifierName + " "+ Tokens.As + " " + targetVariable.AsTypeName;
 
             if (!argList.Any())
             {
-                rewriter.Insert(paramList.RPAREN().Symbol.TokenIndex, newParameter);
+                rewriter.InsertBefore(paramList.RPAREN().Symbol.TokenIndex, newParameter);
             }
             else if (targetMethod.DeclarationType != DeclarationType.PropertyLet &&
                      targetMethod.DeclarationType != DeclarationType.PropertySet)
             {
-                rewriter.Insert(paramList.RPAREN().Symbol.TokenIndex, $", {newParameter}");
+                rewriter.InsertBefore(paramList.RPAREN().Symbol.TokenIndex, $", {newParameter}");
             }
             else
             {
                 var lastParam = argList.Last();
-                rewriter.Insert(lastParam.Start.TokenIndex, $"{newParameter}, ");
+                rewriter.InsertBefore(lastParam.Start.TokenIndex, $"{newParameter}, ");
             }
         }
 

Rubberduck.Parsing/PostProcessing/IModuleRewriter.cs

@@ -50,11 +50,18 @@ public interface IModuleRewriter
         void Replace(IToken token, string content);
 
         /// <summary>
-        /// Inserts specified content at the specified token index in the module. Use <see cref="Rewrite"/> method to apply changes.
+        /// Inserts specified content before the specified token index in the module. Use <see cref="Rewrite"/> method to apply changes.
         /// </summary>
         /// <param name="tokenIndex">The index of the insertion point in the module's lexer token stream.</param>
         /// <param name="content">The literal content to insert.</param>
-        void Insert(int tokenIndex, string content);
+        void InsertBefore(int tokenIndex, string content);
+
+        /// <summary>
+        /// Inserts specified content after the specified token index in the module. Use <see cref="Rewrite"/> method to apply changes.
+        /// </summary>
+        /// <param name="tokenIndex">The index of the insertion point in the module's lexer token stream.</param>
+        /// <param name="content">The literal content to insert.</param>
+        void InsertAfter(int tokenIndex, string content);
 
         /// <summary>
         /// Gets the text between specified token positions (inclusive).
@@ -67,5 +74,7 @@ public interface IModuleRewriter
         /// </summary>
         /// <returns></returns>
         string GetText();
+
+        ITokenStream TokenStream { get; }
     }
 }

Rubberduck.Parsing/PostProcessing/ModuleRewriter.cs

@@ -12,6 +12,8 @@ public class ModuleRewriter : IModuleRewriter
         private readonly ICodeModule _module;
         private readonly TokenStreamRewriter _rewriter;
 
+        public ITokenStream TokenStream => _rewriter.TokenStream;
+
         public ModuleRewriter(ICodeModule module, TokenStreamRewriter rewriter)
         {
             _module = module;
@@ -69,11 +71,16 @@ public void Replace(IToken token, string content)
             _rewriter.Replace(token, content);
         }
 
-        public void Insert(int tokenIndex, string content)
+        public void InsertBefore(int tokenIndex, string content)
         {
             _rewriter.InsertBefore(tokenIndex, content);
         }
 
+        public void InsertAfter(int tokenIndex, string content)
+        {
+            _rewriter.InsertAfter(tokenIndex, content);
+        }
+
         public string GetText(int startTokenIndex, int stopTokenIndex)
         {
             return _rewriter.GetText(Interval.Of(startTokenIndex, stopTokenIndex));

Rubberduck.Parsing/Symbols/DeclarationType.cs

@@ -7,63 +7,34 @@ namespace Rubberduck.Parsing.Symbols
     [Flags]
     public enum DeclarationType
     {
-        [DebuggerDisplay("Project")]
         Project = 1 << 0,
-        [DebuggerDisplay("Module")]
         Module = 1 << 1,
-        [DebuggerDisplay("ProceduralModule")]
         ProceduralModule = 1 << 2 | Module,
-        [DebuggerDisplay("ClassModule")]
         ClassModule = 1 << 3 | Module,
-        [DebuggerDisplay("UserForm")]
         UserForm = 1 << 4 | ClassModule,
-        [DebuggerDisplay("Document")]
         Document = 1 << 5 | ClassModule,
-        [DebuggerDisplay("ModuleOption")]
         ModuleOption = 1 << 6,
-        [DebuggerDisplay("Member")]
         Member = 1 << 7,
-        [DebuggerDisplay("Procedure")]
         Procedure = 1 << 8 | Member,
-        [DebuggerDisplay("Function")]
         Function = 1 << 9 | Member,
-        [DebuggerDisplay("Property")]
         Property = 1 << 10 | Member,
-        [DebuggerDisplay("PropertyGet")]
         PropertyGet = 1 << 11 | Property | Function,
-        [DebuggerDisplay("PropertyLet")]
         PropertyLet = 1 << 12 | Property | Procedure,
-        [DebuggerDisplay("PropertySet")]
         PropertySet = 1 << 13 | Property | Procedure,
-        [DebuggerDisplay("Parameter")]
         Parameter = 1 << 14,
-        [DebuggerDisplay("Variable")]
         Variable = 1 << 15,
-        [DebuggerDisplay("Control")]
         Control = 1 << 16 | Variable,
-        [DebuggerDisplay("Constant")]
         Constant = 1 << 17,
-        [DebuggerDisplay("Enumeration")]
         Enumeration = 1 << 18,
-        [DebuggerDisplay("EnumerationMember")]
         EnumerationMember = 1 << 19,
-        [DebuggerDisplay("Event")]
         Event = 1 << 20,
-        [DebuggerDisplay("UserDefinedType")]
         UserDefinedType = 1 << 21,
-        [DebuggerDisplay("UserDefinedTypeMember")]
         UserDefinedTypeMember = 1 << 22,
-        [DebuggerDisplay("LibraryFunction")]
         LibraryFunction = 1 << 23 | Function,
-        [DebuggerDisplay("LibraryProcedure")]
         LibraryProcedure = 1 << 24 | Procedure,
-        [DebuggerDisplay("LineLabel")]
         LineLabel = 1 << 25,
-        [DebuggerDisplay("UnresolvedMember")]
         UnresolvedMember = 1 << 26,
-        [DebuggerDisplay("BracketedExpression")]
         BracketedExpression = 1 << 27,
-        [DebuggerDisplay("ComAlias")]
         ComAlias = 1 << 28
     }
 

RubberduckTests/Refactoring/EncapsulateFieldTests.cs

@@ -493,7 +493,7 @@ public void EncapsulatePublicField_FieldDeclarationHasMultipleFields_MoveLast()
             //Expectation
             const string expectedCode =
 @"Public fizz, _
-    buzz As Boolean         
+buzz As Boolean
 Private bazz As Date
 
 Public Property Get Name() As Date
@@ -571,7 +571,6 @@ End Property
 
             var refactoring = new EncapsulateFieldRefactoring(vbe.Object, CreateIndenter(vbe.Object), factory.Object);
             refactoring.Refactor(qualifiedSelection);
-            refactoring.Refactor(qualifiedSelection);
 
             var rewriter = state.GetRewriter(model.TargetDeclaration);
             Assert.AreEqual(expectedCode, rewriter.GetText());
@@ -726,7 +725,7 @@ Sub Bar(ByVal v As Integer)
             Assert.AreEqual(expectedCode1, rewriter1.GetText());
 
             var rewriter2 = state.GetRewriter(module2.Parent);
-            Assert.AreEqual(expectedCode1, rewriter2.GetText());
+            Assert.AreEqual(expectedCode2, rewriter2.GetText());
         }
 
         [TestMethod]