From 776904bb0e36d401bc188e37e16d77415a0fbb0a Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Fri, 27 Jan 2017 22:21:04 +0100 Subject: [PATCH 01/12] First refactoring of Parse and ParseAll --- .../VBA/EnumerableExtensions.cs | 5 + Rubberduck.Parsing/VBA/ParseCoordinator.cs | 206 ++++++++---------- .../ComponentTypeExtensionTests.cs | 32 --- 3 files changed, 93 insertions(+), 150 deletions(-) delete mode 100644 RubberduckTests/ComponentTypeExtensionTests.cs diff --git a/Rubberduck.Parsing/VBA/EnumerableExtensions.cs b/Rubberduck.Parsing/VBA/EnumerableExtensions.cs index 8f8cdd7916..64113a9cdb 100644 --- a/Rubberduck.Parsing/VBA/EnumerableExtensions.cs +++ b/Rubberduck.Parsing/VBA/EnumerableExtensions.cs @@ -20,5 +20,10 @@ public static IEnumerable DistinctBy(this IEnumerable source, Fun var hashSet = new HashSet(); return source.Where(item => hashSet.Add(keySelector(item))); } + + public static HashSet ToHashSet(this IEnumerable source) + { + return new HashSet(source); + } } } diff --git a/Rubberduck.Parsing/VBA/ParseCoordinator.cs b/Rubberduck.Parsing/VBA/ParseCoordinator.cs index 480610b80d..18c0e5150d 100644 --- a/Rubberduck.Parsing/VBA/ParseCoordinator.cs +++ b/Rubberduck.Parsing/VBA/ParseCoordinator.cs @@ -26,7 +26,7 @@ namespace Rubberduck.Parsing.VBA public class ParseCoordinator : IParseCoordinator { public RubberduckParserState State { get { return _state; } } - + private readonly ConcurrentDictionary> _currentTasks = new ConcurrentDictionary>(); @@ -91,33 +91,50 @@ public void Parse(CancellationTokenSource token) { State.RefreshProjects(_vbe); - var components = new List(); - foreach (var project in State.Projects) - { - foreach (var component in project.VBComponents) - { - components.Add(component); - } - } + var components = State.Projects.SelectMany(project => project.VBComponents).ToList(); // tests do not fire events when components are removed--clear components - foreach (var tree in State.ParseTrees) - { - State.ClearStateCache(tree.Key); // handle potentially removed components without crashing - } + ClearComponentStateCacheForTests(); SyncComReferences(State.Projects); State.RefreshFinder(_hostApp); - + AddBuiltInDeclarations(); State.RefreshFinder(_hostApp); + SetModuleStates(components, ParserState.Pending); + + // invalidation cleanup should go into ParseAsync? + CleanUpComponentAttributes(components); + + _projectDeclarations.Clear(); + State.ClearBuiltInReferences(); + + ParseComponents(components, token); + + if (token.IsCancellationRequested || State.Status >= ParserState.Error) + { + return; + } + + State.SetStatusAndFireStateChanged(this, ParserState.ResolvedDeclarations); + + ResolveReferences(token.Token); + + State.RebuildSelectionCache(); + + } + + private void SetModuleStates(List components, ParserState parserState) + { foreach (var component in components) { - State.SetModuleState(component, ParserState.Pending); + State.SetModuleState(component, parserState); } + } - // invalidation cleanup should go into ParseAsync? + private void CleanUpComponentAttributes(List components) + { foreach (var key in _componentAttributes.Keys) { if (!components.Contains(key)) @@ -125,10 +142,18 @@ public void Parse(CancellationTokenSource token) _componentAttributes.Remove(key); } } + } - _projectDeclarations.Clear(); - State.ClearBuiltInReferences(); + private void ClearComponentStateCacheForTests() + { + foreach (var tree in State.ParseTrees) + { + State.ClearStateCache(tree.Key); // handle potentially removed components without crashing + } + } + private void ParseComponents(List components, CancellationTokenSource token) + { var parseTasks = new Task[components.Count]; for (var i = 0; i < components.Count; i++) { @@ -147,74 +172,35 @@ public void Parse(CancellationTokenSource token) var qualifiedName = new QualifiedModuleName(components[index]); State.SetModuleState(components[index], ParserState.ResolvingDeclarations); + ResolveDeclarations(qualifiedName.Component, State.ParseTrees.Find(s => s.Key == qualifiedName).Value); }); parseTasks[i].Start(); } - Task.WaitAll(parseTasks); + } - if (State.Status < ParserState.Error) - { - State.SetStatusAndFireStateChanged(this, ParserState.ResolvedDeclarations); - Task.WaitAll(ResolveReferencesAsync(token.Token)); - State.RebuildSelectionCache(); - } + private void ResolveReferences(CancellationToken token) + { + Task.WaitAll(ResolveReferencesAsync(token)); } + /// /// Starts parsing all components of all unprotected VBProjects associated with the VBE-Instance passed to the constructor of this parser instance. /// private void ParseAll(object requestor, CancellationTokenSource token) { State.RefreshProjects(_vbe); - - var components = new List(); - foreach (var project in State.Projects) - { - foreach (IVBComponent component in project.VBComponents) - { - components.Add(component); - } - } - - var componentsRemoved = false; - foreach (var declaration in State.AllUserDeclarations) - { - if (!declaration.DeclarationType.HasFlag(DeclarationType.Module)) - { - continue; - } - - var componentExists = false; - foreach (var component in components) - { - if (component.Name == declaration.ComponentName && - component.Collection.Parent.HelpFile == declaration.ProjectId) - { - componentExists = true; - break; - } - } - - if (!componentExists) - { - componentsRemoved = true; - State.ClearStateCache(declaration.QualifiedName.QualifiedModuleName); - } - } - - var toParse = new List(); - foreach (var component in components) - { - if (State.IsNewOrModified(component)) - { - toParse.Add(component); - } - } - + + var components = State.Projects.SelectMany(project => project.VBComponents).ToList(); + + var componentsRemoved = ClearStateCashForRemovedComponents(components); + + var toParse = components.Where(component => State.IsNewOrModified(component)).ToList(); + if (toParse.Count == 0) { if (componentsRemoved) // trigger UI updates @@ -225,11 +211,8 @@ private void ParseAll(object requestor, CancellationTokenSource token) State.SetStatusAndFireStateChanged(requestor, State.Status); //return; // returning here leaves state in 'ResolvedDeclarations' when a module is removed, which disables refresh } - - foreach (var component in toParse) - { - State.SetModuleState(component, ParserState.Pending); - } + + SetModuleStates(toParse, ParserState.Pending); SyncComReferences(State.Projects); State.RefreshFinder(_hostApp); @@ -238,13 +221,7 @@ private void ParseAll(object requestor, CancellationTokenSource token) State.RefreshFinder(_hostApp); // invalidation cleanup should go into ParseAsync? - foreach (var key in _componentAttributes.Keys) - { - if (!components.Contains(key)) - { - _componentAttributes.Remove(key); - } - } + CleanUpComponentAttributes(components); if (token.IsCancellationRequested) { @@ -254,53 +231,46 @@ private void ParseAll(object requestor, CancellationTokenSource token) _projectDeclarations.Clear(); State.ClearBuiltInReferences(); - var parseTasks = new Task[toParse.Count]; - for (var i = 0; i < toParse.Count; i++) - { - var index = i; - parseTasks[i] = new Task(() => - { - ParseAsync(toParse[index], token).Wait(token.Token); + ParseComponents(toParse, token); - if (token.IsCancellationRequested) - { - return; - } - - if (State.Status == ParserState.Error) { return; } - - var qualifiedName = new QualifiedModuleName(toParse[index]); - - State.SetModuleState(toParse[index], ParserState.ResolvingDeclarations); - - ResolveDeclarations(qualifiedName.Component, - State.ParseTrees.Find(s => s.Key == qualifiedName).Value); - }); - - parseTasks[i].Start(); - } - - if (token.IsCancellationRequested) + if (token.IsCancellationRequested || State.Status >= ParserState.Error) { return; } - Task.WaitAll(parseTasks); + Debug.Assert(State.ParseTrees.Count == components.Count, string.Format("ParserState has {0} parse trees for {1} components.", State.ParseTrees.Count, components.Count)); + + State.SetStatusAndFireStateChanged(requestor, ParserState.ResolvedDeclarations); + + ResolveReferences(token.Token); + + State.RebuildSelectionCache(); + } - if (token.IsCancellationRequested) + /// + /// Clears state cach for removed components. + /// Returns whether components have been removed. + /// + private bool ClearStateCashForRemovedComponents(List components) + { + var removedModuledecalrations = RemovedModuleDeclarations(components); + var componentRemoved = removedModuledecalrations.Any(); + foreach (var declaration in removedModuledecalrations) { - return; + State.ClearStateCache(declaration.QualifiedName.QualifiedModuleName); } + return componentRemoved; + } - if (State.Status < ParserState.Error) - { - Debug.Assert(State.ParseTrees.Count == components.Count, string.Format("ParserState has {0} parse trees for {1} components.", State.ParseTrees.Count, components.Count)); - State.SetStatusAndFireStateChanged(requestor, ParserState.ResolvedDeclarations); - Task.WaitAll(ResolveReferencesAsync(token.Token)); - State.RebuildSelectionCache(); - } + private IEnumerable RemovedModuleDeclarations(List components) + { + var moduleDeclarations = State.AllUserDeclarations.Where(declaration => declaration.DeclarationType.HasFlag(DeclarationType.Module)); + var componentKeys = components.Select(component => new { name = component.Name, projectId = component.Collection.Parent.HelpFile }).ToHashSet(); + var removedModuledecalrations = moduleDeclarations.Where(declaration => !componentKeys.Contains(new { name = declaration.ComponentName, projectId = declaration.ProjectId })); + return removedModuledecalrations; } + private Task[] ResolveReferencesAsync(CancellationToken token) { foreach (var kvp in State.ParseTrees) diff --git a/RubberduckTests/ComponentTypeExtensionTests.cs b/RubberduckTests/ComponentTypeExtensionTests.cs deleted file mode 100644 index 43cfc2a6b5..0000000000 --- a/RubberduckTests/ComponentTypeExtensionTests.cs +++ /dev/null @@ -1,32 +0,0 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; -using Rubberduck.VBEditor.SafeComWrappers; -using Rubberduck.VBEditor.Extensions; - -namespace RubberduckTests -{ - [TestClass] - public class ComponentTypeExtensionTests - { - [TestMethod] - public void ClassReturnsCls() - { - var type = ComponentType.ClassModule; - - Assert.AreEqual(".cls", type.FileExtension()); - } - - [TestMethod] - public void FormReturnsFrm() - { - var type = ComponentType.UserForm; - Assert.AreEqual(".frm", type.FileExtension()); - } - - [TestMethod] - public void StandardReturnsBas() - { - var type = ComponentType.StandardModule; - Assert.AreEqual(".bas", type.FileExtension()); - } - } -} From 1dcc114bfb5a28348a327b389eb0c6c68701b45b Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Sat, 28 Jan 2017 14:33:01 +0100 Subject: [PATCH 02/12] Generate task to capture component parser result to allow to push the state changes after the component parser returned. --- Rubberduck.Parsing/VBA/ParseCoordinator.cs | 42 ++++++++++++++++------ 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/Rubberduck.Parsing/VBA/ParseCoordinator.cs b/Rubberduck.Parsing/VBA/ParseCoordinator.cs index 18c0e5150d..7c4ef3987a 100644 --- a/Rubberduck.Parsing/VBA/ParseCoordinator.cs +++ b/Rubberduck.Parsing/VBA/ParseCoordinator.cs @@ -551,30 +551,50 @@ private void Cancel(bool createNewTokenSource = true) private void ParseAsyncInternal(IVBComponent component, CancellationToken token, TokenStreamRewriter rewriter = null) { + State.SetModuleState(component, ParserState.Parsing); + var preprocessor = _preprocessorFactory(); var parser = new ComponentParseTask(component, preprocessor, _attributeParser, rewriter); - parser.ParseFailure += (sender, e) => + + var finishedParseTask = FinishedComponentParseTask(parser, token); //This runs synchronously. + + if (finishedParseTask.IsFaulted) { - State.SetModuleState(component, ParserState.Error, e.Cause as SyntaxErrorException); - }; - parser.ParseCompleted += (sender, e) => + State.SetModuleState(component, ParserState.Error, finishedParseTask.Exception.InnerException as SyntaxErrorException); + } + else if (finishedParseTask.IsCompleted) { + var result = finishedParseTask.Result; lock (State) + { lock (component) { - State.SetModuleAttributes(component, e.Attributes); - State.AddParseTree(component, e.ParseTree); - State.AddTokenStream(component, e.Tokens); - State.SetModuleComments(component, e.Comments); - State.SetModuleAnnotations(component, e.Annotations); + State.SetModuleAttributes(component, result.Attributes); + State.AddParseTree(component, result.ParseTree); + State.AddTokenStream(component, result.Tokens); + State.SetModuleComments(component, result.Comments); + State.SetModuleAnnotations(component, result.Annotations); // This really needs to go last State.SetModuleState(component, ParserState.Parsed); } + } + } + } + + private Task FinishedComponentParseTask(ComponentParseTask parser, CancellationToken token) + { + var tcs = new TaskCompletionSource(); + parser.ParseFailure += (sender, e) => + { + tcs.SetException(e.Cause); + }; + parser.ParseCompleted += (sender, e) => + { + tcs.SetResult(e); }; - State.SetModuleState(component, ParserState.Parsing); - parser.Start(token); + return tcs.Task; } private readonly ConcurrentDictionary _projectDeclarations = new ConcurrentDictionary(); From 4343e99b8bfa650dca553140bab6b72a48d21607 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 28 Jan 2017 14:25:42 -0600 Subject: [PATCH 03/12] Refactor (fix) MemberNotOnInterfaceInspection to collect Declarations from resolver. --- .../MemberNotOnInterfaceInspection.cs | 66 ++++--------------- .../MemberNotOnInterfaceInspectionResult.cs | 8 +-- Rubberduck.Parsing/Rubberduck.Parsing.csproj | 1 + .../Symbols/DeclarationFinder.cs | 32 +++++++++ .../Symbols/IdentifierReferenceResolver.cs | 19 ++++-- .../Symbols/UnboundMemberDeclaration.cs | 39 +++++++++++ .../MemberNotOnInterfaceInspectionTests.cs | 33 ++++++++-- 7 files changed, 130 insertions(+), 68 deletions(-) create mode 100644 Rubberduck.Parsing/Symbols/UnboundMemberDeclaration.cs diff --git a/RetailCoder.VBE/Inspections/MemberNotOnInterfaceInspection.cs b/RetailCoder.VBE/Inspections/MemberNotOnInterfaceInspection.cs index 112ddecaa5..88379cca05 100644 --- a/RetailCoder.VBE/Inspections/MemberNotOnInterfaceInspection.cs +++ b/RetailCoder.VBE/Inspections/MemberNotOnInterfaceInspection.cs @@ -1,26 +1,15 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; -using Antlr4.Runtime; using Rubberduck.Inspections.Abstract; using Rubberduck.Inspections.Resources; using Rubberduck.Inspections.Results; -using Rubberduck.Parsing; -using Rubberduck.Parsing.Grammar; using Rubberduck.Parsing.Symbols; using Rubberduck.Parsing.VBA; -using Rubberduck.VBEditor; namespace Rubberduck.Inspections { public sealed class MemberNotOnInterfaceInspection : InspectionBase { - private static readonly List InterestingTypes = new List - { - typeof(VBAParser.MemberAccessExprContext), - typeof(VBAParser.WithMemberAccessExprContext) - }; - public MemberNotOnInterfaceInspection(RubberduckParserState state, CodeInspectionSeverity defaultSeverity = CodeInspectionSeverity.Warning) : base(state, defaultSeverity) { @@ -32,51 +21,20 @@ public MemberNotOnInterfaceInspection(RubberduckParserState state, CodeInspectio public override IEnumerable GetInspectionResults() { + var unresolved = State.DeclarationFinder.UnresolvedMemberDeclarations().Where(decl => !IsIgnoringInspectionResultFor(decl, AnnotationName)).ToList(); + var targets = Declarations.Where(decl => decl.AsTypeDeclaration != null && - decl.ParentDeclaration.DeclarationType != DeclarationType.Project && + decl.AsTypeDeclaration.IsBuiltIn && decl.AsTypeDeclaration.DeclarationType == DeclarationType.ClassModule && - ((ClassModuleDeclaration)decl.AsTypeDeclaration).IsExtensible && - decl.References.Any(usage => InterestingTypes.Contains(usage.Context.Parent.GetType()))) - .ToList(); + ((ClassModuleDeclaration)decl.AsTypeDeclaration).IsExtensible) + .SelectMany(decl => decl.References).ToList(); - //Unfortunately finding implemented members is fairly expensive, so group by the return type. - return (from access in targets.GroupBy(t => t.AsTypeDeclaration) - let typeDeclaration = access.Key - let typeMembers = new HashSet(BuiltInDeclarations.Where(d => d.ParentDeclaration != null && d.ParentDeclaration.Equals(typeDeclaration)) - .Select(d => d.IdentifierName) - .Distinct()) - from references in access.Select(usage => usage.References.Where(r => InterestingTypes.Contains(r.Context.Parent.GetType()))) - from reference in references.Where(r => !r.IsIgnoringInspectionResultFor(AnnotationName)) - let identifier = reference.Context.Parent.GetChild(reference.Context.Parent.ChildCount - 1) - where !typeMembers.Contains(identifier.GetText()) - let pseudoDeclaration = CreatePseudoDeclaration((ParserRuleContext) identifier, reference) - where !pseudoDeclaration.Annotations.Any() - select new MemberNotOnInterfaceInspectionResult(this, pseudoDeclaration, (ParserRuleContext) identifier, typeDeclaration)) - .Cast().ToList(); - } - - //Builds a throw-away Declaration for the indentifiers found by the inspection. These aren't (and shouldn't be) created by the parser. - //Used to pass to the InspectionResult to make it selectable. - private static Declaration CreatePseudoDeclaration(ParserRuleContext context, IdentifierReference reference) - { - return new Declaration( - new QualifiedMemberName(reference.QualifiedModuleName, context.GetText()), - null, - null, - string.Empty, - string.Empty, - false, - false, - Accessibility.Implicit, - DeclarationType.Variable, - context, - context.GetSelection(), - false, - null, - true, - null, - null, - true); + return (from access in unresolved + let callingContext = targets.FirstOrDefault(usage => usage.Context.Equals(access.CallingContext)) + where callingContext != null + select + new MemberNotOnInterfaceInspectionResult(this, access, callingContext.Declaration.AsTypeDeclaration)) + .ToList(); } } } diff --git a/RetailCoder.VBE/Inspections/Results/MemberNotOnInterfaceInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/MemberNotOnInterfaceInspectionResult.cs index ab35c62694..0c5c0d3895 100644 --- a/RetailCoder.VBE/Inspections/Results/MemberNotOnInterfaceInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/MemberNotOnInterfaceInspectionResult.cs @@ -9,14 +9,12 @@ namespace Rubberduck.Inspections.Results { public class MemberNotOnInterfaceInspectionResult : InspectionResultBase { - private readonly ParserRuleContext _member; private readonly Declaration _asTypeDeclaration; private IEnumerable _quickFixes; - public MemberNotOnInterfaceInspectionResult(IInspection inspection, Declaration target, ParserRuleContext member, Declaration asTypeDeclaration) + public MemberNotOnInterfaceInspectionResult(IInspection inspection, Declaration target, Declaration asTypeDeclaration) : base(inspection, target) { - _member = member; _asTypeDeclaration = asTypeDeclaration; } @@ -26,14 +24,14 @@ public override IEnumerable QuickFixes { return _quickFixes ?? (_quickFixes = new QuickFixBase[] { - new IgnoreOnceQuickFix(_member, QualifiedSelection, Inspection.AnnotationName) + new IgnoreOnceQuickFix(Target.Context, QualifiedSelection, Inspection.AnnotationName) }); } } public override string Description { - get { return string.Format(InspectionsUI.MemberNotOnInterfaceInspectionResultFormat, _member.GetText(), _asTypeDeclaration.IdentifierName); } + get { return string.Format(InspectionsUI.MemberNotOnInterfaceInspectionResultFormat, Target.IdentifierName, _asTypeDeclaration.IdentifierName); } } } } diff --git a/Rubberduck.Parsing/Rubberduck.Parsing.csproj b/Rubberduck.Parsing/Rubberduck.Parsing.csproj index a482a365a3..b724c9f605 100644 --- a/Rubberduck.Parsing/Rubberduck.Parsing.csproj +++ b/Rubberduck.Parsing/Rubberduck.Parsing.csproj @@ -285,6 +285,7 @@ + diff --git a/Rubberduck.Parsing/Symbols/DeclarationFinder.cs b/Rubberduck.Parsing/Symbols/DeclarationFinder.cs index 7c9ee49c74..9217db2fde 100644 --- a/Rubberduck.Parsing/Symbols/DeclarationFinder.cs +++ b/Rubberduck.Parsing/Symbols/DeclarationFinder.cs @@ -1,5 +1,6 @@ using NLog; using Rubberduck.Parsing.Annotations; +using Rubberduck.Parsing.Binding; using Rubberduck.VBEditor; using System; using System.Collections.Concurrent; @@ -37,6 +38,7 @@ public class DeclarationFinder private readonly ConcurrentDictionary _declarationsByName; private readonly ConcurrentDictionary _declarations; private readonly ConcurrentDictionary> _undeclared; + private readonly ConcurrentBag _unresolved; private readonly ConcurrentDictionary _annotations; private readonly ConcurrentDictionary _parametersByParent; private readonly ConcurrentDictionary _userDeclarationsByType; @@ -94,6 +96,7 @@ public DeclarationFinder(IReadOnlyList declarations, IEnumerable>(new Dictionary>()); + _unresolved = new ConcurrentBag(new List()); _annotationService = new AnnotationService(this); var implementsInstructions = UserDeclarations(DeclarationType.ClassModule).SelectMany(cls => @@ -174,6 +177,14 @@ public IEnumerable UserDeclarations(DeclarationType type) return result; } + public IEnumerable UnresolvedMemberDeclarations() + { + lock (ThreadLock) + { + return _unresolved.ToArray(); + } + } + public IEnumerable FindHandlersForWithEventsField(Declaration field) { Declaration[] result; @@ -485,6 +496,27 @@ public Declaration OnUndeclaredVariable(Declaration enclosingProcedure, string i return undeclaredLocal; } + public void AddUnboundContext(Declaration parentDeclaration, VBAParser.LExprContext context, IBoundExpression withExpression) + { + + //The only forms we care about right now are MemberAccessExprContext or WithMemberAccessExprContext. + var access = (ParserRuleContext)context.GetChild(0) + ?? context.GetChild(0); + if (access == null) + { + return; + } + + var identifier = access.GetChild(0); + var annotations = _annotationService.FindAnnotations(parentDeclaration.QualifiedName.QualifiedModuleName, context.Start.Line); + + var declaration = new UnboundMemberDeclaration(parentDeclaration, identifier, + (access is VBAParser.MemberAccessExprContext) ? (ParserRuleContext)access.children[0] : withExpression.Context, + annotations); + + _unresolved.Add(declaration); + } + public Declaration OnBracketedExpression(string expression, ParserRuleContext context) { var hostApp = FindProject(_hostApp == null ? "VBA" : _hostApp.ApplicationName); diff --git a/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs b/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs index 1817740425..61eb9be810 100644 --- a/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs +++ b/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs @@ -151,18 +151,27 @@ private void ResolveDefault( bool isAssignmentTarget = false, bool hasExplicitLetStatement = false) { + var withExpression = GetInnerMostWithExpression(); var boundExpression = _bindingService.ResolveDefault( _moduleDeclaration, _currentParent, expression, - GetInnerMostWithExpression(), + withExpression, statementContext); if (boundExpression.Classification == ExpressionClassification.ResolutionFailed) { - Logger.Warn( - string.Format( - "Default Context: Failed to resolve {0}. Binding as much as we can.", - expression.GetText())); + var lexpr = expression as VBAParser.LExprContext ?? expression.GetChild(0); + if (lexpr != null) + { + _declarationFinder.AddUnboundContext(_currentParent, lexpr, withExpression); + } + else + { + Logger.Warn( + string.Format( + "Default Context: Failed to resolve {0}. Binding as much as we can.", + expression.GetText())); + } } _boundExpressionVisitor.AddIdentifierReferences(boundExpression, _qualifiedModuleName, _currentScope, _currentParent, isAssignmentTarget, hasExplicitLetStatement); } diff --git a/Rubberduck.Parsing/Symbols/UnboundMemberDeclaration.cs b/Rubberduck.Parsing/Symbols/UnboundMemberDeclaration.cs new file mode 100644 index 0000000000..5cf9f8a72f --- /dev/null +++ b/Rubberduck.Parsing/Symbols/UnboundMemberDeclaration.cs @@ -0,0 +1,39 @@ +using System.Collections.Generic; +using Antlr4.Runtime; +using Rubberduck.Parsing.Annotations; +using Rubberduck.VBEditor; + +namespace Rubberduck.Parsing.Symbols +{ + /// + /// These declarations are created from unresolved member accesses in the DeclarationFinder and are collected for use by inspections. They + /// should NOT be added to the Declaration collections in the parser state. + /// + public class UnboundMemberDeclaration : Declaration + { + /// + /// Context on the LHS of the member access. + /// + public ParserRuleContext CallingContext { get; private set; } + + public UnboundMemberDeclaration(Declaration parentDeclaration, ParserRuleContext unboundIdentifier, ParserRuleContext callingContext, IEnumerable annotations) : + base(new QualifiedMemberName(parentDeclaration.QualifiedName.QualifiedModuleName, unboundIdentifier.GetText()), + parentDeclaration, + parentDeclaration, + "Variant", + string.Empty, + false, + false, + Accessibility.Implicit, + DeclarationType.UnresolvedMember, + unboundIdentifier, + unboundIdentifier.GetSelection(), + false, + null, + false, + annotations) + { + CallingContext = callingContext; + } + } +} diff --git a/RubberduckTests/Inspections/MemberNotOnInterfaceInspectionTests.cs b/RubberduckTests/Inspections/MemberNotOnInterfaceInspectionTests.cs index c0f9ee41ae..00ebd0ca07 100644 --- a/RubberduckTests/Inspections/MemberNotOnInterfaceInspectionTests.cs +++ b/RubberduckTests/Inspections/MemberNotOnInterfaceInspectionTests.cs @@ -15,12 +15,16 @@ namespace RubberduckTests.Inspections [TestClass] public class MemberNotOnInterfaceInspectionTests { - private static ParseCoordinator ArrangeParser(string inputCode) + private static ParseCoordinator ArrangeParser(string inputCode, string library = "Scripting") { var builder = new MockVbeBuilder(); var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) .AddComponent("Codez", ComponentType.StandardModule, inputCode) - .AddReference("Scripting", MockVbeBuilder.LibraryPathScripting, 1, 0, true) + .AddReference(library, + library.Equals("Scripting") ? MockVbeBuilder.LibraryPathScripting : MockVbeBuilder.LibraryPathMsExcel, + 1, + library.Equals("Scripting") ? 0 : 8, + true) .Build(); var vbe = builder.AddProject(project).Build(); @@ -29,7 +33,7 @@ private static ParseCoordinator ArrangeParser(string inputCode) mockHost.SetupAllProperties(); var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); - parser.State.AddTestLibrary("Scripting.1.0.xml"); + parser.State.AddTestLibrary(library.Equals("Scripting") ? "Scripting.1.0.xml" : "Excel.1.8.xml"); return parser; } @@ -81,6 +85,28 @@ Dim dict As Dictionary Assert.AreEqual(1, inspectionResults.Count()); } + [TestMethod] + [DeploymentItem(@"Testfiles\")] + [TestCategory("Inspections")] + public void MemberNotOnInterface_ReturnsResult_ApplicationObject() + { + const string inputCode = +@"Sub Foo() + Application.NonMember +End Sub"; + + //Arrange + var parser = ArrangeParser(inputCode, "Excel"); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new MemberNotOnInterfaceInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + [TestMethod] [DeploymentItem(@"Testfiles\")] [TestCategory("Inspections")] @@ -155,7 +181,6 @@ Debug.Print x.NonMember [TestCategory("Inspections")] public void MemberNotOnInterface_ReturnsResult_WithBlock() { - Assert.Inconclusive("This is currently not working."); const string inputCode = @"Sub Foo() Dim dict As New Dictionary From 7dbac243c65648b7f0ddac4e2abdd34b961e34f3 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 28 Jan 2017 17:53:33 -0600 Subject: [PATCH 04/12] Implement ApplicationWorksheetFunctionInspection. --- .../ApplicationWorksheetFunctionInspection.cs | 43 ++ .../ApplicationWorksheetFunctionQuickFix.cs | 44 ++ .../Resources/InspectionsUI.Designer.cs | 36 ++ .../Inspections/Resources/InspectionsUI.resx | 13 + ...cationWorksheetFunctionInspectionResult.cs | 40 ++ RetailCoder.VBE/Rubberduck.csproj | 3 + ...icationWorksheetFunctionInspectionTests.cs | 439 ++++++++++++++++++ RubberduckTests/RubberduckTests.csproj | 1 + 8 files changed, 619 insertions(+) create mode 100644 RetailCoder.VBE/Inspections/ApplicationWorksheetFunctionInspection.cs create mode 100644 RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs create mode 100644 RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs create mode 100644 RubberduckTests/Inspections/ApplicationWorksheetFunctionInspectionTests.cs diff --git a/RetailCoder.VBE/Inspections/ApplicationWorksheetFunctionInspection.cs b/RetailCoder.VBE/Inspections/ApplicationWorksheetFunctionInspection.cs new file mode 100644 index 0000000000..88f5e546d2 --- /dev/null +++ b/RetailCoder.VBE/Inspections/ApplicationWorksheetFunctionInspection.cs @@ -0,0 +1,43 @@ +using System.Collections.Generic; +using System.Linq; +using Rubberduck.Inspections.Abstract; +using Rubberduck.Inspections.Resources; +using Rubberduck.Inspections.Results; +using Rubberduck.Parsing.Symbols; +using Rubberduck.Parsing.VBA; +using Rubberduck.VBEditor; + +namespace Rubberduck.Inspections +{ + public class ApplicationWorksheetFunctionInspection : InspectionBase + { + public ApplicationWorksheetFunctionInspection(RubberduckParserState state) + : base(state, CodeInspectionSeverity.Suggestion) + { } + + public override string Meta { get { return InspectionsUI.ApplicationWorksheetFunctionInspectionMeta; } } + public override string Description { get { return InspectionsUI.ApplicationWorksheetFunctionInspectionName; } } + public override CodeInspectionType InspectionType { get { return CodeInspectionType.CodeQualityIssues; } } + + public override IEnumerable GetInspectionResults() + { + var excel = State.DeclarationFinder.Projects.SingleOrDefault(item => item.IsBuiltIn && item.IdentifierName == "Excel"); + if (excel == null) { return Enumerable.Empty(); } + + var members = new HashSet(BuiltInDeclarations.Where(decl => decl.DeclarationType == DeclarationType.Function && + decl.ParentDeclaration != null && + decl.ParentDeclaration.ComponentName.Equals("WorksheetFunction")) + .Select(decl => decl.IdentifierName)); + + var usages = BuiltInDeclarations.Where(decl => decl.References.Any() && + decl.ProjectName.Equals("Excel") && + decl.ComponentName.Equals("Application") && + members.Contains(decl.IdentifierName)); + + return (from usage in usages + from reference in usage.References.Where(use => !IsIgnoringInspectionResultFor(use, AnnotationName)) + let qualifiedSelection = new QualifiedSelection(reference.QualifiedModuleName, reference.Selection) + select new ApplicationWorksheetFunctionInspectionResult(this, qualifiedSelection, usage.IdentifierName)); + } + } +} diff --git a/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs b/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs new file mode 100644 index 0000000000..59f8aece69 --- /dev/null +++ b/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs @@ -0,0 +1,44 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Antlr4.Runtime; +using Rubberduck.Inspections.Abstract; +using Rubberduck.Inspections.Resources; +using Rubberduck.VBEditor; + +namespace Rubberduck.Inspections.QuickFixes +{ + public class ApplicationWorksheetFunctionQuickFix : QuickFixBase + { + private readonly string _memberName; + + public ApplicationWorksheetFunctionQuickFix(QualifiedSelection selection, string memberName) + : base(null, selection, InspectionsUI.ApplicationWorksheetFunctionQuickFix) + { + _memberName = memberName; + } + + public override bool CanFixInModule { get { return true; } } + public override bool CanFixInProject { get { return true; } } + + public override void Fix() + { + var module = Selection.QualifiedName.Component.CodeModule; + + var oldContent = module.GetLines(Selection.Selection); + var newCall = string.Format("WorksheetFunction.{0}", _memberName); + var start = Selection.Selection.StartColumn - 1; + //The member being called will always be a single token, so this will always be safe (it will be a single line). + var end = Selection.Selection.EndColumn - 1; + var newContent = oldContent.Substring(0, start) + newCall + + (oldContent.Length > end + ? oldContent.Substring(end, oldContent.Length - end) + : string.Empty); + + module.DeleteLines(Selection.Selection); + module.InsertLines(Selection.Selection.StartLine, newContent); + } + } +} diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs index 4cec373c95..539d05d526 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs @@ -69,6 +69,42 @@ public static string AggregateInspectionResultFormat { } } + /// + /// Looks up a localized string similar to The Excel Application object does not implement the WorksheetFunction interface directly. All calls made to WorksheetFunction members are handled as late bound and errors in the called member will be returned wrapped in a Variant of VbVarType.vbError. This makes errors un-trappable with error handlers and adds a performance penalty in comparison to early bound calls. Consider calling Application.WorksheetFunction explicitly.. + /// + public static string ApplicationWorksheetFunctionInspectionMeta { + get { + return ResourceManager.GetString("ApplicationWorksheetFunctionInspectionMeta", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Late bound WorksheetFunction call.. + /// + public static string ApplicationWorksheetFunctionInspectionName { + get { + return ResourceManager.GetString("ApplicationWorksheetFunctionInspectionName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use of late bound Application.{0} member.. + /// + public static string ApplicationWorksheetFunctionInspectionResultFormat { + get { + return ResourceManager.GetString("ApplicationWorksheetFunctionInspectionResultFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use Application.WorksheetFunction explicitly.. + /// + public static string ApplicationWorksheetFunctionQuickFix { + get { + return ResourceManager.GetString("ApplicationWorksheetFunctionQuickFix", resourceCulture); + } + } + /// /// Looks up a localized string similar to Parameter is passed by value, but is assigned a new value/reference. Consider making a local copy instead if the caller isn't supposed to know the new value. If the caller should see the new value, the parameter should be passed ByRef instead, and you have a bug.. /// diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx index 7ad65b14d6..9cd5a7edc4 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx @@ -634,4 +634,17 @@ If the parameter can be null, ignore this inspection result; passing a null valu Expression '{0}' cannot be validated at compile-time. + + The Excel Application object does not implement the WorksheetFunction interface directly. All calls made to WorksheetFunction members are handled as late bound and errors in the called member will be returned wrapped in a Variant of VbVarType.vbError. This makes errors un-trappable with error handlers and adds a performance penalty in comparison to early bound calls. Consider calling Application.WorksheetFunction explicitly. + + + Late bound WorksheetFunction call. + + + Use of late bound Application.{0} member. + {0} Member name + + + Use Application.WorksheetFunction explicitly. + \ No newline at end of file diff --git a/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs new file mode 100644 index 0000000000..8e8c183593 --- /dev/null +++ b/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs @@ -0,0 +1,40 @@ +using System.Collections.Generic; +using Rubberduck.Common; +using Rubberduck.Inspections.Abstract; +using Rubberduck.Inspections.QuickFixes; +using Rubberduck.Inspections.Resources; +using Rubberduck.VBEditor; + +namespace Rubberduck.Inspections.Results +{ + public class ApplicationWorksheetFunctionInspectionResult : InspectionResultBase + { + private readonly QualifiedSelection _qualifiedSelection; + private readonly string _memberName; + private IEnumerable _quickFixes; + + public ApplicationWorksheetFunctionInspectionResult(IInspection inspection, QualifiedSelection qualifiedSelection, string memberName) + : base(inspection, qualifiedSelection.QualifiedName) + { + _memberName = memberName; + _qualifiedSelection = qualifiedSelection; + } + + public override IEnumerable QuickFixes + { + get + { + return _quickFixes ?? (_quickFixes = new QuickFixBase[] + { + new IgnoreOnceQuickFix(null, _qualifiedSelection, Inspection.AnnotationName), + new ApplicationWorksheetFunctionQuickFix(_qualifiedSelection, _memberName) + }); + } + } + + public override string Description + { + get { return string.Format(InspectionsUI.ApplicationWorksheetFunctionInspectionResultFormat, _memberName).Captialize(); } + } + } +} diff --git a/RetailCoder.VBE/Rubberduck.csproj b/RetailCoder.VBE/Rubberduck.csproj index 1f51a4752e..a6b38e5f48 100644 --- a/RetailCoder.VBE/Rubberduck.csproj +++ b/RetailCoder.VBE/Rubberduck.csproj @@ -365,17 +365,20 @@ + + True True InspectionsUI.resx + diff --git a/RubberduckTests/Inspections/ApplicationWorksheetFunctionInspectionTests.cs b/RubberduckTests/Inspections/ApplicationWorksheetFunctionInspectionTests.cs new file mode 100644 index 0000000000..73c84bdb9e --- /dev/null +++ b/RubberduckTests/Inspections/ApplicationWorksheetFunctionInspectionTests.cs @@ -0,0 +1,439 @@ +using System.Linq; +using System.Threading; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Rubberduck.Inspections; +using Rubberduck.Inspections.QuickFixes; +using Rubberduck.Parsing.VBA; +using Rubberduck.VBEditor.Application; +using Rubberduck.VBEditor.Events; +using Rubberduck.VBEditor.SafeComWrappers; +using RubberduckTests.Mocks; + +namespace RubberduckTests.Inspections +{ + [TestClass] + public class ApplicationWorksheetFunctionInspectionTests + { + private static ParseCoordinator ArrangeParser(string inputCode) + { + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) + .AddComponent("Module1", ComponentType.StandardModule, inputCode) + .AddReference("Excel", MockVbeBuilder.LibraryPathMsExcel, 1, 8, true) + .Build(); + + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); + + parser.State.AddTestLibrary("Excel.1.8.xml"); + return parser; + } + + [TestMethod] + [DeploymentItem(@"TestFiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_ReturnsResult_GlobalApplication() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + foo = Application.Pi +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + [DeploymentItem(@"TestFiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_ReturnsResult_WithGlobalApplication() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + With Application + foo = .Pi + End With +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + [DeploymentItem(@"TestFiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_ReturnsResult_ApplicationVariable() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + Dim xlApp as Excel.Application + Set xlApp = Application + foo = xlApp.Pi +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + [DeploymentItem(@"TestFiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_ReturnsResult_WithApplicationVariable() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + Dim xlApp as Excel.Application + Set xlApp = Application + With xlApp + foo = .Pi + End With +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + [DeploymentItem(@"TestFiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_DoesNotReturnResult_ExplicitUseGlobalApplication() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + foo = Application.WorksheetFunction.Pi +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [DeploymentItem(@"TestFiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_DoesNotReturnResult_ExplicitUseApplicationVariable() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + Dim xlApp as Excel.Application + Set xlApp = Application + foo = xlApp.WorksheetFunction.Pi +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_DoesNotReturnResult_NoExcelReference() + { + const string inputCode = +@"Sub NonExcelSub() + Dim foo As Double + foo = Application.Pi +End Sub +"; + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) + .AddComponent("Module1", ComponentType.StandardModule, inputCode) + .Build(); + + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [DeploymentItem(@"Testfiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_Ignored_DoesNotReturnResult() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + '@Ignore ApplicationWorksheetFunction + foo = Application.Pi +End Sub +"; + + //Arrange + var parser = ArrangeParser(inputCode); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [DeploymentItem(@"Testfiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_IgnoreQuickFixWorks() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + foo = Application.Pi +End Sub +"; + + const string expectedCode = +@"Sub ExcelSub() + Dim foo As Double +'@Ignore ApplicationWorksheetFunction + foo = Application.Pi +End Sub +"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) + .AddComponent("Module1", ComponentType.StandardModule, inputCode) + .AddReference("Excel", MockVbeBuilder.LibraryPathMsExcel, 1, 8, true) + .Build(); + + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); + + parser.State.AddTestLibrary("Excel.1.8.xml"); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + inspectionResults.First().QuickFixes.Single(s => s is IgnoreOnceQuickFix).Fix(); + + var actualCode = project.Object.VBComponents[0].CodeModule.Content(); + + Assert.AreEqual(expectedCode, actualCode); + } + + [TestMethod] + [DeploymentItem(@"Testfiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_UseExplicitlyQuickFixWorks() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + foo = Application.Pi +End Sub +"; + + const string expectedCode = +@"Sub ExcelSub() + Dim foo As Double + foo = Application.WorksheetFunction.Pi +End Sub +"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) + .AddComponent("Module1", ComponentType.StandardModule, inputCode) + .AddReference("Excel", MockVbeBuilder.LibraryPathMsExcel, 1, 8, true) + .Build(); + + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); + + parser.State.AddTestLibrary("Excel.1.8.xml"); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + inspectionResults.First().QuickFixes.Single(s => s is ApplicationWorksheetFunctionQuickFix).Fix(); + + var actualCode = project.Object.VBComponents[0].CodeModule.Content(); + + Assert.AreEqual(expectedCode, actualCode); + } + + [TestMethod] + [DeploymentItem(@"Testfiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_UseExplicitlyQuickFixWorks_WithBlock() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As Double + With Application + foo = .Pi + End With +End Sub +"; + + const string expectedCode = +@"Sub ExcelSub() + Dim foo As Double + With Application + foo = .WorksheetFunction.Pi + End With +End Sub +"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) + .AddComponent("Module1", ComponentType.StandardModule, inputCode) + .AddReference("Excel", MockVbeBuilder.LibraryPathMsExcel, 1, 8, true) + .Build(); + + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); + + parser.State.AddTestLibrary("Excel.1.8.xml"); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + inspectionResults.First().QuickFixes.Single(s => s is ApplicationWorksheetFunctionQuickFix).Fix(); + + var actualCode = project.Object.VBComponents[0].CodeModule.Content(); + + Assert.AreEqual(expectedCode, actualCode); + } + + [TestMethod] + [DeploymentItem(@"Testfiles\")] + [TestCategory("Inspections")] + public void ApplicationWorksheetFunction_UseExplicitlyQuickFixWorks_HasParameters() + { + const string inputCode = +@"Sub ExcelSub() + Dim foo As String + foo = Application.Proper(""foobar"") +End Sub +"; + + const string expectedCode = +@"Sub ExcelSub() + Dim foo As String + foo = Application.WorksheetFunction.Proper(""foobar"") +End Sub +"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected) + .AddComponent("Module1", ComponentType.StandardModule, inputCode) + .AddReference("Excel", MockVbeBuilder.LibraryPathMsExcel, 1, 8, true) + .Build(); + + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock().Object)); + + parser.State.AddTestLibrary("Excel.1.8.xml"); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ApplicationWorksheetFunctionInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + inspectionResults.First().QuickFixes.Single(s => s is ApplicationWorksheetFunctionQuickFix).Fix(); + + var actualCode = project.Object.VBComponents[0].CodeModule.Content(); + + Assert.AreEqual(expectedCode, actualCode); + } + } +} diff --git a/RubberduckTests/RubberduckTests.csproj b/RubberduckTests/RubberduckTests.csproj index 0f085f8310..370477fcb9 100644 --- a/RubberduckTests/RubberduckTests.csproj +++ b/RubberduckTests/RubberduckTests.csproj @@ -90,6 +90,7 @@ + From 91668fd717180fc700b3a48005ab9e3f1c3fff02 Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Sat, 28 Jan 2017 19:00:25 +0100 Subject: [PATCH 05/12] Made ParseComponents use Parallel.ForEach with MaxDegreeOfParallelism --- Rubberduck.Parsing/VBA/ParseCoordinator.cs | 257 ++++++++++----------- 1 file changed, 121 insertions(+), 136 deletions(-) diff --git a/Rubberduck.Parsing/VBA/ParseCoordinator.cs b/Rubberduck.Parsing/VBA/ParseCoordinator.cs index 7c4ef3987a..450f6b4a01 100644 --- a/Rubberduck.Parsing/VBA/ParseCoordinator.cs +++ b/Rubberduck.Parsing/VBA/ParseCoordinator.cs @@ -27,8 +27,7 @@ public class ParseCoordinator : IParseCoordinator { public RubberduckParserState State { get { return _state; } } - private readonly ConcurrentDictionary> _currentTasks = - new ConcurrentDictionary>(); + private const int _maxDegreeOfParserParallelism = 8; private readonly IDictionary, Attributes>> _componentAttributes = new Dictionary, Attributes>>(); @@ -84,6 +83,20 @@ private void ReparseRequested(object sender, EventArgs e) } } + private void Cancel(bool createNewTokenSource = true) + { + lock (_cancellationTokens[0]) + { + _cancellationTokens[0].Cancel(); + _cancellationTokens[0].Dispose(); + if (createNewTokenSource) + { + _cancellationTokens.Add(new CancellationTokenSource()); + } + _cancellationTokens.RemoveAt(0); + } + } + /// /// For the use of tests only /// @@ -96,21 +109,40 @@ public void Parse(CancellationTokenSource token) // tests do not fire events when components are removed--clear components ClearComponentStateCacheForTests(); - SyncComReferences(State.Projects); - State.RefreshFinder(_hostApp); - - AddBuiltInDeclarations(); - State.RefreshFinder(_hostApp); + ExecuteCommenParseActivities(components, token); + + } + private void ExecuteCommenParseActivities(List components, CancellationTokenSource token) + { SetModuleStates(components, ParserState.Pending); + SyncComReferences(State.Projects); + RefreshDeclarationFinder(); + + AddBuiltInDeclarations(); + RefreshDeclarationFinder(); + // invalidation cleanup should go into ParseAsync? CleanUpComponentAttributes(components); + if (token.IsCancellationRequested) + { + return; + } + _projectDeclarations.Clear(); State.ClearBuiltInReferences(); - ParseComponents(components, token); + ParseComponents(components, token.Token); + + if (token.IsCancellationRequested || State.Status >= ParserState.Error) + { + return; + } + + ResolveAllDeclarations(components, token.Token); + RefreshDeclarationFinder(); if (token.IsCancellationRequested || State.Status >= ParserState.Error) { @@ -120,9 +152,13 @@ public void Parse(CancellationTokenSource token) State.SetStatusAndFireStateChanged(this, ParserState.ResolvedDeclarations); ResolveReferences(token.Token); - + State.RebuildSelectionCache(); + } + private void RefreshDeclarationFinder() + { + State.RefreshFinder(_hostApp); } private void SetModuleStates(List components, ParserState parserState) @@ -152,36 +188,92 @@ private void ClearComponentStateCacheForTests() } } - private void ParseComponents(List components, CancellationTokenSource token) + private void ParseComponents(List components, CancellationToken token) { - var parseTasks = new Task[components.Count]; - for (var i = 0; i < components.Count; i++) - { - var index = i; - parseTasks[i] = new Task(() => + var options = new ParallelOptions(); + options.CancellationToken = token; + options.MaxDegreeOfParallelism = _maxDegreeOfParserParallelism; + + Parallel.ForEach(components, + options, + component => { - ParseAsync(components[index], token).Wait(token.Token); + State.ClearStateCache(component); + State.SetModuleState(component, ParserState.Parsing); + var finishedParseTask = FinishedParseComponentTask(component, token); + ProcessComponentParseResults(component, finishedParseTask); + } + ); + } - if (token.IsCancellationRequested) - { - return; - } + private Task FinishedParseComponentTask(IVBComponent component, CancellationToken token, TokenStreamRewriter rewriter = null) + { + var tcs = new TaskCompletionSource(); - if (State.Status == ParserState.Error) { return; } + var preprocessor = _preprocessorFactory(); + var parser = new ComponentParseTask(component, preprocessor, _attributeParser, rewriter); - var qualifiedName = new QualifiedModuleName(components[index]); + parser.ParseFailure += (sender, e) => + { + tcs.SetException(e.Cause); + }; + parser.ParseCompleted += (sender, e) => + { + tcs.SetResult(e); + }; - State.SetModuleState(components[index], ParserState.ResolvingDeclarations); + parser.Start(token); - ResolveDeclarations(qualifiedName.Component, - State.ParseTrees.Find(s => s.Key == qualifiedName).Value); - }); + return tcs.Task; + } + + + private void ProcessComponentParseResults(IVBComponent component, Task finishedParseTask) + { + finishedParseTask.Wait(); + if (finishedParseTask.IsFaulted) + { + State.SetModuleState(component, ParserState.Error, finishedParseTask.Exception.InnerException as SyntaxErrorException); + } + else if (finishedParseTask.IsCompleted) + { + var result = finishedParseTask.Result; + lock (State) + { + lock (component) + { + State.SetModuleAttributes(component, result.Attributes); + State.AddParseTree(component, result.ParseTree); + State.AddTokenStream(component, result.Tokens); + State.SetModuleComments(component, result.Comments); + State.SetModuleAnnotations(component, result.Annotations); - parseTasks[i].Start(); + // This really needs to go last + State.SetModuleState(component, ParserState.Parsed); + } + } } - Task.WaitAll(parseTasks); } + private void ResolveAllDeclarations(List components, CancellationToken token) + { + var options = new ParallelOptions(); + options.CancellationToken = token; + options.MaxDegreeOfParallelism = _maxDegreeOfParserParallelism; + + Parallel.ForEach(components, + options, + component => + { + var qualifiedName = new QualifiedModuleName(component); + State.SetModuleState(component, ParserState.ResolvingDeclarations); + ResolveDeclarations(qualifiedName.Component, + State.ParseTrees.Find(s => s.Key == qualifiedName).Value); + } + ); + } + + private void ResolveReferences(CancellationToken token) { Task.WaitAll(ResolveReferencesAsync(token)); @@ -212,39 +304,7 @@ private void ParseAll(object requestor, CancellationTokenSource token) //return; // returning here leaves state in 'ResolvedDeclarations' when a module is removed, which disables refresh } - SetModuleStates(toParse, ParserState.Pending); - - SyncComReferences(State.Projects); - State.RefreshFinder(_hostApp); - - AddBuiltInDeclarations(); - State.RefreshFinder(_hostApp); - - // invalidation cleanup should go into ParseAsync? - CleanUpComponentAttributes(components); - - if (token.IsCancellationRequested) - { - return; - } - - _projectDeclarations.Clear(); - State.ClearBuiltInReferences(); - - ParseComponents(toParse, token); - - if (token.IsCancellationRequested || State.Status >= ParserState.Error) - { - return; - } - - Debug.Assert(State.ParseTrees.Count == components.Count, string.Format("ParserState has {0} parse trees for {1} components.", State.ParseTrees.Count, components.Count)); - - State.SetStatusAndFireStateChanged(requestor, ParserState.ResolvedDeclarations); - - ResolveReferences(token.Token); - - State.RebuildSelectionCache(); + ExecuteCommenParseActivities(components, token); } /// @@ -521,81 +581,6 @@ private void UnloadComReference(IReference reference, IReadOnlyList } } - private Task ParseAsync(IVBComponent component, CancellationTokenSource token, TokenStreamRewriter rewriter = null) - { - State.ClearStateCache(component); - - var task = new Task(() => ParseAsyncInternal(component, token.Token, rewriter)); - _currentTasks.TryAdd(component, Tuple.Create(task, token)); - - Tuple removedTask; - task.ContinueWith(t => _currentTasks.TryRemove(component, out removedTask), token.Token); // default also executes on cancel - // See http://stackoverflow.com/questions/6800705/why-is-taskscheduler-current-the-default-taskscheduler - task.Start(TaskScheduler.Default); - return task; - } - - private void Cancel(bool createNewTokenSource = true) - { - lock (_cancellationTokens[0]) - { - _cancellationTokens[0].Cancel(); - _cancellationTokens[0].Dispose(); - if (createNewTokenSource) - { - _cancellationTokens.Add(new CancellationTokenSource()); - } - _cancellationTokens.RemoveAt(0); - } - } - - private void ParseAsyncInternal(IVBComponent component, CancellationToken token, TokenStreamRewriter rewriter = null) - { - State.SetModuleState(component, ParserState.Parsing); - - var preprocessor = _preprocessorFactory(); - var parser = new ComponentParseTask(component, preprocessor, _attributeParser, rewriter); - - var finishedParseTask = FinishedComponentParseTask(parser, token); //This runs synchronously. - - if (finishedParseTask.IsFaulted) - { - State.SetModuleState(component, ParserState.Error, finishedParseTask.Exception.InnerException as SyntaxErrorException); - } - else if (finishedParseTask.IsCompleted) - { - var result = finishedParseTask.Result; - lock (State) - { - lock (component) - { - State.SetModuleAttributes(component, result.Attributes); - State.AddParseTree(component, result.ParseTree); - State.AddTokenStream(component, result.Tokens); - State.SetModuleComments(component, result.Comments); - State.SetModuleAnnotations(component, result.Annotations); - - // This really needs to go last - State.SetModuleState(component, ParserState.Parsed); - } - } - } - } - - private Task FinishedComponentParseTask(ComponentParseTask parser, CancellationToken token) - { - var tcs = new TaskCompletionSource(); - parser.ParseFailure += (sender, e) => - { - tcs.SetException(e.Cause); - }; - parser.ParseCompleted += (sender, e) => - { - tcs.SetResult(e); - }; - parser.Start(token); - return tcs.Task; - } private readonly ConcurrentDictionary _projectDeclarations = new ConcurrentDictionary(); private void ResolveDeclarations(IVBComponent component, IParseTree tree) From 2d2435594614d6afac207710b0553296a40b4bd2 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 28 Jan 2017 19:23:15 -0600 Subject: [PATCH 06/12] Forgot to override QualifiedSelection in ApplicationWorksheetFunctionInspectionResult - fixes NRE. --- .../QuickFixes/ApplicationWorksheetFunctionQuickFix.cs | 8 +------- .../ApplicationWorksheetFunctionInspectionResult.cs | 5 +++++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs b/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs index 59f8aece69..49eb553fc5 100644 --- a/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs +++ b/RetailCoder.VBE/Inspections/QuickFixes/ApplicationWorksheetFunctionQuickFix.cs @@ -1,10 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using Antlr4.Runtime; -using Rubberduck.Inspections.Abstract; +using Rubberduck.Inspections.Abstract; using Rubberduck.Inspections.Resources; using Rubberduck.VBEditor; diff --git a/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs index 8e8c183593..dccb841150 100644 --- a/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/ApplicationWorksheetFunctionInspectionResult.cs @@ -20,6 +20,11 @@ public ApplicationWorksheetFunctionInspectionResult(IInspection inspection, Qual _qualifiedSelection = qualifiedSelection; } + public override QualifiedSelection QualifiedSelection + { + get { return _qualifiedSelection; } + } + public override IEnumerable QuickFixes { get From 0e4e331df9e90006de18904899219a08ffad61cf Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 28 Jan 2017 19:58:27 -0600 Subject: [PATCH 07/12] Add quick fix warning to ApplicationWorksheetFunctionInspectionMeta --- RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs | 2 +- RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs index 539d05d526..74022f361a 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs @@ -70,7 +70,7 @@ public static string AggregateInspectionResultFormat { } /// - /// Looks up a localized string similar to The Excel Application object does not implement the WorksheetFunction interface directly. All calls made to WorksheetFunction members are handled as late bound and errors in the called member will be returned wrapped in a Variant of VbVarType.vbError. This makes errors un-trappable with error handlers and adds a performance penalty in comparison to early bound calls. Consider calling Application.WorksheetFunction explicitly.. + /// Looks up a localized string similar to The Excel Application object does not implement the WorksheetFunction interface directly. All calls made to WorksheetFunction members are handled as late bound and errors in the called member will be returned wrapped in a Variant of VbVarType.vbError. This makes errors un-trappable with error handlers and adds a performance penalty in comparison to early bound calls. Consider calling Application.WorksheetFunction explicitly. Note: If this call generated errors in the past, those errors were ignored. If appl [rest of string was truncated]";. /// public static string ApplicationWorksheetFunctionInspectionMeta { get { diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx index 9cd5a7edc4..c2997ce134 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx @@ -635,7 +635,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu Expression '{0}' cannot be validated at compile-time. - The Excel Application object does not implement the WorksheetFunction interface directly. All calls made to WorksheetFunction members are handled as late bound and errors in the called member will be returned wrapped in a Variant of VbVarType.vbError. This makes errors un-trappable with error handlers and adds a performance penalty in comparison to early bound calls. Consider calling Application.WorksheetFunction explicitly. + The Excel Application object does not implement the WorksheetFunction interface directly. All calls made to WorksheetFunction members are handled as late bound and errors in the called member will be returned wrapped in a Variant of VbVarType.vbError. This makes errors un-trappable with error handlers and adds a performance penalty in comparison to early bound calls. Consider calling Application.WorksheetFunction explicitly. Note: If this call generated errors in the past, those errors were ignored. If applying the quick fix, proper error handling should be in place. Late bound WorksheetFunction call. From c5481ffcba9d0fadd900d79d7944a74a96cdcc14 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 28 Jan 2017 22:13:53 -0600 Subject: [PATCH 08/12] Resolve AsTypeNames for UserForm controls. --- .../Symbols/DeclarationSymbolsListener.cs | 4 +- .../Office.Core/Abstract/IControl.cs | 64 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/Rubberduck.Parsing/Symbols/DeclarationSymbolsListener.cs b/Rubberduck.Parsing/Symbols/DeclarationSymbolsListener.cs index 260e5d2cfc..d7d53ab10c 100644 --- a/Rubberduck.Parsing/Symbols/DeclarationSymbolsListener.cs +++ b/Rubberduck.Parsing/Symbols/DeclarationSymbolsListener.cs @@ -9,6 +9,7 @@ using System.Runtime.InteropServices; using Rubberduck.VBEditor.SafeComWrappers; using Rubberduck.VBEditor.SafeComWrappers.Abstract; +using Rubberduck.VBEditor.SafeComWrappers.Office.Core.Abstract; namespace Rubberduck.Parsing.Symbols { @@ -168,12 +169,13 @@ private void DeclareControlsAsMembers(IVBComponent form) foreach (var control in form.Controls) { + var typeName = control.TypeName(); // The as type declaration should be TextBox, CheckBox, etc. depending on the type. var declaration = new Declaration( _qualifiedName.QualifyMemberName(control.Name), _parentDeclaration, _currentScopeDeclaration, - "Control", + string.IsNullOrEmpty(typeName) ? "Control" : typeName, null, true, true, diff --git a/Rubberduck.VBEEditor/SafeComWrappers/Office.Core/Abstract/IControl.cs b/Rubberduck.VBEEditor/SafeComWrappers/Office.Core/Abstract/IControl.cs index 37c852d166..5093493183 100644 --- a/Rubberduck.VBEEditor/SafeComWrappers/Office.Core/Abstract/IControl.cs +++ b/Rubberduck.VBEEditor/SafeComWrappers/Office.Core/Abstract/IControl.cs @@ -1,4 +1,6 @@ using System; +using System.Runtime.InteropServices; +using System.Runtime.InteropServices.ComTypes; using Rubberduck.VBEditor.SafeComWrappers.Abstract; namespace Rubberduck.VBEditor.SafeComWrappers.Office.Core.Abstract @@ -7,4 +9,66 @@ public interface IControl : ISafeComWrapper, IEquatable { string Name { get; set; } } + + public static class ControlExtensions + { + public static string TypeName(this IControl control) + { + try + { + var dispatch = control.Target as IDispatch; + if (dispatch == null) + { + return "Control"; + } + ITypeInfo info; + dispatch.GetTypeInfo(0, 0, out info); + string name; + string docs; + int context; + string help; + info.GetDocumentation(-1, out name, out docs, out context, out help); + return name; + } + catch + { + return "Control"; + } + } + + [ComImport] + [Guid("00020400-0000-0000-C000-000000000046")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + interface IDispatch + { + [PreserveSig] + int GetTypeInfoCount(out int Count); + + [PreserveSig] + int GetTypeInfo( + [MarshalAs(UnmanagedType.U4)] int iTInfo, + [MarshalAs(UnmanagedType.U4)] int lcid, + out ITypeInfo typeInfo); + + [PreserveSig] + int GetIDsOfNames( + ref Guid riid, + [MarshalAs(UnmanagedType.LPArray, ArraySubType = UnmanagedType.LPWStr)] + string[] rgsNames, + int cNames, + int lcid, + [MarshalAs(UnmanagedType.LPArray)] int[] rgDispId); + + [PreserveSig] + int Invoke( + int dispIdMember, + ref Guid riid, + uint lcid, + ushort wFlags, + ref System.Runtime.InteropServices.ComTypes.DISPPARAMS pDispParams, + out object pVarResult, + ref System.Runtime.InteropServices.ComTypes.EXCEPINFO pExcepInfo, + IntPtr[] pArgErr); + } + } } \ No newline at end of file From bf65bf0eac0c53d1fb3adb4292d6075dc30f6963 Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Sun, 29 Jan 2017 12:59:08 +0100 Subject: [PATCH 09/12] Set MaxDegreeOfParserParallelism to universally acceptable value. --- Rubberduck.Parsing/VBA/ParseCoordinator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rubberduck.Parsing/VBA/ParseCoordinator.cs b/Rubberduck.Parsing/VBA/ParseCoordinator.cs index 450f6b4a01..69bc962498 100644 --- a/Rubberduck.Parsing/VBA/ParseCoordinator.cs +++ b/Rubberduck.Parsing/VBA/ParseCoordinator.cs @@ -27,7 +27,7 @@ public class ParseCoordinator : IParseCoordinator { public RubberduckParserState State { get { return _state; } } - private const int _maxDegreeOfParserParallelism = 8; + private const int _maxDegreeOfParserParallelism = -1; private readonly IDictionary, Attributes>> _componentAttributes = new Dictionary, Attributes>>(); From 3fe56a21027ea9c7e98417fd1b0aaa5a4a65ec73 Mon Sep 17 00:00:00 2001 From: Max Doerner Date: Sun, 29 Jan 2017 16:17:48 +0100 Subject: [PATCH 10/12] Repaired optimization not to parse unaltered components that got lost in the refactoring --- Rubberduck.Parsing/VBA/ParseCoordinator.cs | 28 ++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Rubberduck.Parsing/VBA/ParseCoordinator.cs b/Rubberduck.Parsing/VBA/ParseCoordinator.cs index 69bc962498..e3ba1967a6 100644 --- a/Rubberduck.Parsing/VBA/ParseCoordinator.cs +++ b/Rubberduck.Parsing/VBA/ParseCoordinator.cs @@ -109,13 +109,16 @@ public void Parse(CancellationTokenSource token) // tests do not fire events when components are removed--clear components ClearComponentStateCacheForTests(); - ExecuteCommenParseActivities(components, token); + // invalidation cleanup should go into ParseAsync? + CleanUpComponentAttributes(components); + + ExecuteCommonParseActivities(components, token); } - private void ExecuteCommenParseActivities(List components, CancellationTokenSource token) + private void ExecuteCommonParseActivities(List toParse, CancellationTokenSource token) { - SetModuleStates(components, ParserState.Pending); + SetModuleStates(toParse, ParserState.Pending); SyncComReferences(State.Projects); RefreshDeclarationFinder(); @@ -123,9 +126,6 @@ private void ExecuteCommenParseActivities(List components, Cancell AddBuiltInDeclarations(); RefreshDeclarationFinder(); - // invalidation cleanup should go into ParseAsync? - CleanUpComponentAttributes(components); - if (token.IsCancellationRequested) { return; @@ -134,14 +134,14 @@ private void ExecuteCommenParseActivities(List components, Cancell _projectDeclarations.Clear(); State.ClearBuiltInReferences(); - ParseComponents(components, token.Token); + ParseComponents(toParse, token.Token); if (token.IsCancellationRequested || State.Status >= ParserState.Error) { return; } - ResolveAllDeclarations(components, token.Token); + ResolveAllDeclarations(toParse, token.Token); RefreshDeclarationFinder(); if (token.IsCancellationRequested || State.Status >= ParserState.Error) @@ -198,8 +198,8 @@ private void ParseComponents(List components, CancellationToken to options, component => { - State.ClearStateCache(component); State.SetModuleState(component, ParserState.Parsing); + State.ClearStateCache(component); var finishedParseTask = FinishedParseComponentTask(component, token); ProcessComponentParseResults(component, finishedParseTask); } @@ -255,6 +255,7 @@ private void ProcessComponentParseResults(IVBComponent component, Task components, CancellationToken token) { var options = new ParallelOptions(); @@ -288,9 +289,12 @@ private void ParseAll(object requestor, CancellationTokenSource token) State.RefreshProjects(_vbe); var components = State.Projects.SelectMany(project => project.VBComponents).ToList(); - + var componentsRemoved = ClearStateCashForRemovedComponents(components); - + + // invalidation cleanup should go into ParseAsync? + CleanUpComponentAttributes(components); + var toParse = components.Where(component => State.IsNewOrModified(component)).ToList(); if (toParse.Count == 0) @@ -304,7 +308,7 @@ private void ParseAll(object requestor, CancellationTokenSource token) //return; // returning here leaves state in 'ResolvedDeclarations' when a module is removed, which disables refresh } - ExecuteCommenParseActivities(components, token); + ExecuteCommonParseActivities(toParse, token); } /// From 538483695b2d163862bc5024dbaa45d93fc984b1 Mon Sep 17 00:00:00 2001 From: comintern Date: Sun, 29 Jan 2017 11:36:45 -0600 Subject: [PATCH 11/12] Only discover ProjectDisplayName when absolutely necessary. --- .../Abstract/InspectionResultBase.cs | 24 ++++---- RetailCoder.VBE/UnitTesting/TestEngine.cs | 5 +- RetailCoder.VBE/UnitTesting/TestMethod.cs | 2 +- RetailCoder.VBE/UnitTesting/UnitTestUtils.cs | 20 +++---- Rubberduck.Parsing/Symbols/Declaration.cs | 6 ++ .../Symbols/ProjectDeclaration.cs | 55 ++++++++++++++++++- Rubberduck.VBEEditor/Application/AccessApp.cs | 3 +- .../Application/AutoCADApp.cs | 4 +- .../Application/CorelDRAWApp.cs | 5 +- Rubberduck.VBEEditor/Application/ExcelApp.cs | 9 +-- .../Application/FallbackApp.cs | 7 ++- .../Application/HostApplicationBase.cs | 2 +- .../Application/IHostApplication.cs | 7 ++- .../Application/OutlookApp.cs | 4 +- .../Application/PowerPointApp.cs | 4 +- .../Application/ProjectApp.cs | 4 +- .../Application/PublisherApp.cs | 2 +- .../Application/SolidWorksApp.cs | 5 +- Rubberduck.VBEEditor/Application/Visio.cs | 6 +- Rubberduck.VBEEditor/Application/WordApp.cs | 11 ++-- Rubberduck.VBEEditor/QualifiedModuleName.cs | 54 +----------------- RubberduckTests/UnitTesting/DiscoveryTests.cs | 16 +++--- 22 files changed, 134 insertions(+), 121 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs b/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs index 24faa17ee6..60191463b2 100644 --- a/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs +++ b/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs @@ -87,18 +87,18 @@ public virtual int CompareTo(IInspectionResult other) return Inspection.CompareTo(other.Inspection); } - public override string ToString() - { - var module = QualifiedSelection.QualifiedName; - return string.Format( - InspectionsUI.QualifiedSelectionInspection, - Inspection.Severity, - Description, - "(" + module.ProjectDisplayName + ")", - module.ProjectName, - module.ComponentName, - QualifiedSelection.Selection.StartLine); - } + //public override string ToString() + //{ + // var module = QualifiedSelection.QualifiedName; + // return string.Format( + // InspectionsUI.QualifiedSelectionInspection, + // Inspection.Severity, + // Description, + // "(" + module.ProjectDisplayName + ")", + // module.ProjectName, + // module.ComponentName, + // QualifiedSelection.Selection.StartLine); + //} public virtual NavigateCodeEventArgs GetNavigationArgs() { diff --git a/RetailCoder.VBE/UnitTesting/TestEngine.cs b/RetailCoder.VBE/UnitTesting/TestEngine.cs index b097690c81..b10cb4560d 100644 --- a/RetailCoder.VBE/UnitTesting/TestEngine.cs +++ b/RetailCoder.VBE/UnitTesting/TestEngine.cs @@ -3,11 +3,10 @@ using System.Diagnostics; using System.Linq; using Rubberduck.Parsing.Annotations; +using Rubberduck.Parsing.Symbols; using Rubberduck.Parsing.VBA; using Rubberduck.UI.UnitTesting; -using Rubberduck.VBEditor; using Rubberduck.VBEditor.Application; -using Rubberduck.VBEditor.Extensions; using Rubberduck.VBEditor.SafeComWrappers.Abstract; namespace Rubberduck.UnitTesting @@ -99,7 +98,7 @@ public void Run(IEnumerable tests) } } - private void Run(IEnumerable members) + private void Run(IEnumerable members) { if (_hostApplication == null) { diff --git a/RetailCoder.VBE/UnitTesting/TestMethod.cs b/RetailCoder.VBE/UnitTesting/TestMethod.cs index ff0e07cedc..eaaec2d537 100644 --- a/RetailCoder.VBE/UnitTesting/TestMethod.cs +++ b/RetailCoder.VBE/UnitTesting/TestMethod.cs @@ -44,7 +44,7 @@ public void Run() try { AssertHandler.OnAssertCompleted += HandleAssertCompleted; - _hostApp.Run(Declaration.QualifiedName); + _hostApp.Run(Declaration); AssertHandler.OnAssertCompleted -= HandleAssertCompleted; result = EvaluateResults(); diff --git a/RetailCoder.VBE/UnitTesting/UnitTestUtils.cs b/RetailCoder.VBE/UnitTesting/UnitTestUtils.cs index 88f1134b65..b67c7939aa 100644 --- a/RetailCoder.VBE/UnitTesting/UnitTestUtils.cs +++ b/RetailCoder.VBE/UnitTesting/UnitTestUtils.cs @@ -38,40 +38,36 @@ public static bool IsTestMethod(RubberduckParserState state, Declaration item) item.Annotations.Any(a => a.AnnotationType == AnnotationType.TestMethod); } - public static IEnumerable FindModuleInitializeMethods(this QualifiedModuleName module, RubberduckParserState state) + public static IEnumerable FindModuleInitializeMethods(this QualifiedModuleName module, RubberduckParserState state) { return GetTestModuleProcedures(state) .Where(m => m.QualifiedName.QualifiedModuleName == module && - m.Annotations.Any(a => a.AnnotationType == AnnotationType.ModuleInitialize)) - .Select(s => s.QualifiedName); + m.Annotations.Any(a => a.AnnotationType == AnnotationType.ModuleInitialize)); } - public static IEnumerable FindModuleCleanupMethods(this QualifiedModuleName module, RubberduckParserState state) + public static IEnumerable FindModuleCleanupMethods(this QualifiedModuleName module, RubberduckParserState state) { return GetTestModuleProcedures(state) .Where(m => m.QualifiedName.QualifiedModuleName == module && - m.Annotations.Any(a => a.AnnotationType == AnnotationType.ModuleCleanup)) - .Select(s => s.QualifiedName); + m.Annotations.Any(a => a.AnnotationType == AnnotationType.ModuleCleanup)); } - public static IEnumerable FindTestInitializeMethods(this QualifiedModuleName module, RubberduckParserState state) + public static IEnumerable FindTestInitializeMethods(this QualifiedModuleName module, RubberduckParserState state) { return GetTestModuleProcedures(state) .Where(m => m.QualifiedName.QualifiedModuleName == module && - m.Annotations.Any(a => a.AnnotationType == AnnotationType.TestInitialize)) - .Select(s => s.QualifiedName); + m.Annotations.Any(a => a.AnnotationType == AnnotationType.TestInitialize)); } - public static IEnumerable FindTestCleanupMethods(this QualifiedModuleName module, RubberduckParserState state) + public static IEnumerable FindTestCleanupMethods(this QualifiedModuleName module, RubberduckParserState state) { return GetTestModuleProcedures(state) .Where(m => m.QualifiedName.QualifiedModuleName == module && - m.Annotations.Any(a => a.AnnotationType == AnnotationType.TestCleanup)) - .Select(s => s.QualifiedName); + m.Annotations.Any(a => a.AnnotationType == AnnotationType.TestCleanup)); } private static IEnumerable GetTestModuleProcedures(RubberduckParserState state) diff --git a/Rubberduck.Parsing/Symbols/Declaration.cs b/Rubberduck.Parsing/Symbols/Declaration.cs index 59ee00bf04..77497db371 100644 --- a/Rubberduck.Parsing/Symbols/Declaration.cs +++ b/Rubberduck.Parsing/Symbols/Declaration.cs @@ -424,6 +424,12 @@ public string ProjectName get { return _projectName; } } + /// + /// WARNING: This property has side effects. It changes the ActiveVBProject, which causes a flicker in the VBE. + /// This should only be called if it is *absolutely* necessary. + /// + public virtual string ProjectDisplayName { get { return _parentDeclaration.ProjectDisplayName; } } + public object[] ToArray() { return new object[] { ProjectName, CustomFolder, ComponentName, DeclarationType.ToString(), Scope, IdentifierName, AsTypeName }; diff --git a/Rubberduck.Parsing/Symbols/ProjectDeclaration.cs b/Rubberduck.Parsing/Symbols/ProjectDeclaration.cs index bd70ccbcb4..99b19931c1 100644 --- a/Rubberduck.Parsing/Symbols/ProjectDeclaration.cs +++ b/Rubberduck.Parsing/Symbols/ProjectDeclaration.cs @@ -1,4 +1,5 @@ -using Rubberduck.Parsing.ComReflection; +using System.Text.RegularExpressions; +using Rubberduck.Parsing.ComReflection; using Rubberduck.VBEditor; using System.Collections.Generic; using System.Linq; @@ -70,5 +71,57 @@ public void AddProjectReference(string referencedProjectId, int priority) } _projectReferences.Add(new ProjectReference(referencedProjectId, priority)); } + + private static readonly Regex CaptionProjectRegex = new Regex(@"^(?:[^-]+)(?:\s-\s)(?.+)(?:\s-\s.*)?$"); + private static readonly Regex OpenModuleRegex = new Regex(@"^(?.+)(?\s-\s\[.*\((Code|UserForm)\)\])$"); + + private string _displayName; + /// + /// WARNING: This property has side effects. It changes the ActiveVBProject, which causes a flicker in the VBE. + /// This should only be called if it is *absolutely* necessary. + /// + public override string ProjectDisplayName + { + get + { + if (_displayName != null) + { + return _displayName; + } + + if (_project == null) + { + _displayName = string.Empty; + return _displayName; + } + + var vbe = _project.VBE; + var activeProject = vbe.ActiveVBProject; + var mainWindow = vbe.MainWindow; + { + try + { + if (_project.HelpFile != activeProject.HelpFile) + { + vbe.ActiveVBProject = _project; + } + + var caption = mainWindow.Caption; + if (CaptionProjectRegex.IsMatch(caption)) + { + caption = CaptionProjectRegex.Matches(caption)[0].Groups["project"].Value; + _displayName = OpenModuleRegex.IsMatch(caption) + ? OpenModuleRegex.Matches(caption)[0].Groups["project"].Value + : caption; + } + } + catch + { + _displayName = string.Empty; + } + return _displayName; + } + } + } } } diff --git a/Rubberduck.VBEEditor/Application/AccessApp.cs b/Rubberduck.VBEEditor/Application/AccessApp.cs index ce51fde2de..fc71c5a3a4 100644 --- a/Rubberduck.VBEEditor/Application/AccessApp.cs +++ b/Rubberduck.VBEEditor/Application/AccessApp.cs @@ -11,8 +11,9 @@ public class AccessApp : HostApplicationBase - /// Runs VBA procedure specified by name. + /// Runs VBA procedure specified by name. WARNING: The parameter is declared as dynamic to prevent circular referencing. + /// This should ONLY be passed a Declaration object. /// - /// The method to be executed. - void Run(QualifiedMemberName qualifiedMemberName); + /// The Declaration object for the method to be executed. + void Run(dynamic declaration); /// /// Gets the name of the application. diff --git a/Rubberduck.VBEEditor/Application/OutlookApp.cs b/Rubberduck.VBEEditor/Application/OutlookApp.cs index ebc16a9ee6..ebdc10739c 100644 --- a/Rubberduck.VBEEditor/Application/OutlookApp.cs +++ b/Rubberduck.VBEEditor/Application/OutlookApp.cs @@ -10,7 +10,7 @@ public OutlookApp() : base("Outlook") { } - public override void Run(QualifiedMemberName qualifiedMemberName) + public override void Run(dynamic declaration) { // note: does not work. http://stackoverflow.com/q/31954364/1188513 //var app = Application.GetType(); @@ -28,7 +28,7 @@ public override void Run(QualifiedMemberName qualifiedMemberName) var exp = app.ActiveExplorer(); CommandBar cb = exp.CommandBars.Add("RubberduckCallbackProxy", Temporary: true); CommandBarControl btn = cb.Controls.Add(MsoControlType.msoControlButton, 1); - btn.OnAction = qualifiedMemberName.ToString(); + btn.OnAction = declaration.QualifiedName.ToString(); btn.Execute(); cb.Delete(); } diff --git a/Rubberduck.VBEEditor/Application/PowerPointApp.cs b/Rubberduck.VBEEditor/Application/PowerPointApp.cs index f3b4a5e405..17cb7244cc 100644 --- a/Rubberduck.VBEEditor/Application/PowerPointApp.cs +++ b/Rubberduck.VBEEditor/Application/PowerPointApp.cs @@ -4,9 +4,9 @@ public class PowerPointApp : HostApplicationBase() - .FirstOrDefault(doc => doc.Name == qualifiedMemberName.QualifiedModuleName.ProjectDisplayName - || doc.get_AttachedTemplate().Name == qualifiedMemberName.QualifiedModuleName.ProjectDisplayName); + .FirstOrDefault(doc => doc.Name == declaration.ProjectDisplayName + || doc.get_AttachedTemplate().Name == declaration.ProjectDisplayName); if (activeDoc != targetDoc && targetDoc != null) { targetDoc.Activate(); diff --git a/Rubberduck.VBEEditor/QualifiedModuleName.cs b/Rubberduck.VBEEditor/QualifiedModuleName.cs index 0fa2f33cf6..0469810768 100644 --- a/Rubberduck.VBEEditor/QualifiedModuleName.cs +++ b/Rubberduck.VBEEditor/QualifiedModuleName.cs @@ -1,5 +1,5 @@ using System; -using System.Text.RegularExpressions; +using System.Globalization; using Rubberduck.VBEditor.SafeComWrappers; using Rubberduck.VBEditor.SafeComWrappers.Abstract; @@ -19,7 +19,7 @@ public static string GetProjectId(IVBProject project) if (string.IsNullOrEmpty(project.HelpFile)) { - project.HelpFile = project.GetHashCode().ToString(); + project.HelpFile = project.GetHashCode().ToString(CultureInfo.InvariantCulture); } return project.HelpFile; @@ -40,8 +40,6 @@ public QualifiedModuleName(IVBProject project) _projectPath = string.Empty; _projectId = GetProjectId(project); _contentHashCode = 0; - _projectDisplayName = string.Empty; - SetProjectDisplayName(project); } public QualifiedModuleName(IVBComponent component) @@ -63,8 +61,6 @@ public QualifiedModuleName(IVBComponent component) _projectName = project == null ? string.Empty : project.Name; _projectPath = string.Empty; _projectId = GetProjectId(project); - _projectDisplayName = string.Empty; - SetProjectDisplayName(project); } /// @@ -74,9 +70,8 @@ public QualifiedModuleName(IVBComponent component) public QualifiedModuleName(string projectName, string projectPath, string componentName) { _projectName = projectName; - _projectDisplayName = null; _projectPath = projectPath; - _projectId = string.Format("{0};{1}", _projectName, _projectPath).GetHashCode().ToString(); + _projectId = string.Format("{0};{1}", _projectName, _projectPath).GetHashCode().ToString(CultureInfo.InvariantCulture); _componentName = componentName; _component = null; _componentType = ComponentType.ComComponent; @@ -111,49 +106,6 @@ public QualifiedMemberName QualifyMemberName(string member) private readonly string _projectPath; public string ProjectPath { get { return _projectPath; } } - private static readonly Regex CaptionProjectRegex = new Regex(@"^(?:[^-]+)(?:\s-\s)(?.+)(?:\s-\s.*)?$"); - private static readonly Regex OpenModuleRegex = new Regex(@"^(?.+)(?\s-\s\[.*\((Code|UserForm)\)\])$"); - - // because this causes a flicker in the VBE, we only want to do it once. - // we also want to defer it as long as possible because it is only - // needed in a couple places, and QualifiedModuleName is used in many places. - private string _projectDisplayName; - public string ProjectDisplayName - { - get { return _projectDisplayName; } - } - - private void SetProjectDisplayName(IVBProject project) - { - if (project == null) - { - return; - } - var vbe = project.VBE; - var activeProject = vbe.ActiveVBProject; - var mainWindow = vbe.MainWindow; - { - try - { - if (project.HelpFile != activeProject.HelpFile) - { - vbe.ActiveVBProject = project; - } - - var caption = mainWindow.Caption; - if (CaptionProjectRegex.IsMatch(caption)) - { - caption = CaptionProjectRegex.Matches(caption)[0].Groups["project"].Value; - _projectDisplayName = OpenModuleRegex.IsMatch(caption) - ? OpenModuleRegex.Matches(caption)[0].Groups["project"].Value - : caption; - } - } - // ReSharper disable once EmptyGeneralCatchClause - catch { } - } - } - public override string ToString() { return string.IsNullOrEmpty(_componentName) && string.IsNullOrEmpty(_projectName) diff --git a/RubberduckTests/UnitTesting/DiscoveryTests.cs b/RubberduckTests/UnitTesting/DiscoveryTests.cs index 29c098bcc9..f7beb0cb91 100644 --- a/RubberduckTests/UnitTesting/DiscoveryTests.cs +++ b/RubberduckTests/UnitTesting/DiscoveryTests.cs @@ -128,8 +128,8 @@ public void Discovery_DiscoversAnnotatedTestInitInGivenTestModule() var initMethods = qualifiedModuleName.FindTestInitializeMethods(parser.State).ToList(); Assert.AreEqual(1, initMethods.Count); - Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedModuleName.ComponentName); - Assert.AreEqual("TestInitialize", initMethods.ElementAt(0).MemberName); + Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedName.QualifiedModuleName.ComponentName); + Assert.AreEqual("TestInitialize", initMethods.ElementAt(0).QualifiedName.MemberName); } [TestMethod] @@ -154,8 +154,8 @@ public void Discovery_DiscoversAnnotatedTestCleanupInGivenTestModule() var initMethods = qualifiedModuleName.FindTestCleanupMethods(parser.State).ToList(); Assert.AreEqual(1, initMethods.Count); - Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedModuleName.ComponentName); - Assert.AreEqual("TestCleanup", initMethods.ElementAt(0).MemberName); + Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedName.QualifiedModuleName.ComponentName); + Assert.AreEqual("TestCleanup", initMethods.ElementAt(0).QualifiedName.MemberName); } [TestMethod] @@ -268,8 +268,8 @@ public void Discovery_DiscoversAnnotatedModuleInitInGivenTestModule() var initMethods = qualifiedModuleName.FindModuleInitializeMethods(parser.State).ToList(); Assert.AreEqual(1, initMethods.Count); - Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedModuleName.ComponentName); - Assert.AreEqual("ModuleInitialize", initMethods.ElementAt(0).MemberName); + Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedName.QualifiedModuleName.ComponentName); + Assert.AreEqual("ModuleInitialize", initMethods.ElementAt(0).QualifiedName.MemberName); } [TestMethod] @@ -294,8 +294,8 @@ public void Discovery_DiscoversAnnotatedModuleCleanupInGivenTestModule() var initMethods = qualifiedModuleName.FindModuleCleanupMethods(parser.State).ToList(); Assert.AreEqual(1, initMethods.Count); - Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedModuleName.ComponentName); - Assert.AreEqual("ModuleCleanup", initMethods.ElementAt(0).MemberName); + Assert.AreEqual("TestModule1", initMethods.ElementAt(0).QualifiedName.QualifiedModuleName.ComponentName); + Assert.AreEqual("ModuleCleanup", initMethods.ElementAt(0).QualifiedName.MemberName); } [TestMethod] From d90bb6ca1fbe9f906e0d2c689a0e3813e9a65699 Mon Sep 17 00:00:00 2001 From: comintern Date: Sun, 29 Jan 2017 12:33:00 -0600 Subject: [PATCH 12/12] Fix IsImplicitByRef flags for built-in parameters. --- .../DeclarationLoaders/FormEventDeclarations.cs | 6 ++++++ .../Symbols/ParameterDeclaration.cs | 16 ++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Rubberduck.Parsing/Symbols/DeclarationLoaders/FormEventDeclarations.cs b/Rubberduck.Parsing/Symbols/DeclarationLoaders/FormEventDeclarations.cs index cd31d20535..46dad30964 100644 --- a/Rubberduck.Parsing/Symbols/DeclarationLoaders/FormEventDeclarations.cs +++ b/Rubberduck.Parsing/Symbols/DeclarationLoaders/FormEventDeclarations.cs @@ -141,6 +141,9 @@ private static ParameterDeclaration UserFormQueryCloseEventCancelParameter(Decla null, string.Empty, false, + true, + false, + false, true); } @@ -155,6 +158,9 @@ private static ParameterDeclaration UserFormQueryCloseEventCloseModeParameter(De null, string.Empty, false, + true, + false, + false, true); } diff --git a/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs b/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs index 87f353806a..41d38515f3 100644 --- a/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs +++ b/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs @@ -40,6 +40,7 @@ public ParameterDeclaration(QualifiedMemberName qualifiedName, { _isOptional = isOptional; _isByRef = isByRef; + _isImplicitByRef = false; IsParamArray = isParamArray; } @@ -56,7 +57,8 @@ public ParameterDeclaration(QualifiedMemberName qualifiedName, bool isOptional, bool isByRef, bool isArray = false, - bool isParamArray = false) + bool isParamArray = false, + bool isBuiltIn = false) : base( qualifiedName, parentDeclaration, @@ -71,7 +73,7 @@ public ParameterDeclaration(QualifiedMemberName qualifiedName, selection, isArray, asTypeContext, - false) + isBuiltIn) { _isOptional = isOptional; _isByRef = isByRef; @@ -88,12 +90,10 @@ public ParameterDeclaration(ComParameter parameter, Declaration parent, Qualifie null, parameter.IsOptional, parameter.IsByRef, - parameter.IsArray) - { - IsParamArray = parameter.IsParamArray; - } - - + parameter.IsArray, + parameter.IsParamArray) + { } + public bool IsOptional { get { return _isOptional; } } public bool IsByRef { get { return _isByRef; } } public bool IsImplicitByRef { get { return _isImplicitByRef; } }