Merge pull request #3117 from MDoerner/Fix3115

retailcoder · web-flow · commit f62c7a04b0f0 · 2017-07-11T22:47:54.000-04:00
Cache Invalidation for Projects Outside of the Sinks
diff --git a/RetailCoder.VBE/Navigation/CodeExplorer/CodeExplorerViewModel.cs b/RetailCoder.VBE/Navigation/CodeExplorer/CodeExplorerViewModel.cs
@@ -254,7 +254,7 @@ private void HandleStateChanged(object sender, ParserStateEventArgs e)
                 return;
             }
 
-            var userDeclarations = _state.AllUserDeclarations
+            var userDeclarations = _state.DeclarationFinder.AllUserDeclarations
                 .GroupBy(declaration => declaration.ProjectId)
                 .ToList();
 
diff --git a/Rubberduck.Parsing/Symbols/DeclarationFinder.cs b/Rubberduck.Parsing/Symbols/DeclarationFinder.cs
@@ -376,6 +376,8 @@ public IEnumerable<Declaration> UserDeclarations(DeclarationType type)
                     .SelectMany(item => item.Value);
         }
 
+        public IEnumerable<Declaration> AllUserDeclarations => _userDeclarationsByType.AllValues();
+
         public IEnumerable<Declaration> BuiltInDeclarations(DeclarationType type)
         {
             List<Declaration> result;
@@ -386,6 +388,8 @@ public IEnumerable<Declaration> BuiltInDeclarations(DeclarationType type)
                     .SelectMany(item => item.Value);
         }
 
+        public IEnumerable<Declaration> AllBuiltInDeclarations => _builtInDeclarationsByType.Value.AllValues();
+
         public IEnumerable<Declaration> DeclarationsWithType(DeclarationType type)
         {
             return BuiltInDeclarations(type).Concat(UserDeclarations(type));
diff --git a/Rubberduck.Parsing/VBA/ParseCoordinator.cs b/Rubberduck.Parsing/VBA/ParseCoordinator.cs
@@ -7,6 +7,7 @@
 using System.Diagnostics;
 using System.Linq;
 using NLog;
+using Rubberduck.VBEditor.SafeComWrappers.Abstract;
 
 namespace Rubberduck.Parsing.VBA
 {
@@ -241,14 +242,19 @@ private void ExecuteCommonParseActivities(IReadOnlyCollection<QualifiedModuleNam
                 token.ThrowIfCancellationRequested();
 
                 _parsingStageService.ResolveDeclarations(toParse, token);
-                RefreshDeclarationFinder();
             }
 
             if (token.IsCancellationRequested || State.Status >= ParserState.Error)
             {
                 throw new OperationCanceledException(token);
             }
 
+            //We need to refresh the DeclarationFinder before the handlers for ResolvedDeclarations run no matter 
+            //whether we parsed or resolved something because modules not referenced by any remeining module might
+            //have been removed. E.g. the CodeExplorer needs this update. 
+            RefreshDeclarationFinder();
+            token.ThrowIfCancellationRequested();
+
             //Explicitly setting the overall state here guarantees that the handlers attached 
             //to the state change to ResolvedDeclarations always run, provided there is no error. 
             State.SetStatusAndFireStateChanged(this, ParserState.ResolvedDeclarations);
@@ -361,12 +367,19 @@ private void ParseAllInternal(object requestor, CancellationToken token)
             var removedModules = RemovedModules(modules);
             token.ThrowIfCancellationRequested();
 
+            var removedProjects = RemovedProjects(_projectManager.Projects);
+            token.ThrowIfCancellationRequested();
+
             var toReResolveReferences = _parsingCacheService.ModulesReferencingAny(removedModules);
             token.ThrowIfCancellationRequested();
 
             CleanUpRemovedComponents(removedModules, token);
             token.ThrowIfCancellationRequested();
 
+            //This must come after the component cleanup because of cache invalidation.
+            CleanUpRemovedProjects(removedProjects);
+            token.ThrowIfCancellationRequested();
+
             var toParse = modules.Where(module => State.IsNewOrModified(module)).ToHashSet();
             token.ThrowIfCancellationRequested();
 
@@ -395,6 +408,20 @@ private void ClearStateCache(IEnumerable<QualifiedModuleName> modules)
             }
         }
 
+        private void CleanUpRemovedProjects(IReadOnlyCollection<Tuple<string,string>> removedProjects)
+        {
+            var removedProjectIds = removedProjects.Select(removedProject => removedProject.Item1);
+            ClearStateCache(removedProjectIds);
+        }
+
+        private void ClearStateCache(IEnumerable<string> projectIds)
+        {
+            foreach (var projectId in projectIds)
+            {
+                State.ClearStateCache(projectId);
+            }
+        }
+
         private IReadOnlyCollection<QualifiedModuleName> RemovedModules(IReadOnlyCollection<QualifiedModuleName> modules)
         {
             var modulesWithModuleDeclarations = State.DeclarationFinder.UserDeclarations(DeclarationType.Module).Select(declaration => declaration.QualifiedName.QualifiedModuleName);
@@ -403,6 +430,14 @@ private IReadOnlyCollection<QualifiedModuleName> RemovedModules(IReadOnlyCollect
             return removedModuledecalrations.ToHashSet().AsReadOnly();
         }
 
+        private IReadOnlyCollection<Tuple<string,string>> RemovedProjects(IReadOnlyCollection<IVBProject> projects)
+        {
+            var projectsWithProjectDeclarations = State.DeclarationFinder.UserDeclarations(DeclarationType.Project).Select(declaration => new Tuple<string,string>(declaration.ProjectId, declaration.ProjectName));
+            var currentlyExistingProjects = projects.Select(project => new Tuple<string, string>(project.ProjectId, project.Name)).ToHashSet();
+            var removedProjects = projectsWithProjectDeclarations.Where(project => !currentlyExistingProjects.Contains(project));
+            return removedProjects.ToHashSet().AsReadOnly();
+        }
+
 
         public void Dispose()
         {
diff --git a/Rubberduck.Parsing/VBA/RubberduckParserState.cs b/Rubberduck.Parsing/VBA/RubberduckParserState.cs
@@ -140,7 +140,6 @@ private void Sinks_ProjectAdded(object sender, ProjectEventArgs e)
             if (!e.Project.VBE.IsInDesignMode) { return; }
 
             Logger.Debug("Project '{0}' was added.", e.ProjectId);
-            RefreshProjects(_vbe); // note side-effect: assigns ProjectId/HelpFile
             OnParseRequested(sender);
         }
 
@@ -149,7 +148,6 @@ private void Sinks_ProjectRemoved(object sender, ProjectEventArgs e)
             if (!e.Project.VBE.IsInDesignMode) { return; }
             
             Debug.Assert(e.ProjectId != null);
-            RemoveProject(e.ProjectId, true);
             OnParseRequested(sender);
         }
 
@@ -164,9 +162,6 @@ private void Sinks_ProjectRenamed(object sender, ProjectRenamedEventArgs e)
 
             Logger.Debug("Project {0} was renamed.", e.ProjectId);
 
-            RemoveProject(e.ProjectId);
-            RefreshProjects(e.Project.VBE);
-
             OnParseRequested(sender);
         }
 
@@ -207,14 +202,26 @@ private void Sinks_ComponentRenamed(object sender, ComponentRenamedEventArgs e)
 
             Logger.Debug("Component '{0}' was renamed to '{1}'.", e.OldName, e.Component.Name);
 
+            //todo: Find out for which situation this drastic (and problematic) cache invalidation has been introduced.
+            if (ComponentIsWorksheet(e))
+            {
+                RemoveProject(e.ProjectId);
+                Logger.Debug("Project '{0}' was removed.", e.Component.Name);
+            }
+
+            OnParseRequested(sender);
+        }
+
+        private bool ComponentIsWorksheet(ComponentRenamedEventArgs e)
+        {
             var componentIsWorksheet = false;
             foreach (var declaration in AllUserDeclarations)
             {
                 if (declaration.ProjectId == e.ProjectId &&
                     declaration.DeclarationType == DeclarationType.ClassModule &&
                     declaration.IdentifierName == e.OldName)
                 {
-                    foreach (var superType in ((ClassModuleDeclaration) declaration).Supertypes)
+                    foreach (var superType in ((ClassModuleDeclaration)declaration).Supertypes)
                     {
                         if (superType.IdentifierName == "Worksheet")
                         {
@@ -227,19 +234,7 @@ private void Sinks_ComponentRenamed(object sender, ComponentRenamedEventArgs e)
                 }
             }
 
-            if (componentIsWorksheet)
-            {
-                RemoveProject(e.ProjectId);
-                Logger.Debug("Project '{0}' was removed.", e.Component.Name);
-
-                RefreshProjects(e.Project.VBE);
-            }
-            else
-            {
-                RemoveRenamedComponent(e.ProjectId, e.OldName);
-            }
-
-            OnParseRequested(sender);
+            return componentIsWorksheet;
         }
 
         public void OnStatusMessageUpdate(string message)
@@ -765,20 +760,20 @@ public void AddUnresolvedMemberDeclaration(UnboundMemberDeclaration declaration)
             }
         }
 
-        private void ClearStateCache(string projectId, bool notifyStateChanged = false)
+        public void ClearStateCache(string projectId, bool notifyStateChanged = false)
         {
             try
             {
-                foreach (var moduleState in _moduleStates)
+                foreach (var moduleState in _moduleStates.Where(moduleState => moduleState.Key.ProjectId == projectId))
                 {
-                    if (moduleState.Key.ProjectId == projectId && moduleState.Key.Component != null)
+                    if (moduleState.Key.Component != null)
                     {
                         while (!ClearStateCache(moduleState.Key.Component))
                         {
                             // until Hell freezes over?
                         }
                     }
-                    else if (moduleState.Key.ProjectId == projectId && moduleState.Key.Component == null)
+                    else if (moduleState.Key.Component == null)
                     {
                         // store project module name
                         var qualifiedModuleName = moduleState.Key;
@@ -790,8 +785,9 @@ private void ClearStateCache(string projectId, bool notifyStateChanged = false)
                     }
                 }
             }
-            catch (COMException)
+            catch (COMException exception)
             {
+                Logger.Error(exception, $"Unexpected COMException while clearing the project with projectId {projectId}. Clearing all modules.");
                 _moduleStates.Clear();
             }
 
@@ -849,28 +845,6 @@ public bool ClearStateCache(QualifiedModuleName module, bool notifyStateChanged
             return success;
         }
 
-        private bool RemoveRenamedComponent(string projectId, string oldComponentName)
-        {
-            var keys = new List<QualifiedModuleName>();
-            foreach (var key in _moduleStates.Keys)
-            {
-                if (key.ComponentName == oldComponentName && key.ProjectId == projectId)
-                {
-                    keys.Add(key);
-                }
-            }
-
-            var success = keys.Count != 0 && RemoveKeysFromCollections(keys);
-
-            if (success)
-            {
-                OnStateChanged(this, ParserState.ResolvedDeclarations);   // trigger test explorer and code explorer updates
-                OnStateChanged(this, ParserState.Ready);   // trigger find all references &c. updates
-            }
-
-            return success;
-        }
-
         private bool RemoveKeysFromCollections(IEnumerable<QualifiedModuleName> keys)
         {
             var success = true;

Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,7 @@ private void HandleStateChanged(object sender, ParserStateEventArgs e)`
`254`	`254`	`return;`
`255`	`255`	`}`
`256`	`256`
`257`		`- var userDeclarations = _state.AllUserDeclarations`
	`257`	`+ var userDeclarations = _state.DeclarationFinder.AllUserDeclarations`
`258`	`258`	`.GroupBy(declaration => declaration.ProjectId)`
`259`	`259`	`.ToList();`
`260`	`260`
Original file line number	Diff line number	Diff line change
`@@ -376,6 +376,8 @@ public IEnumerable<Declaration> UserDeclarations(DeclarationType type)`
`376`	`376`	`.SelectMany(item => item.Value);`
`377`	`377`	`}`
`378`	`378`
	`379`	`+ public IEnumerable<Declaration> AllUserDeclarations => _userDeclarationsByType.AllValues();`
	`380`	`+`
`379`	`381`	`public IEnumerable<Declaration> BuiltInDeclarations(DeclarationType type)`
`380`	`382`	`{`
`381`	`383`	`List<Declaration> result;`
`@@ -386,6 +388,8 @@ public IEnumerable<Declaration> BuiltInDeclarations(DeclarationType type)`
`386`	`388`	`.SelectMany(item => item.Value);`
`387`	`389`	`}`
`388`	`390`
	`391`	`+ public IEnumerable<Declaration> AllBuiltInDeclarations => _builtInDeclarationsByType.Value.AllValues();`
	`392`	`+`
`389`	`393`	`public IEnumerable<Declaration> DeclarationsWithType(DeclarationType type)`
`390`	`394`	`{`
`391`	`395`	`return BuiltInDeclarations(type).Concat(UserDeclarations(type));`
Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,6 @@ private void Sinks_ProjectAdded(object sender, ProjectEventArgs e)`
`140`	`140`	`if (!e.Project.VBE.IsInDesignMode) { return; }`
`141`	`141`
`142`	`142`	`Logger.Debug("Project '{0}' was added.", e.ProjectId);`
`143`		`- RefreshProjects(_vbe); // note side-effect: assigns ProjectId/HelpFile`
`144`	`143`	`OnParseRequested(sender);`
`145`	`144`	`}`
`146`	`145`
`@@ -149,7 +148,6 @@ private void Sinks_ProjectRemoved(object sender, ProjectEventArgs e)`
`149`	`148`	`if (!e.Project.VBE.IsInDesignMode) { return; }`
`150`	`149`
`151`	`150`	`Debug.Assert(e.ProjectId != null);`
`152`		`- RemoveProject(e.ProjectId, true);`
`153`	`151`	`OnParseRequested(sender);`
`154`	`152`	`}`
`155`	`153`
`@@ -164,9 +162,6 @@ private void Sinks_ProjectRenamed(object sender, ProjectRenamedEventArgs e)`
`164`	`162`
`165`	`163`	`Logger.Debug("Project {0} was renamed.", e.ProjectId);`
`166`	`164`
`167`		`- RemoveProject(e.ProjectId);`
`168`		`- RefreshProjects(e.Project.VBE);`
`169`		`-`
`170`	`165`	`OnParseRequested(sender);`
`171`	`166`	`}`
`172`	`167`
`@@ -207,14 +202,26 @@ private void Sinks_ComponentRenamed(object sender, ComponentRenamedEventArgs e)`
`207`	`202`
`208`	`203`	`Logger.Debug("Component '{0}' was renamed to '{1}'.", e.OldName, e.Component.Name);`
`209`	`204`
	`205`	`+ //todo: Find out for which situation this drastic (and problematic) cache invalidation has been introduced.`
	`206`	`+ if (ComponentIsWorksheet(e))`
	`207`	`+ {`
	`208`	`+ RemoveProject(e.ProjectId);`
	`209`	`+ Logger.Debug("Project '{0}' was removed.", e.Component.Name);`
	`210`	`+ }`
	`211`	`+`
	`212`	`+ OnParseRequested(sender);`
	`213`	`+ }`
	`214`	`+`
	`215`	`+ private bool ComponentIsWorksheet(ComponentRenamedEventArgs e)`
	`216`	`+ {`
`210`	`217`	`var componentIsWorksheet = false;`
`211`	`218`	`foreach (var declaration in AllUserDeclarations)`
`212`	`219`	`{`
`213`	`220`	`if (declaration.ProjectId == e.ProjectId &&`
`214`	`221`	`declaration.DeclarationType == DeclarationType.ClassModule &&`
`215`	`222`	`declaration.IdentifierName == e.OldName)`
`216`	`223`	`{`
`217`		`- foreach (var superType in ((ClassModuleDeclaration) declaration).Supertypes)`
	`224`	`+ foreach (var superType in ((ClassModuleDeclaration)declaration).Supertypes)`
`218`	`225`	`{`
`219`	`226`	`if (superType.IdentifierName == "Worksheet")`
`220`	`227`	`{`
`@@ -227,19 +234,7 @@ private void Sinks_ComponentRenamed(object sender, ComponentRenamedEventArgs e)`
`227`	`234`	`}`
`228`	`235`	`}`
`229`	`236`
`230`		`- if (componentIsWorksheet)`
`231`		`- {`
`232`		`- RemoveProject(e.ProjectId);`
`233`		`- Logger.Debug("Project '{0}' was removed.", e.Component.Name);`
`234`		`-`
`235`		`- RefreshProjects(e.Project.VBE);`
`236`		`- }`
`237`		`- else`
`238`		`- {`
`239`		`- RemoveRenamedComponent(e.ProjectId, e.OldName);`
`240`		`- }`
`241`		`-`
`242`		`- OnParseRequested(sender);`
	`237`	`+ return componentIsWorksheet;`
`243`	`238`	`}`
`244`	`239`
`245`	`240`	`public void OnStatusMessageUpdate(string message)`
`@@ -765,20 +760,20 @@ public void AddUnresolvedMemberDeclaration(UnboundMemberDeclaration declaration)`
`765`	`760`	`}`
`766`	`761`	`}`
`767`	`762`
`768`		`- private void ClearStateCache(string projectId, bool notifyStateChanged = false)`
	`763`	`+ public void ClearStateCache(string projectId, bool notifyStateChanged = false)`
`769`	`764`	`{`
`770`	`765`	`try`
`771`	`766`	`{`
`772`		`- foreach (var moduleState in _moduleStates)`
	`767`	`+ foreach (var moduleState in _moduleStates.Where(moduleState => moduleState.Key.ProjectId == projectId))`
`773`	`768`	`{`
`774`		`- if (moduleState.Key.ProjectId == projectId && moduleState.Key.Component != null)`
	`769`	`+ if (moduleState.Key.Component != null)`
`775`	`770`	`{`
`776`	`771`	`while (!ClearStateCache(moduleState.Key.Component))`
`777`	`772`	`{`
`778`	`773`	`// until Hell freezes over?`
`779`	`774`	`}`
`780`	`775`	`}`
`781`		`- else if (moduleState.Key.ProjectId == projectId && moduleState.Key.Component == null)`
	`776`	`+ else if (moduleState.Key.Component == null)`
`782`	`777`	`{`
`783`	`778`	`// store project module name`
`784`	`779`	`var qualifiedModuleName = moduleState.Key;`
`@@ -790,8 +785,9 @@ private void ClearStateCache(string projectId, bool notifyStateChanged = false)`
`790`	`785`	`}`
`791`	`786`	`}`
`792`	`787`	`}`
`793`		`- catch (COMException)`
	`788`	`+ catch (COMException exception)`
`794`	`789`	`{`
	`790`	`+ Logger.Error(exception, $"Unexpected COMException while clearing the project with projectId {projectId}. Clearing all modules.");`
`795`	`791`	`_moduleStates.Clear();`
`796`	`792`	`}`
`797`	`793`
`@@ -849,28 +845,6 @@ public bool ClearStateCache(QualifiedModuleName module, bool notifyStateChanged`
`849`	`845`	`return success;`
`850`	`846`	`}`
`851`	`847`
`852`		`- private bool RemoveRenamedComponent(string projectId, string oldComponentName)`
`853`		`- {`
`854`		`- var keys = new List<QualifiedModuleName>();`
`855`		`- foreach (var key in _moduleStates.Keys)`
`856`		`- {`
`857`		`- if (key.ComponentName == oldComponentName && key.ProjectId == projectId)`
`858`		`- {`
`859`		`- keys.Add(key);`
`860`		`- }`
`861`		`- }`
`862`		`-`
`863`		`- var success = keys.Count != 0 && RemoveKeysFromCollections(keys);`
`864`		`-`
`865`		`- if (success)`
`866`		`- {`
`867`		`- OnStateChanged(this, ParserState.ResolvedDeclarations); // trigger test explorer and code explorer updates`
`868`		`- OnStateChanged(this, ParserState.Ready); // trigger find all references &c. updates`
`869`		`- }`
`870`		`-`
`871`		`- return success;`
`872`		`- }`
`873`		`-`
`874`	`848`	`private bool RemoveKeysFromCollections(IEnumerable<QualifiedModuleName> keys)`
`875`	`849`	`{`
`876`	`850`	`var success = true;`