Invalidate rewrite sessions on checkout of a dirty rewriter

A rewriter can only be dirty on checkout if the code of the corresponding module has been modified since the last parse. Applying such a rewriter will erase all modifications since the last parse and thus probably destroy work by the user. To avoid this, the rewrite sessions will invalidate themselves when they encounter a stale module.

To allow to communicate the reason of the invalidation, IsInvalidated and Invalidate have been replaced by a Status property taking values in the new RewriteSessionState enum. It can only be changed from Valid to some other state, Afterwards, all attempts to set it are ignored. Consequently, the Status shows the original reason for invalidation.

Author: MDoerner

Rubberduck.Parsing/Rewriter/IRewriteSession.cs

@@ -7,8 +7,15 @@ public interface IRewriteSession
     {
         IModuleRewriter CheckOutModuleRewriter(QualifiedModuleName module);
         bool TryRewrite();
-        bool IsInvalidated { get; }
-        void Invalidate();
+        RewriteSessionState Status { get; set; }
         CodeKind TargetCodeKind { get; }
     }
+
+    public enum RewriteSessionState
+    {
+        Valid,
+        RewriteApplied,
+        OtherSessionsRewriteApplied,
+        StaleParseTree
+    }
 }

Rubberduck.Parsing/Rewriter/RewriteSessionBase.cs

@@ -15,7 +15,7 @@ public abstract class RewriteSessionBase : IRewriteSession
         private readonly Func<IRewriteSession, bool> _rewritingAllowed;
 
         protected readonly Logger Logger = LogManager.GetCurrentClassLogger();
-        private readonly object _invalidationLockObject = new object();
+        private readonly object _statusLockObject = new object();
 
         public abstract CodeKind TargetCodeKind { get; }
 
@@ -25,6 +25,27 @@ protected RewriteSessionBase(IRewriterProvider rewriterProvider, Func<IRewriteSe
             _rewritingAllowed = rewritingAllowed;
         }
 
+        private RewriteSessionState _status = RewriteSessionState.Valid;
+        public RewriteSessionState Status
+        {
+            get
+            {
+                lock (_statusLockObject)
+                {
+                    return _status;
+                }
+            }
+            set
+            {
+                lock (_statusLockObject)
+                {
+                    if (_status == RewriteSessionState.Valid)
+                    {
+                        _status = value;
+                    }
+                }
+            }
+        }
 
         public IModuleRewriter CheckOutModuleRewriter(QualifiedModuleName module)
         {
@@ -35,6 +56,13 @@ public IModuleRewriter CheckOutModuleRewriter(QualifiedModuleName module)
 
             rewriter = ModuleRewriter(module);
             CheckedOutModuleRewriters.Add(module, rewriter);
+
+            if (rewriter.IsDirty)
+            {
+                //The parse tree is stale.
+                Status = RewriteSessionState.StaleParseTree;
+            }
+
             return rewriter;
         }
 
@@ -48,9 +76,9 @@ public bool TryRewrite()
             }
 
             //This is thread-safe because, once invalidated, there is no way back.
-            if (IsInvalidated)
+            if (Status != RewriteSessionState.Valid)
             {
-                Logger.Warn("Tried to execute Rewrite on a RewriteSession that was already invalidated.");
+                Logger.Warn($"Tried to execute Rewrite on a RewriteSession that was in the invalid status {Status}.");
                 return false;
             }            
 
@@ -64,25 +92,5 @@ public bool TryRewrite()
         }
 
         protected abstract bool TryRewriteInternal();
-
-        private bool _isInvalidated = false;
-        public bool IsInvalidated
-        {
-            get
-            {
-                lock (_invalidationLockObject)
-                {
-                    return _isInvalidated;
-                }
-            }
-        }
-
-        public void Invalidate()
-        {
-            lock(_invalidationLockObject)
-            {
-                _isInvalidated = true;
-            }
-        }
     }
 }

Rubberduck.Parsing/Rewriter/RewritingManager.cs

@@ -50,6 +50,8 @@ private bool TryAllowExclusiveRewrite(IRewriteSession rewriteSession)
                     return false;
                 }
 
+                rewriteSession.Status = RewriteSessionState.RewriteApplied;
+
                 InvalidateAllSessionsInternal();
                 return true;
             }
@@ -80,13 +82,13 @@ private void InvalidateAllSessionsInternal()
         {
             foreach (var rewriteSession in _activeCodePaneSessions)
             {
-                rewriteSession.Invalidate();
+                rewriteSession.Status = RewriteSessionState.OtherSessionsRewriteApplied;
             }
             _activeCodePaneSessions.Clear();
 
             foreach (var rewriteSession in _activeAttributesSessions)
             {
-                rewriteSession.Invalidate();
+                rewriteSession.Status = RewriteSessionState.OtherSessionsRewriteApplied;
             }
             _activeAttributesSessions.Clear();
         }

RubberduckTests/Rewriter/AttributesRewriteSessionTests.cs

@@ -89,9 +89,9 @@ public void TargetsAttributesCode()
             Assert.AreEqual(CodeKind.AttributesCode, rewriteSession.TargetCodeKind);
         }
 
-        protected override IRewriteSession RewriteSession(IParseManager parseManager, Func<IRewriteSession, bool> rewritingAllowed, out MockRewriterProvider mockProvider)
+        protected override IRewriteSession RewriteSession(IParseManager parseManager, Func<IRewriteSession, bool> rewritingAllowed, out MockRewriterProvider mockProvider, bool rewritersAreDirty = false)
         {
-            mockProvider = new MockRewriterProvider();
+            mockProvider = new MockRewriterProvider(rewritersAreDirty);
             return new AttributesRewriteSession(parseManager, mockProvider, rewritingAllowed);
         }
     }

RubberduckTests/Rewriter/CodePaneRewriteSessionTests.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Linq;
 using Moq;
 using NUnit.Framework;
 using Rubberduck.Parsing.Rewriter;
@@ -41,7 +40,7 @@ public void DoesNotCallRequestsParseAfterRewriteIfInvalidated()
             var module = new QualifiedModuleName("TestProject", string.Empty, "TestModule");
             var otherModule = new QualifiedModuleName("TestProject", string.Empty, "OtherTestModule");
             rewriteSession.CheckOutModuleRewriter(module);
-            rewriteSession.Invalidate();
+            rewriteSession.Status = RewriteSessionState.OtherSessionsRewriteApplied;
             rewriteSession.CheckOutModuleRewriter(otherModule);
 
             rewriteSession.TryRewrite();
@@ -99,9 +98,9 @@ public void TargetsCodePaneCode()
             Assert.AreEqual(CodeKind.CodePaneCode, rewriteSession.TargetCodeKind);
         }
 
-        protected override IRewriteSession RewriteSession(IParseManager parseManager, Func<IRewriteSession, bool> rewritingAllowed, out MockRewriterProvider mockProvider)
+        protected override IRewriteSession RewriteSession(IParseManager parseManager, Func<IRewriteSession, bool> rewritingAllowed, out MockRewriterProvider mockProvider, bool rewritersAreDirty = false)
         {
-            mockProvider = new MockRewriterProvider();
+            mockProvider = new MockRewriterProvider(rewritersAreDirty);
             return new CodePaneRewriteSession(parseManager, mockProvider, rewritingAllowed);
         }
     }