Move EncapsulateField to the ISelectionService

MDoerner · MDoerner · commit 047fd73ebeac · 2019-02-10T23:08:19.000+01:00
diff --git a/Rubberduck.CodeAnalysis/QuickFixes/EncapsulateFieldQuickFix.cs b/Rubberduck.CodeAnalysis/QuickFixes/EncapsulateFieldQuickFix.cs
@@ -7,22 +7,23 @@
 using Rubberduck.Refactorings.EncapsulateField;
 using Rubberduck.SmartIndenter;
 using Rubberduck.VBEditor.SafeComWrappers.Abstract;
+using Rubberduck.VBEditor.Utility;
 
 namespace Rubberduck.Inspections.QuickFixes
 {
     public sealed class EncapsulateFieldQuickFix : QuickFixBase
     {
         private readonly RubberduckParserState _state;
-        private readonly IVBE _vbe;
+        private readonly ISelectionService _selectionService;
         private readonly IRewritingManager _rewritingManager;
         private readonly IIndenter _indenter;
         private readonly IRefactoringPresenterFactory _factory;
         
-        public EncapsulateFieldQuickFix(RubberduckParserState state, IVBE vbe, IIndenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager)
+        public EncapsulateFieldQuickFix(RubberduckParserState state, IIndenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager, ISelectionService selectionService)
             : base(typeof(EncapsulatePublicFieldInspection))
         {
             _state = state;
-            _vbe = vbe;
+            _selectionService = selectionService;
             _rewritingManager = rewritingManager;
             _indenter = indenter;
             _factory = factory;
@@ -31,7 +32,7 @@ public EncapsulateFieldQuickFix(RubberduckParserState state, IVBE vbe, IIndenter
         //The rewriteSession is optional since it is not used in this particular quickfix because it is a refactoring quickfix.
         public override void Fix(IInspectionResult result, IRewriteSession rewriteSession = null)
         {
-            var refactoring = new EncapsulateFieldRefactoring(_state, _vbe, _indenter, _factory, _rewritingManager);
+            var refactoring = new EncapsulateFieldRefactoring(_state, _indenter, _factory, _rewritingManager, _selectionService);
             refactoring.Refactor(result.Target);
         }
 
diff --git a/Rubberduck.Core/UI/Command/Refactorings/RefactorEncapsulateFieldCommand.cs b/Rubberduck.Core/UI/Command/Refactorings/RefactorEncapsulateFieldCommand.cs
@@ -5,8 +5,8 @@
 using Rubberduck.Refactorings;
 using Rubberduck.Refactorings.EncapsulateField;
 using Rubberduck.SmartIndenter;
-using Rubberduck.UI.Refactorings.EncapsulateField;
 using Rubberduck.VBEditor.SafeComWrappers.Abstract;
+using Rubberduck.VBEditor.Utility;
 
 namespace Rubberduck.UI.Command.Refactorings
 {
@@ -15,30 +15,36 @@ public class RefactorEncapsulateFieldCommand : RefactorCommandBase
     {
         private readonly RubberduckParserState _state;
         private readonly IRewritingManager _rewritingManager;
+        private readonly ISelectionService _selectionService;
         private readonly Indenter _indenter;
         private readonly IRefactoringPresenterFactory _factory;
 
-        public RefactorEncapsulateFieldCommand(IVBE vbe, RubberduckParserState state, Indenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager)
+        public RefactorEncapsulateFieldCommand(IVBE vbe, RubberduckParserState state, Indenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager, ISelectionService selectionService)
             : base(vbe)
         {
             _state = state;
             _rewritingManager = rewritingManager;
             _indenter = indenter;
             _factory = factory;
+            _selectionService = selectionService;
         }
 
         protected override bool EvaluateCanExecute(object parameter)
         {
-            Declaration target;
-            using (var pane = Vbe.ActiveCodePane)
+            //This should come first because it does not require COM access.
+            if (_state.Status != ParserState.Ready)
             {
-                if (pane == null || _state.Status != ParserState.Ready)
-                {
-                    return false;
-                }
+                return false;
+            }
 
-                target = _state.FindSelectedDeclaration(pane);
+            var activeSelection = _selectionService.ActiveSelection();
+            if (!activeSelection.HasValue)
+            {
+                return false;
             }
+
+            var target = _state.DeclarationFinder.FindSelectedDeclaration(activeSelection.Value);
+
             return target != null
                 && target.DeclarationType == DeclarationType.Variable
                 && !target.ParentScopeDeclaration.DeclarationType.HasFlag(DeclarationType.Member)
@@ -47,15 +53,12 @@ protected override bool EvaluateCanExecute(object parameter)
 
         protected override void OnExecute(object parameter)
         {
-            using(var activePane = Vbe.ActiveCodePane)
+            if(!_selectionService.ActiveSelection().HasValue)
             {
-                if (activePane == null || activePane.IsWrappingNullReference)
-                {
-                    return;
-                }
+                return;
             }
 
-            var refactoring = new EncapsulateFieldRefactoring(_state, Vbe, _indenter, _factory, _rewritingManager);
+            var refactoring = new EncapsulateFieldRefactoring(_state, _indenter, _factory, _rewritingManager, _selectionService);
             refactoring.Refactor();
         }
     }
diff --git a/Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs b/Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs
@@ -269,25 +269,14 @@ private IDictionary<Declaration, List<Declaration>> FindAllHandlersByWithEventFi
             return handlersByWithEventsField;
         }
 
-        public Declaration FindSelectedDeclaration(ICodePane activeCodePane)
+        public Declaration FindSelectedDeclaration(QualifiedSelection qualifiedSelection)
         {
-            if (activeCodePane == null || activeCodePane.IsWrappingNullReference)
-            {
-                return null;
-            }
-            
-            var qualifiedSelection = activeCodePane.GetQualifiedSelection();
-            if (!qualifiedSelection.HasValue || qualifiedSelection.Value.Equals(default))
-            {
-                return null;
-            }
-
-            var selection = qualifiedSelection.Value.Selection;
+            var selection = qualifiedSelection.Selection;
 
             // statistically we'll be on an IdentifierReference more often than on a Declaration:
             var matches = _referencesBySelection
-                .Where(kvp => kvp.Key.QualifiedName.Equals(qualifiedSelection.Value.QualifiedName)
-                    && kvp.Key.Selection.ContainsFirstCharacter(qualifiedSelection.Value.Selection))
+                .Where(kvp => kvp.Key.QualifiedName.Equals(qualifiedSelection.QualifiedName)
+                              && kvp.Key.Selection.ContainsFirstCharacter(qualifiedSelection.Selection))
                 .SelectMany(kvp => kvp.Value)
                 .OrderByDescending(reference => reference.Declaration.DeclarationType)
                 .Select(reference => reference.Declaration)
@@ -297,8 +286,8 @@ public Declaration FindSelectedDeclaration(ICodePane activeCodePane)
             if (!matches.Any())
             {
                 matches = _declarationsBySelection
-                    .Where(kvp => kvp.Key.QualifiedName.Equals(qualifiedSelection.Value.QualifiedName)
-                        && kvp.Key.Selection.ContainsFirstCharacter(selection))
+                    .Where(kvp => kvp.Key.QualifiedName.Equals(qualifiedSelection.QualifiedName)
+                                  && kvp.Key.Selection.ContainsFirstCharacter(selection))
                     .SelectMany(kvp => kvp.Value)
                     .OrderByDescending(declaration => declaration.DeclarationType)
                     .Distinct()
@@ -308,7 +297,7 @@ public Declaration FindSelectedDeclaration(ICodePane activeCodePane)
             switch (matches.Length)
             {
                 case 0:
-                    return ModuleDeclaration(qualifiedSelection.Value.QualifiedName);
+                    return ModuleDeclaration(qualifiedSelection.QualifiedName);
 
                 case 1:
                     return matches.Single();
@@ -319,6 +308,22 @@ public Declaration FindSelectedDeclaration(ICodePane activeCodePane)
             }
         }
 
+        public Declaration FindSelectedDeclaration(ICodePane activeCodePane)
+        {
+            if (activeCodePane == null || activeCodePane.IsWrappingNullReference)
+            {
+                return null;
+            }
+
+            var qualifiedSelection = activeCodePane.GetQualifiedSelection();
+            if (!qualifiedSelection.HasValue || qualifiedSelection.Value.Equals(default))
+            {
+                return null;
+            }
+
+            return FindSelectedDeclaration(qualifiedSelection.Value);
+        }
+
         public IEnumerable<Declaration> FreshUndeclared => _newUndeclared.AllValues();
 
         //This does not need a lock because enumerators over a ConcurrentBag uses a snapshot.    
diff --git a/Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs b/Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs
@@ -17,24 +17,24 @@ namespace Rubberduck.Refactorings.EncapsulateField
     public class EncapsulateFieldRefactoring : IRefactoring
     {
         private readonly RubberduckParserState _state;
-        private readonly IVBE _vbe;
         private readonly IIndenter _indenter;
         private readonly IRewritingManager _rewritingManager;
+        private readonly ISelectionService _selectionService;
         private readonly IRefactoringPresenterFactory _factory;
         private EncapsulateFieldModel _model;
         
-        public EncapsulateFieldRefactoring(RubberduckParserState state, IVBE vbe, IIndenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager)
+        public EncapsulateFieldRefactoring(RubberduckParserState state, IIndenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager, ISelectionService selectionService)
         {
             _state = state;
-            _vbe = vbe;
             _indenter = indenter;
             _factory = factory;
             _rewritingManager = rewritingManager;
+            _selectionService = selectionService;
         }
 
         private EncapsulateFieldModel InitializeModel()
         {
-            var selection = _vbe.GetActiveSelection();
+            var selection = _selectionService.ActiveSelection();
 
             if (!selection.HasValue)
             {
@@ -74,29 +74,16 @@ public void Refactor()
 
         public void Refactor(QualifiedSelection target)
         {
-            using (var pane = _vbe.ActiveCodePane)
+            if (!_selectionService.TrySetActiveSelection(target))
             {
-                if (pane == null || pane.IsWrappingNullReference)
-                {
-                    return;
-                }
-                pane.Selection = target.Selection;
+                return;
             }
             Refactor();
         }
 
         public void Refactor(Declaration target)
         {
-            using (var pane = _vbe.ActiveCodePane)
-            {
-                if (pane == null || pane.IsWrappingNullReference)
-                {
-                    return;
-                }
-
-                pane.Selection = target.QualifiedSelection.Selection;
-            }
-            Refactor();
+            Refactor(target.QualifiedSelection);
         }
 
         private void AddProperty(IRewriteSession rewriteSession)
diff --git a/RubberduckTests/Commands/RefactorCommandTests.cs b/RubberduckTests/Commands/RefactorCommandTests.cs
@@ -8,7 +8,9 @@
 using Rubberduck.VBEditor.SafeComWrappers.Abstract;
 using RubberduckTests.Mocks;
 using Rubberduck.Interaction;
+using Rubberduck.Parsing.Rewriter;
 using Rubberduck.Refactorings;
+using Rubberduck.VBEditor.Utility;
 
 namespace RubberduckTests.Commands
 {
@@ -25,8 +27,7 @@ public void EncapsulateField_CanExecute_NullActiveCodePane()
             var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);
             using (state)
             {
-                var factory = new Mock<IRefactoringPresenterFactory>();
-                var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);
+                var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);
                 Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));
             }
         }
@@ -41,9 +42,8 @@ public void EncapsulateField_CanExecute_NonReadyState()
             using (state)
             {
                 state.SetStatusAndFireStateChanged(this, ParserState.ResolvedDeclarations, CancellationToken.None);
-                
-                var factory = new Mock<IRefactoringPresenterFactory>();
-                var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);
+
+                var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);
                 Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));
             }
         }
@@ -61,8 +61,7 @@ Dim d As Boolean
             var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);
             using (state)
             {
-                var factory = new Mock<IRefactoringPresenterFactory>();
-                var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);
+                var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);
                 Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));
             }
         }
@@ -80,8 +79,7 @@ Sub Foo()
             var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);
             using (state)
             {
-                var factory = new Mock<IRefactoringPresenterFactory>();
-                var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);
+                var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);
                 Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));
             }
         }
@@ -99,12 +97,19 @@ Sub Foo()
             var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);
             using (state)
             {
-                var factory = new Mock<IRefactoringPresenterFactory>();
-                var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);
+                var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);
                 Assert.IsTrue(encapsulateFieldCommand.CanExecute(null));
             }
         }
 
+        private RefactorEncapsulateFieldCommand TestRefactorEncapsulateFieldCommand(IVBE vbe, RubberduckParserState state, IRewritingManager rewritingManager)
+        {
+            var factory = new Mock<IRefactoringPresenterFactory>().Object;
+            var selectionService = new Mock<ISelectionService>().Object;
+            return new RefactorEncapsulateFieldCommand(vbe, state, null, factory, rewritingManager, selectionService);
+        }
+
+
         [Category("Commands")]
         [Test]
         public void ExtractInterface_CanExecute_NullActiveCodePane()
diff --git a/RubberduckTests/Refactoring/EncapsulateFieldTests.cs b/RubberduckTests/Refactoring/EncapsulateFieldTests.cs

Original file line number	Diff line number	Diff line change
`@@ -17,24 +17,24 @@ namespace Rubberduck.Refactorings.EncapsulateField`
`17`	`17`	`public class EncapsulateFieldRefactoring : IRefactoring`
`18`	`18`	`{`
`19`	`19`	`private readonly RubberduckParserState _state;`
`20`		`- private readonly IVBE _vbe;`
`21`	`20`	`private readonly IIndenter _indenter;`
`22`	`21`	`private readonly IRewritingManager _rewritingManager;`
	`22`	`+ private readonly ISelectionService _selectionService;`
`23`	`23`	`private readonly IRefactoringPresenterFactory _factory;`
`24`	`24`	`private EncapsulateFieldModel _model;`
`25`	`25`
`26`		`- public EncapsulateFieldRefactoring(RubberduckParserState state, IVBE vbe, IIndenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager)`
	`26`	`+ public EncapsulateFieldRefactoring(RubberduckParserState state, IIndenter indenter, IRefactoringPresenterFactory factory, IRewritingManager rewritingManager, ISelectionService selectionService)`
`27`	`27`	`{`
`28`	`28`	`_state = state;`
`29`		`- _vbe = vbe;`
`30`	`29`	`_indenter = indenter;`
`31`	`30`	`_factory = factory;`
`32`	`31`	`_rewritingManager = rewritingManager;`
	`32`	`+ _selectionService = selectionService;`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`private EncapsulateFieldModel InitializeModel()`
`36`	`36`	`{`
`37`		`- var selection = _vbe.GetActiveSelection();`
	`37`	`+ var selection = _selectionService.ActiveSelection();`
`38`	`38`
`39`	`39`	`if (!selection.HasValue)`
`40`	`40`	`{`
`@@ -74,29 +74,16 @@ public void Refactor()`
`74`	`74`
`75`	`75`	`public void Refactor(QualifiedSelection target)`
`76`	`76`	`{`
`77`		`- using (var pane = _vbe.ActiveCodePane)`
	`77`	`+ if (!_selectionService.TrySetActiveSelection(target))`
`78`	`78`	`{`
`79`		`- if (pane == null \|\| pane.IsWrappingNullReference)`
`80`		`- {`
`81`		`- return;`
`82`		`- }`
`83`		`- pane.Selection = target.Selection;`
	`79`	`+ return;`
`84`	`80`	`}`
`85`	`81`	`Refactor();`
`86`	`82`	`}`
`87`	`83`
`88`	`84`	`public void Refactor(Declaration target)`
`89`	`85`	`{`
`90`		`- using (var pane = _vbe.ActiveCodePane)`
`91`		`- {`
`92`		`- if (pane == null \|\| pane.IsWrappingNullReference)`
`93`		`- {`
`94`		`- return;`
`95`		`- }`
`96`		`-`
`97`		`- pane.Selection = target.QualifiedSelection.Selection;`
`98`		`- }`
`99`		`- Refactor();`
	`86`	`+ Refactor(target.QualifiedSelection);`
`100`	`87`	`}`
`101`	`88`
`102`	`89`	`private void AddProperty(IRewriteSession rewriteSession)`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,9 @@`
`8`	`8`	`using Rubberduck.VBEditor.SafeComWrappers.Abstract;`
`9`	`9`	`using RubberduckTests.Mocks;`
`10`	`10`	`using Rubberduck.Interaction;`
	`11`	`+using Rubberduck.Parsing.Rewriter;`
`11`	`12`	`using Rubberduck.Refactorings;`
	`13`	`+using Rubberduck.VBEditor.Utility;`
`12`	`14`
`13`	`15`	`namespace RubberduckTests.Commands`
`14`	`16`	`{`
`@@ -25,8 +27,7 @@ public void EncapsulateField_CanExecute_NullActiveCodePane()`
`25`	`27`	`var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);`
`26`	`28`	`using (state)`
`27`	`29`	`{`
`28`		`- var factory = new Mock<IRefactoringPresenterFactory>();`
`29`		`- var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);`
	`30`	`+ var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);`
`30`	`31`	`Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));`
`31`	`32`	`}`
`32`	`33`	`}`
`@@ -41,9 +42,8 @@ public void EncapsulateField_CanExecute_NonReadyState()`
`41`	`42`	`using (state)`
`42`	`43`	`{`
`43`	`44`	`state.SetStatusAndFireStateChanged(this, ParserState.ResolvedDeclarations, CancellationToken.None);`
`44`		`-`
`45`		`- var factory = new Mock<IRefactoringPresenterFactory>();`
`46`		`- var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);`
	`45`	`+`
	`46`	`+ var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);`
`47`	`47`	`Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));`
`48`	`48`	`}`
`49`	`49`	`}`
`@@ -61,8 +61,7 @@ Dim d As Boolean`
`61`	`61`	`var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);`
`62`	`62`	`using (state)`
`63`	`63`	`{`
`64`		`- var factory = new Mock<IRefactoringPresenterFactory>();`
`65`		`- var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);`
	`64`	`+ var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);`
`66`	`65`	`Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));`
`67`	`66`	`}`
`68`	`67`	`}`
`@@ -80,8 +79,7 @@ Sub Foo()`
`80`	`79`	`var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);`
`81`	`80`	`using (state)`
`82`	`81`	`{`
`83`		`- var factory = new Mock<IRefactoringPresenterFactory>();`
`84`		`- var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);`
	`82`	`+ var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);`
`85`	`83`	`Assert.IsFalse(encapsulateFieldCommand.CanExecute(null));`
`86`	`84`	`}`
`87`	`85`	`}`
`@@ -99,12 +97,19 @@ Sub Foo()`
`99`	`97`	`var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);`
`100`	`98`	`using (state)`
`101`	`99`	`{`
`102`		`- var factory = new Mock<IRefactoringPresenterFactory>();`
`103`		`- var encapsulateFieldCommand = new RefactorEncapsulateFieldCommand(vbe.Object, state, null, factory.Object, rewritingManager);`
	`100`	`+ var encapsulateFieldCommand = TestRefactorEncapsulateFieldCommand(vbe.Object, state, rewritingManager);`
`104`	`101`	`Assert.IsTrue(encapsulateFieldCommand.CanExecute(null));`
`105`	`102`	`}`
`106`	`103`	`}`
`107`	`104`
	`105`	`+ private RefactorEncapsulateFieldCommand TestRefactorEncapsulateFieldCommand(IVBE vbe, RubberduckParserState state, IRewritingManager rewritingManager)`
	`106`	`+ {`
	`107`	`+ var factory = new Mock<IRefactoringPresenterFactory>().Object;`
	`108`	`+ var selectionService = new Mock<ISelectionService>().Object;`
	`109`	`+ return new RefactorEncapsulateFieldCommand(vbe, state, null, factory, rewritingManager, selectionService);`
	`110`	`+ }`
	`111`	`+`
	`112`	`+`
`108`	`113`	`[Category("Commands")]`
`109`	`114`	`[Test]`
`110`	`115`	`public void ExtractInterface_CanExecute_NullActiveCodePane()`