Only save and recover selections for open code panes

Accessing a code pane has the side effect of opening it if it is not open already. To avoid opening windows because of selection recovery, the recovery has to be restricted to modules already open.

Author: MDoerner

Rubberduck.Parsing/Rewriter/AttributesRewriteSession.cs

@@ -28,6 +28,8 @@ protected override bool TryRewriteInternal()
             //The suspension ensures that only one parse gets executed instead of two for each rewritten module.
             GuaranteeReparseAfterRewrite();
             PrimeActiveCodePaneRecovery();
+            //Attribute rewrites close the affected code panes, so we have to recover the open state.
+            PrimeOpenStateRecovery();
 
             var result = _parseManager.OnSuspendParser(this, new[] {ParserState.Ready, ParserState.ResolvedDeclarations}, ExecuteAllRewriters);
             if(result != SuspensionResult.Completed)
@@ -61,17 +63,20 @@ private void PrimeActiveCodePaneRecovery()
             SelectionRecoverer.RecoverActiveCodePaneOnNextParse();
         }
 
+        private void PrimeOpenStateRecovery()
+        {
+            SelectionRecoverer.SaveOpenState(CheckedOutModules);
+            SelectionRecoverer.RecoverOpenStateOnNextParse();
+        }
+
         private void ExecuteAllRewriters()
         {
-            //Attribute rewrites close the affected code panes, so we have to recover the open state. 
-            SelectionRecoverer.SaveOpenState(CheckedOutModuleRewriters.Keys);
             foreach (var module in CheckedOutModuleRewriters.Keys)
             {
                 //We have to mark the modules explicitly as modified because attributes only changes do not alter the code pane code.
                 _parseManager.MarkAsModified(module);
                 CheckedOutModuleRewriters[module].Rewrite();
             }
-            SelectionRecoverer.RecoverOpenState();
         }
     }
 }

Rubberduck.Parsing/Rewriter/RewriteSessionBase.cs

@@ -103,7 +103,7 @@ public bool TryRewrite()
 
         private void PrimeSelectionRecovery()
         {
-            SelectionRecoverer.SaveSelections(CheckedOutModuleRewriters.Keys);
+            SelectionRecoverer.SaveSelections(CheckedOutModules);
 
             foreach (var (module, rewriter) in CheckedOutModuleRewriters)
             {

Rubberduck.Parsing/Rewriter/SelectionRecoverer.cs

@@ -28,7 +28,9 @@ public SelectionRecoverer(ISelectionService selectionService, IParseManager pars
         public void SaveSelections(IEnumerable<QualifiedModuleName> modules)
         {
             _savedSelections.Clear();
-            foreach (var module in modules.Distinct())
+            var openModules = _selectionService.OpenModules();
+
+            foreach (var module in modules.Where(module => openModules.Contains(module)).Distinct())
             {
                 var selection = _selectionService.Selection(module);
                 if (selection.HasValue)
@@ -48,7 +50,10 @@ public void AdjustSavedSelection(QualifiedModuleName module, Selection selection
 
         public void ReplaceSavedSelection(QualifiedModuleName module, Selection replacementSelection)
         {
-            _savedSelections[module] = replacementSelection;
+            if (_savedSelections.ContainsKey(module) || _selectionService.OpenModules().Contains(module))
+            {
+                _savedSelections[module] = replacementSelection;
+            }
         }
 
         public void RecoverSavedSelections()

RubberduckTests/Rewriter/AttributesRewriteSessionTests.cs

@@ -91,18 +91,18 @@ public void TargetsAttributesCode()
 
         [Test]
         [Category("Rewriter")]
-        public void CallsRecoverOpenStateOnRewrite()
+        public void CallsRecoverOpenStateOnNextParseOnRewrite()
         {
             var selectionRecovererMock = new Mock<ISelectionRecoverer>();
-            selectionRecovererMock.Setup(m => m.RecoverOpenState());
+            selectionRecovererMock.Setup(m => m.RecoverOpenStateOnNextParse());
 
             var rewriteSession = RewriteSession(session => true, out var mockRewriterProvider, selectionRecoverer: selectionRecovererMock.Object);
             var module = new QualifiedModuleName("TestProject", string.Empty, "TestModule");
 
             rewriteSession.CheckOutModuleRewriter(module);
             rewriteSession.TryRewrite();
 
-            selectionRecovererMock.Verify(m => m.RecoverOpenState(), Times.Once);
+            selectionRecovererMock.Verify(m => m.RecoverOpenStateOnNextParse(), Times.Once);
         }
 
         [Test]
@@ -125,20 +125,20 @@ public void SavesOpenStateForCheckedOutModulesOnRewrite()
 
         [Test]
         [Category("Rewriter")]
-        public void SavesOpenStateBeforeRestoringItOnRewrite()
+        public void SavesOpenStateBeforeRecoverOpenStateOnNextParseOnRewrite()
         {
             var selectionRecovererMock = new Mock<ISelectionRecoverer>();
             var lastOperation = string.Empty;
             selectionRecovererMock.Setup(m => m.SaveOpenState(It.IsAny<IEnumerable<QualifiedModuleName>>())).Callback(() => lastOperation = "SaveOpenState");
-            selectionRecovererMock.Setup(m => m.RecoverOpenState()).Callback(() => lastOperation = "RecoverOpenState");
+            selectionRecovererMock.Setup(m => m.RecoverOpenStateOnNextParse()).Callback(() => lastOperation = "RecoverOpenStateOnNextParse");
 
             var rewriteSession = RewriteSession(session => true, out var mockRewriterProvider, selectionRecoverer: selectionRecovererMock.Object);
             var module = new QualifiedModuleName("TestProject", string.Empty, "TestModule");
 
             rewriteSession.CheckOutModuleRewriter(module);
             rewriteSession.TryRewrite();
 
-            Assert.AreEqual("RecoverOpenState", lastOperation);
+            Assert.AreEqual("RecoverOpenStateOnNextParse", lastOperation);
         }
 
         [Test]

RubberduckTests/Rewriter/SelectionRecovererTests.cs

@@ -35,6 +35,25 @@ public void SetsExactlySavedSelectionsOnRecoverSelections()
                 m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, It.IsAny<Selection>()), Times.Never);
         }
 
+        [Test]
+        [Category("Rewriting")]
+        public void SavesSelectionsOnlyForOpenCodePanes()
+        {
+            var selectionServiceMock = TestSelectionServiceMock();
+            var parseManager = new Mock<IParseManager>().Object;
+            var selectionRecoverer = new SelectionRecoverer(selectionServiceMock.Object, parseManager);
+
+            selectionServiceMock.Setup(m => m.OpenModules())
+                .Returns(() => _testModuleSelections.Skip(1).Select(qs => qs.QualifiedName).ToList());
+
+            selectionRecoverer.SaveSelections(_testModuleSelections.Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
+
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[0].QualifiedName), Times.Never);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[1].QualifiedName), Times.Once);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[2].QualifiedName), Times.Never);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[3].QualifiedName), Times.Never);
+        }
+
         [Test]
         [Category("Rewriting")]
         public void SetsExactlyLastSavedSelectionsOnRecoverSelectionsAfterMultipleSaves()
@@ -97,41 +116,64 @@ public void SetsReplacementSelectionOnRecoverSelections_SelectionSavedPreviously
 
             selectionRecoverer.SaveSelections(_testModuleSelections
                 .Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
-            selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[2].QualifiedName, selectionReplacement);
+            selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[0].QualifiedName, selectionReplacement);
             selectionRecoverer.RecoverSavedSelections();
 
-            foreach (var qualifiedSelection in _testModuleSelections.Take(2))
-            {
-                selectionServiceMock.Verify(
-                    m => m.TrySetSelection(qualifiedSelection.QualifiedName, qualifiedSelection.Selection), Times.Once);
-            }
-
             selectionServiceMock.Verify(
-                m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, selectionReplacement), Times.Once);
+                m => m.TrySetSelection(_testModuleSelections[0].QualifiedName, selectionReplacement), Times.Once);
+            selectionServiceMock.Verify(
+                m => m.TrySetSelection(_testModuleSelections[1].QualifiedName, _testModuleSelections[1].Selection),
+                Times.Once);
+            selectionServiceMock.Verify(
+                m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, It.IsAny<Selection>()), Times.Never);
         }
 
         [Test]
         [Category("Rewriting")]
-        public void SetsReplacementSelectionOnRecoverSelections_SelectionNotSavedPreviously()
+        public void SetReplacementSelectionOnRecoverSelections_SelectionNotSavedPreviously_ModuleOpen()
         {
             var selectionServiceMock = TestSelectionServiceMock();
             var parseManager = new Mock<IParseManager>().Object;
             var selectionRecoverer = new SelectionRecoverer(selectionServiceMock.Object, parseManager);
 
             var selectionReplacement = new Selection(22, 2, 44, 5);
 
-            selectionRecoverer.SaveSelections(_testModuleSelections
-                .Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
-            selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[0].QualifiedName, selectionReplacement);
+            selectionRecoverer.SaveSelections(_testModuleSelections.Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
+            selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[2].QualifiedName, selectionReplacement);
             selectionRecoverer.RecoverSavedSelections();
 
-            selectionServiceMock.Verify(
-                m => m.TrySetSelection(_testModuleSelections[0].QualifiedName, selectionReplacement), Times.Once);
-            selectionServiceMock.Verify(
-                m => m.TrySetSelection(_testModuleSelections[1].QualifiedName, _testModuleSelections[1].Selection),
-                Times.Once);
-            selectionServiceMock.Verify(
-                m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, It.IsAny<Selection>()), Times.Never);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[0].QualifiedName, _testModuleSelections[0].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[1].QualifiedName, _testModuleSelections[1].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, selectionReplacement), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[3].QualifiedName, It.IsAny<Selection>()), Times.Never);
+        }
+
+        [Test]
+        [Category("Rewriting")]
+        public void DoesNotSaveOrSetReplacementSelectionOnRecoverSelections_SelectionNotSavedPreviously_ModuleNotOpen()
+        {
+            var selectionServiceMock = TestSelectionServiceMock();
+            var parseManager = new Mock<IParseManager>().Object;
+            var selectionRecoverer = new SelectionRecoverer(selectionServiceMock.Object, parseManager);
+
+            selectionServiceMock.Setup(m => m.OpenModules())
+                .Returns(() => _testModuleSelections.Take(2).Select(qs => qs.QualifiedName).ToList());
+
+            var selectionReplacement = new Selection(22, 2, 44, 5);
+
+            selectionRecoverer.SaveSelections(_testModuleSelections.Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
+            selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[2].QualifiedName, selectionReplacement);
+            selectionRecoverer.RecoverSavedSelections();
+
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[0].QualifiedName), Times.Once);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[1].QualifiedName), Times.Once);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[2].QualifiedName), Times.Never);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[3].QualifiedName), Times.Never);
+
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[0].QualifiedName, _testModuleSelections[0].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[1].QualifiedName, _testModuleSelections[1].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, It.IsAny<Selection>()), Times.Never);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[3].QualifiedName, It.IsAny<Selection>()), Times.Never);
         }
 
         [Test]
@@ -288,34 +330,67 @@ public void SetsModifiedSelectionAfterOffsetIsAppliedOnParseAfterRecoverSelectio
                 m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, It.IsAny<Selection>()), Times.Never);
         }
 
+
+
         [Test]
         [Category("Rewriting")]
-        public void SetsReplacementSelectionOnParseAfterRecoverSelectionsOnNextParse_SelectionSavedPreviously()
+        public void SetReplacementSelectionOnNextParseAfterRecoverSelectionsOnNextParse_SelectionNotSavedPreviously_ModuleOpen()
         {
             var selectionServiceMock = TestSelectionServiceMock();
             var parseManagerMock = new Mock<IParseManager>();
             var selectionRecoverer = new SelectionRecoverer(selectionServiceMock.Object, parseManagerMock.Object);
 
             var selectionReplacement = new Selection(22, 2, 44, 5);
 
-            selectionRecoverer.SaveSelections(_testModuleSelections
-                .Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
+            selectionRecoverer.SaveSelections(_testModuleSelections.Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
             selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[2].QualifiedName, selectionReplacement);
-            selectionRecoverer.RecoverSavedSelections();
+            selectionRecoverer.RecoverSavedSelectionsOnNextParse();
 
-            foreach (var qualifiedSelection in _testModuleSelections.Take(2))
-            {
-                selectionServiceMock.Verify(
-                    m => m.TrySetSelection(qualifiedSelection.QualifiedName, qualifiedSelection.Selection), Times.Once);
-            }
+            var stateEventArgs = new ParserStateEventArgs(_stateExpectedToTriggerTheRecovery, ParserState.Pending,
+                CancellationToken.None);
+            parseManagerMock.Raise(m => m.StateChanged += null, stateEventArgs);
 
-            selectionServiceMock.Verify(
-                m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, selectionReplacement), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[0].QualifiedName, _testModuleSelections[0].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[1].QualifiedName, _testModuleSelections[1].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, selectionReplacement), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[3].QualifiedName, It.IsAny<Selection>()), Times.Never);
+        }
+
+        [Test]
+        [Category("Rewriting")]
+        public void DoesNotSaveOrSetReplacementSelectionOnOnNextParseAfterRecoverSelectionsOnNextParse_SelectionNotSavedPreviously_ModuleNotOpen()
+        {
+            var selectionServiceMock = TestSelectionServiceMock();
+            var parseManagerMock = new Mock<IParseManager>();
+            var selectionRecoverer = new SelectionRecoverer(selectionServiceMock.Object, parseManagerMock.Object);
+
+            selectionServiceMock.Setup(m => m.OpenModules())
+                .Returns(() => _testModuleSelections.Take(2).Select(qs => qs.QualifiedName).ToList());
+
+            var selectionReplacement = new Selection(22, 2, 44, 5);
+
+            selectionRecoverer.SaveSelections(_testModuleSelections.Select(qualifiedSelection => qualifiedSelection.QualifiedName).Take(2));
+            selectionRecoverer.ReplaceSavedSelection(_testModuleSelections[2].QualifiedName, selectionReplacement);
+            selectionRecoverer.RecoverSavedSelectionsOnNextParse();
+
+            var stateEventArgs = new ParserStateEventArgs(_stateExpectedToTriggerTheRecovery, ParserState.Pending,
+                CancellationToken.None);
+            parseManagerMock.Raise(m => m.StateChanged += null, stateEventArgs);
+
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[0].QualifiedName), Times.Once);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[1].QualifiedName), Times.Once);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[2].QualifiedName), Times.Never);
+            selectionServiceMock.Verify(m => m.Selection(_testModuleSelections[3].QualifiedName), Times.Never);
+
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[0].QualifiedName, _testModuleSelections[0].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[1].QualifiedName, _testModuleSelections[1].Selection), Times.Once);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[2].QualifiedName, It.IsAny<Selection>()), Times.Never);
+            selectionServiceMock.Verify(m => m.TrySetSelection(_testModuleSelections[3].QualifiedName, It.IsAny<Selection>()), Times.Never);
         }
 
         [Test]
         [Category("Rewriting")]
-        public void SetsReplacementSelectionOnParseAfterRecoverSelectionsOnNextParse_SelectionNotSavedPreviously()
+        public void SetsReplacementSelectionOnParseAfterRecoverSelectionsOnNextParse_SelectionSavedPreviously()
         {
             var selectionServiceMock = TestSelectionServiceMock();
             var parseManagerMock = new Mock<IParseManager>();