From bade5bcbc5d93e5d951452e6dd39182cec648f95 Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Sat, 5 Jan 2019 21:30:28 +0100 Subject: [PATCH 1/2] Add a mechanism to ask an inspection result whether it gets invalidated This commit introduces the method ChangesInvalidateResult on the IInspectionResult that returns whether a change of the specified modules makes the result invalid. The base implementation returns true if the module specified on the result gets invalidated or else forwards the request to the inspection to allow inspection specific handling. Declaration- and IdentifierReferenceInspectionResults also get invalidated based on the state they carry. AggregateInspectionResults are always considered invalidated as there is no way to see the individual results. --- .../Inspections/Abstract/InspectionBase.cs | 5 + .../Abstract/InspectionResultBase.cs | 9 +- .../Results/DeclarationInspectionResult.cs | 11 +- .../IdentifierReferenceInspectionResult.cs | 19 +- .../QualifiedContextInspectionResult.cs | 5 +- .../Inspections/AggregateInspectionResult.cs | 3 + .../Inspections/Abstract/IInspection.cs | 6 + .../Inspections/Abstract/IInspectionResult.cs | 2 + .../Inspections/InspectionResultTests.cs | 201 ++++++++++++++++++ 9 files changed, 249 insertions(+), 12 deletions(-) create mode 100644 RubberduckTests/Inspections/InspectionResultTests.cs diff --git a/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionBase.cs b/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionBase.cs index 56129ade2c..9e3b513de2 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionBase.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionBase.cs @@ -166,5 +166,10 @@ public IEnumerable GetInspectionResults(CancellationToken tok _logger.Trace("Intercepted invocation of '{0}.{1}' ran for {2}ms", GetType().Name, nameof(DoGetInspectionResults), _stopwatch.ElapsedMilliseconds); return result; } + + public virtual bool ChangesInvalidateResult(IInspectionResult result, ICollection modifiedModules) + { + return true; + } } } \ No newline at end of file diff --git a/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionResultBase.cs b/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionResultBase.cs index 9d7a861b7a..232f203276 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionResultBase.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Abstract/InspectionResultBase.cs @@ -1,4 +1,5 @@ -using System.IO; +using System.Collections.Generic; +using System.IO; using Antlr4.Runtime; using Rubberduck.Common; using Rubberduck.Parsing.Inspections; @@ -39,6 +40,12 @@ public abstract class InspectionResultBase : IInspectionResult, INavigateSource, public Declaration Target { get; } public dynamic Properties { get; } + public virtual bool ChangesInvalidateResult(ICollection modifiedModules) + { + return modifiedModules.Contains(QualifiedName) + || Inspection.ChangesInvalidateResult(this, modifiedModules); + } + /// /// Gets the information needed to select the target instruction in the VBE. /// diff --git a/Rubberduck.CodeAnalysis/Inspections/Results/DeclarationInspectionResult.cs b/Rubberduck.CodeAnalysis/Inspections/Results/DeclarationInspectionResult.cs index b1094d70d6..fe06244018 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Results/DeclarationInspectionResult.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Results/DeclarationInspectionResult.cs @@ -1,4 +1,5 @@ -using Rubberduck.Inspections.Abstract; +using System.Collections.Generic; +using Rubberduck.Inspections.Abstract; using Rubberduck.Parsing; using Rubberduck.Parsing.Inspections.Abstract; using Rubberduck.Parsing.Symbols; @@ -6,7 +7,7 @@ namespace Rubberduck.Inspections.Results { - internal class DeclarationInspectionResult : InspectionResultBase + public class DeclarationInspectionResult : InspectionResultBase { public DeclarationInspectionResult(IInspection inspection, string description, Declaration target, QualifiedContext context = null, dynamic properties = null) : base(inspection, @@ -31,5 +32,11 @@ internal class DeclarationInspectionResult : InspectionResultBase ? target.QualifiedName : GetQualifiedMemberName(target.ParentDeclaration); } + + public override bool ChangesInvalidateResult(ICollection modifiedModules) + { + return modifiedModules.Contains(Target.QualifiedModuleName) + || base.ChangesInvalidateResult(modifiedModules); + } } } diff --git a/Rubberduck.CodeAnalysis/Inspections/Results/IdentifierReferenceInspectionResult.cs b/Rubberduck.CodeAnalysis/Inspections/Results/IdentifierReferenceInspectionResult.cs index 0ced841176..046c3fe2aa 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Results/IdentifierReferenceInspectionResult.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Results/IdentifierReferenceInspectionResult.cs @@ -1,4 +1,5 @@ -using System.Linq; +using System.Collections.Generic; +using System.Linq; using Rubberduck.Inspections.Abstract; using Rubberduck.Parsing; using Rubberduck.Parsing.Inspections.Abstract; @@ -8,24 +9,30 @@ namespace Rubberduck.Inspections.Results { - internal class IdentifierReferenceInspectionResult : InspectionResultBase + public class IdentifierReferenceInspectionResult : InspectionResultBase { - public IdentifierReferenceInspectionResult(IInspection inspection, string description, RubberduckParserState state, IdentifierReference reference, dynamic properties = null) : + public IdentifierReferenceInspectionResult(IInspection inspection, string description, IDeclarationFinderProvider declarationFinderProvider, IdentifierReference reference, dynamic properties = null) : base(inspection, description, reference.QualifiedModuleName, reference.Context, reference.Declaration, new QualifiedSelection(reference.QualifiedModuleName, reference.Context.GetSelection()), - GetQualifiedMemberName(state, reference), + GetQualifiedMemberName(declarationFinderProvider, reference), (object)properties) { } - private static QualifiedMemberName? GetQualifiedMemberName(RubberduckParserState state, IdentifierReference reference) + private static QualifiedMemberName? GetQualifiedMemberName(IDeclarationFinderProvider declarationFinderProvider, IdentifierReference reference) { - var members = state.DeclarationFinder.Members(reference.QualifiedModuleName); + var members = declarationFinderProvider.DeclarationFinder.Members(reference.QualifiedModuleName); return members.SingleOrDefault(m => reference.Context.IsDescendentOf(m.Context))?.QualifiedName; } + + public override bool ChangesInvalidateResult(ICollection modifiedModules) + { + return modifiedModules.Contains(Target.QualifiedModuleName) + || base.ChangesInvalidateResult(modifiedModules); + } } } diff --git a/Rubberduck.CodeAnalysis/Inspections/Results/QualifiedContextInspectionResult.cs b/Rubberduck.CodeAnalysis/Inspections/Results/QualifiedContextInspectionResult.cs index f94c15a07f..eec677f2d1 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Results/QualifiedContextInspectionResult.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Results/QualifiedContextInspectionResult.cs @@ -5,7 +5,7 @@ namespace Rubberduck.Inspections.Results { - internal class QualifiedContextInspectionResult : InspectionResultBase + public class QualifiedContextInspectionResult : InspectionResultBase { public QualifiedContextInspectionResult(IInspection inspection, string description, QualifiedContext context, dynamic properties = null) : base(inspection, @@ -16,7 +16,6 @@ internal class QualifiedContextInspectionResult : InspectionResultBase new QualifiedSelection(context.ModuleName, context.Context.GetSelection()), context.MemberName, (object)properties) - { - } + {} } } diff --git a/Rubberduck.Core/UI/Inspections/AggregateInspectionResult.cs b/Rubberduck.Core/UI/Inspections/AggregateInspectionResult.cs index 902f97238b..59447de1a9 100644 --- a/Rubberduck.Core/UI/Inspections/AggregateInspectionResult.cs +++ b/Rubberduck.Core/UI/Inspections/AggregateInspectionResult.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Antlr4.Runtime; using Rubberduck.Parsing.Inspections.Abstract; using Rubberduck.Parsing.Symbols; @@ -29,6 +30,8 @@ public AggregateInspectionResult(IInspectionResult firstResult, int count) public dynamic Properties => throw new InvalidOperationException(); + public virtual bool ChangesInvalidateResult(ICollection modifiedModules) => true; + public int CompareTo(IInspectionResult other) { if (other == this) diff --git a/Rubberduck.Parsing/Inspections/Abstract/IInspection.cs b/Rubberduck.Parsing/Inspections/Abstract/IInspection.cs index 188a1aaad9..34e5343deb 100644 --- a/Rubberduck.Parsing/Inspections/Abstract/IInspection.cs +++ b/Rubberduck.Parsing/Inspections/Abstract/IInspection.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Threading; +using Rubberduck.VBEditor; namespace Rubberduck.Parsing.Inspections.Abstract { @@ -15,5 +16,10 @@ public interface IInspection : IInspectionModel, IComparable, IComp /// /// Returns inspection results, if any. IEnumerable GetInspectionResults(CancellationToken token); + + /// + /// Specifies whether an inspection result is deemed invalid after the specified modules have changed. + /// + bool ChangesInvalidateResult(IInspectionResult result, ICollection modifiedModules); } } diff --git a/Rubberduck.Parsing/Inspections/Abstract/IInspectionResult.cs b/Rubberduck.Parsing/Inspections/Abstract/IInspectionResult.cs index 45b824e8d5..48cbdedad4 100644 --- a/Rubberduck.Parsing/Inspections/Abstract/IInspectionResult.cs +++ b/Rubberduck.Parsing/Inspections/Abstract/IInspectionResult.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Antlr4.Runtime; using Rubberduck.Parsing.Symbols; using Rubberduck.VBEditor; @@ -14,5 +15,6 @@ public interface IInspectionResult : IComparable, IComparable Declaration Target { get; } ParserRuleContext Context { get; } dynamic Properties { get; } + bool ChangesInvalidateResult(ICollection modifiedModules); } } diff --git a/RubberduckTests/Inspections/InspectionResultTests.cs b/RubberduckTests/Inspections/InspectionResultTests.cs new file mode 100644 index 0000000000..35882b8a4f --- /dev/null +++ b/RubberduckTests/Inspections/InspectionResultTests.cs @@ -0,0 +1,201 @@ +using System.Collections.Generic; +using Moq; +using NUnit.Framework; +using Rubberduck.Inspections.Results; +using Rubberduck.Parsing; +using Rubberduck.Parsing.Annotations; +using Rubberduck.Parsing.Inspections.Abstract; +using Rubberduck.Parsing.Symbols; +using Rubberduck.Parsing.VBA; +using Rubberduck.Parsing.VBA.DeclarationCaching; +using Rubberduck.UI.Inspections; +using Rubberduck.VBEditor; + +namespace RubberduckTests.Inspections +{ + [TestFixture] + public class InspectionResultTests + { + [Test] + public void InspectionResultsAreDeemedInvalidatedIfTheModuleWithTheirQualifiedModuleNameHasBeenModified() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty,"module"); + var context = new QualifiedContext(module, null); + var modifiedModules = new HashSet{module}; + + var inspectionResult = new QualifiedContextInspectionResult(inspectionMock.Object, string.Empty, context); + + Assert.IsTrue(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void InspectionResultsAreDeemedInvalidatedIfTheInspectionDeemsThemInvalidated() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(true); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var context = new QualifiedContext(module, null); + var modifiedModules = new HashSet(); + + var inspectionResult = new QualifiedContextInspectionResult(inspectionMock.Object, string.Empty, context); + + Assert.IsTrue(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void QualifiedContextInspectionResultsAreNotDeemedInvalidatedIfNeitherTheInspectionDeemsThemInvalidatedNorTheirQualifiedModuleNameGetsModified() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var otherModule = new QualifiedModuleName("project", string.Empty, "otherModule"); + var context = new QualifiedContext(module, null); + var modifiedModules = new HashSet{ otherModule }; + + var inspectionResult = new QualifiedContextInspectionResult(inspectionMock.Object, string.Empty, context); + + Assert.IsFalse(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void DeclarationInspectionResultsAreDeemedInvalidatedIfTheirTargetsModuleGetsModified() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var declarationModule = new QualifiedModuleName("project", string.Empty, "declarationModule"); + var declarationMemberName = new QualifiedMemberName(declarationModule, "test"); + var context = new QualifiedContext(module, null); + var declaration = new Declaration(declarationMemberName, null, string.Empty, string.Empty, string.Empty, false, false, + Accessibility.Public, DeclarationType.Constant, null, null, default, false, null); + var modifiedModules = new HashSet{declarationModule}; + + var inspectionResult = new DeclarationInspectionResult(inspectionMock.Object, string.Empty, declaration, context); + + Assert.IsTrue(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void DeclarationInspectionResultsAreNotDeemedInvalidatedIfNeitherTheInspectionDeemsThemInvalidatedNorTheirModuleNorThatOfTheTargetGetModified() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var declarationModule = new QualifiedModuleName("project", string.Empty, "declarationModule"); + var otherModule = new QualifiedModuleName("project", string.Empty, "otherModule"); + var declarationMemberName = new QualifiedMemberName(declarationModule, "test"); + var context = new QualifiedContext(module, null); + var declaration = new Declaration(declarationMemberName, null, string.Empty, string.Empty, string.Empty, false, false, + Accessibility.Public, DeclarationType.Constant, null, null, default, false, null); + var modifiedModules = new HashSet { otherModule }; + + var inspectionResult = new DeclarationInspectionResult(inspectionMock.Object, string.Empty, declaration, context); + + Assert.IsFalse(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void IdentifierRefereneceInspectionResultsAreDeemedInvalidatedIfTheModuleOfTheirReferencedDeclarationGetsModified() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var declarationModule = new QualifiedModuleName("project", string.Empty, "declarationModule"); + var declarationMemberName = new QualifiedMemberName(declarationModule, "test"); + var declaration = new Declaration(declarationMemberName, null, string.Empty, string.Empty, string.Empty, false, false, + Accessibility.Public, DeclarationType.Constant, null, null, default, false, null); + var identifierReference = new IdentifierReference(module, null, null, "test", default, null, declaration); + var modifiedModules = new HashSet { declarationModule }; + + var declarationFinderProviderMock = new Mock(); + var declaratioFinder = new DeclarationFinder(new List(), new List(), + new List()); + declarationFinderProviderMock.SetupGet(m => m.DeclarationFinder).Returns(declaratioFinder); + var inspectionResult = new IdentifierReferenceInspectionResult(inspectionMock.Object, string.Empty, declarationFinderProviderMock.Object, identifierReference); + + Assert.IsTrue(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void IdentifierReferenceInspectionResultsAreNotDeemedInvalidatedIfNeitherTheInspectionDeemsThemInvalidatedNorTheirModuleNorThatOfTheReferencedDeclarationGetModified() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var declarationModule = new QualifiedModuleName("project", string.Empty, "declarationModule"); + var otherModule = new QualifiedModuleName("project", string.Empty, "otherModule"); + var declarationMemberName = new QualifiedMemberName(declarationModule, "test"); + var declaration = new Declaration(declarationMemberName, null, string.Empty, string.Empty, string.Empty, false, false, + Accessibility.Public, DeclarationType.Constant, null, null, default, false, null); + + var identifierReference = new IdentifierReference(module, null, null, "test", default, null, declaration); + var modifiedModules = new HashSet { otherModule }; + + var declarationFinderProviderMock = new Mock(); + var declaratioFinder = new DeclarationFinder(new List(), new List(), + new List()); + declarationFinderProviderMock.SetupGet(m => m.DeclarationFinder).Returns(declaratioFinder); + var inspectionResult = new IdentifierReferenceInspectionResult(inspectionMock.Object, string.Empty, declarationFinderProviderMock.Object, identifierReference); + + Assert.IsFalse(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + + [Test] + public void AggregateInspectionResultsAreAlwaysDeemedInvalidated() + { + var inspectionMock = new Mock(); + inspectionMock + .Setup(m => + m.ChangesInvalidateResult(It.IsAny(), + It.IsAny>())) + .Returns(false); + + var module = new QualifiedModuleName("project", string.Empty, "module"); + var otherModule = new QualifiedModuleName("project", string.Empty, "otherModule"); + var context = new QualifiedContext(module, null); + var modifiedModules = new HashSet { otherModule }; + + var baseInspectionResult = new QualifiedContextInspectionResult(inspectionMock.Object, string.Empty, context); + var inspectionResult = new AggregateInspectionResult(baseInspectionResult, 42); + + Assert.IsTrue(inspectionResult.ChangesInvalidateResult(modifiedModules)); + } + } +} \ No newline at end of file From d3727946ed6e3a726880f8d33cccd65de6fde079 Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Sat, 5 Jan 2019 22:05:17 +0100 Subject: [PATCH 2/2] Wire up inspection result invalidation in the InspectionResultsViewModel The invalidation is currently only used if the inspections do not get run; running the inspections overwrites the collection anyway. Moreover, currently all results get invalidated unconditionally. --- .../Inspections/InspectionResultsViewModel.cs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/Rubberduck.Core/UI/Inspections/InspectionResultsViewModel.cs b/Rubberduck.Core/UI/Inspections/InspectionResultsViewModel.cs index 0e69570584..fa7a6e0d82 100644 --- a/Rubberduck.Core/UI/Inspections/InspectionResultsViewModel.cs +++ b/Rubberduck.Core/UI/Inspections/InspectionResultsViewModel.cs @@ -14,10 +14,11 @@ using Rubberduck.Parsing.Inspections.Abstract; using Rubberduck.Parsing.UIContext; using Rubberduck.Parsing.VBA; +using Rubberduck.Parsing.VBA.Extensions; using Rubberduck.Settings; using Rubberduck.UI.Command; -using Rubberduck.UI.Controls; using Rubberduck.UI.Settings; +using Rubberduck.VBEditor; namespace Rubberduck.UI.Inspections { @@ -383,6 +384,12 @@ private void HandleStateChanged(object sender, ParserStateEventArgs e) { RefreshInspections(e.Token); } + else + { + //Todo: Find a way to get the actually modified modules in here. + var modifiedModules = _state.DeclarationFinder.AllModules.ToHashSet(); + InvalidateStaleInspectionResults(modifiedModules); + } } private async void RefreshInspections(CancellationToken token) @@ -442,6 +449,18 @@ private async void RefreshInspections(CancellationToken token) LogManager.GetCurrentClassLogger().Trace("Inspections loaded in {0}ms", stopwatch.ElapsedMilliseconds); } + private void InvalidateStaleInspectionResults(ICollection modifiedModules) + { + var staleResults = Results.Where(result => result.ChangesInvalidateResult(modifiedModules)).ToList(); + _uiDispatcher.Invoke(() => + { + foreach (var staleResult in staleResults) + { + Results.Remove(staleResult); + } + }); + } + private void ExecuteQuickFixCommand(object parameter) { var quickFix = parameter as IQuickFix;