10
\$\begingroup\$

Release 1.1 of Rubberduck only had a handful of implemented code inspections, more as a proof of concept than anything else. For release 1.2, we now have 19 implementations of our IInspection interface - here is what happens when the user clicks the Fugue Icons - arrow-circle-double.png refresh button on the Code Inspections toolwindow:

private void Refresh()
{
    var code = new VBProjectParseResult(_parser.Parse(VBE.ActiveVBProject));

    var results = new ConcurrentBag<CodeInspectionResultBase>();
    var inspections = _inspections.Where(inspection => inspection.Severity != CodeInspectionSeverity.DoNotShow)
        .Select(inspection =>
            new Task(() =>
            {
                var result = inspection.GetInspectionResults(code);
                foreach (var inspectionResult in result)
                {
                    results.Add(inspectionResult);
                }
            })).ToArray();

    foreach (var inspection in inspections)
    {
        inspection.Start();
    }

    Task.WaitAll(inspections);

    _results = results.ToList();
    Control.SetContent(_results.Select(item => new CodeInspectionResultGridViewItem(item)).OrderBy(item => item.Component).ThenBy(item => item.Line));
}

The idea is to collect inspection results concurrently, wait for all results to be collected, and then to refresh the grid showing the inspection results. The thought of populating the grid as results come in occurred to me, but I presume the cost of repainting the grid every time would outweight the benefits, especially with larger VBA projects - I tested one that yield 177 code issues (including some false positives, but that's a separate issue, I'm working on that!).

Before I implemented the changes I'm about to describe, inspecting issues in that project would simply freeze the VBE (and the host app) and I'd end up stopping the debugger - the project with the 177 issues completely brought our ducky to its knees. Actually, it's not the issues themselves; it's the inefficiency of walking the parse tree as if it were the first and last thing that ever needed to happen - not to mention the loose grammar rules that consider way too many things as a "variable usage"... but I digress, these grammar issues are completely unrelated to this post.

So I changed this member of the IInspection interface from this:

IEnumerable<CodeInspectionResultBase> GetInspectionResults(IEnumerable<VBComponentParseResult> parseResult);

To that:

IEnumerable<CodeInspectionResultBase> GetInspectionResults(VBProjectParseResult parseResult);

The issues are now found in roughly 15-30 seconds, which is better than [no idea, gave up after 5 minutes].

Here's the VBProjectParseResult class:

public class VBProjectParseResult
{
    public VBProjectParseResult(IEnumerable<VBComponentParseResult> parseResults)
    {
        _parseResults = parseResults;
        _inspector = new IdentifierUsageInspector(_parseResults);
    }

    private readonly IEnumerable<VBComponentParseResult> _parseResults;
    public IEnumerable<VBComponentParseResult> ComponentParseResults { get { return _parseResults; } }

    private readonly IdentifierUsageInspector _inspector;
    public IdentifierUsageInspector IdentifierUsageInspector { get { return _inspector; } }
}

It caches an IdentifierUsageInspector - and this is where the whole performance gain was achieved:

public class IdentifierUsageInspector
{
    private readonly IEnumerable<VBComponentParseResult> _parseResult;
    private readonly HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _globals;
    private readonly HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _fields;
    private readonly HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _locals;
    private readonly HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _parameters;

    private readonly HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _assignments;
    private readonly HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _usages;

    public IdentifierUsageInspector(IEnumerable<VBComponentParseResult> parseResult)
    {
        _parseResult = parseResult;
        _globals = GetGlobals();
        _fields = GetFields(_globals);
        _locals = GetLocals();
        _parameters = GetParameters();
        _assignments = GetAssignments();
        _usages = GetIdentifierUsages(_assignments);
    }

    /// <summary>
    /// Gets all private fields whose name clashes with that of a global variable or constant.
    /// </summary>
    /// <remarks>
    /// VBA compiler resolves identifier references to the tightest applicable scope.
    /// </remarks>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> AmbiguousFieldNames()
    {
        // not used.... yet.

        foreach (var field in _fields)
        {
            var fieldName = field.Context.GetText();
            if (_globals.Any(global => global.Context.GetText() == fieldName))
            {
                yield return field;
            }
        }
    }


    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unassignedGlobals;
    /// <summary>
    /// Gets all global-scope fields that are not assigned in any standard or class module.
    /// </summary>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnassignedGlobals()
    {
        if (_unassignedGlobals != null)
        {
            return _unassignedGlobals;
        }

        _unassignedGlobals = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();

        var unassignedGlobals = _globals.Where(context => context.Context.Parent.GetType() != typeof(VBParser.ConstSubStmtContext))
            .Where(global => _assignments.Where(assignment => assignment.QualifiedName.Equals(global.QualifiedName))
                .All(assignment => global.Context.GetText() != assignment.Context.GetText()));

        foreach (var unassignedGlobal in unassignedGlobals)
        {
            var global = unassignedGlobal;
            if (_assignments.Where(assignment => assignment.QualifiedName != global.QualifiedName)
                .All(assignment => global.Context.GetText() != assignment.Context.GetText()))
            {
                _unassignedGlobals.Add(global);
            }
        }

        return _unassignedGlobals;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _allUnassignedVariables;
    /// <summary>
    /// Gets all globals, fields and locals that are not assigned in their respective scope.
    /// </summary>
    /// <returns>
    /// Returns the declaration context's identifier.
    /// </returns>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> AllUnassignedVariables()
    {
        if (_allUnassignedVariables != null)
        {
            return _allUnassignedVariables;
        }

        _allUnassignedVariables = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(UnassignedGlobals().Union(UnassignedFields().Union(UnassignedLocals())));
        return _allUnassignedVariables;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _allUnusedVariables;
    /// <summary>
    /// Gets all globals, fields and locals that are not used and not assigned in their respective scope.
    /// </summary>
    /// <returns>
    /// Returns the declaration context's identifier.
    /// </returns>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> AllUnusedVariables()
    {
        if (_allUnusedVariables != null)
        {
            return _allUnusedVariables;
        }

        _allUnusedVariables = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(UnusedGlobals().Union(UnusedFields().Union(UnusedLocals())));
        return _allUnusedVariables;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _undeclaredVariableUsages;
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UndeclaredVariableUsages()
    {
        if (_undeclaredVariableUsages != null)
        {
            return _undeclaredVariableUsages;
        }

        var undeclared = _usages.Where(usage => _locals.All(local => local.MemberName != usage.MemberName && local.Context.GetText() != usage.Context.GetText())
                                    && _fields.All(field => field.QualifiedName != usage.QualifiedName && field.Context.GetText() != usage.Context.GetText())
                                    && _globals.All(global => global.Context.GetText() != usage.Context.GetText())
                                    && _parameters.All(parameter => parameter.MemberName != usage.MemberName && parameter.Context.GetText() != usage.Context.GetText()));

        _undeclaredVariableUsages = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(undeclared);
        return _undeclaredVariableUsages;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _allUnassignedVariableUsages;
    /// <summary>
    /// Gets all globals, fields and locals that are unassigned (used or not) in their respective scope.
    /// </summary>
    /// <returns>
    /// Returns the variable call context's identifier.
    /// </returns>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> AllUnassignedVariableUsages()
    {
        if (_allUnassignedVariableUsages != null)
        {
            return _allUnassignedVariableUsages;
        }

        var variables = AllUnassignedVariables();
        _allUnassignedVariableUsages = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
                                                          _usages.Where(usage => variables.Any(variable => usage.QualifiedName == variable.QualifiedName
                                                          && usage.Context.GetText() == variable.Context.GetText()))
                                                          );

        return _allUnassignedVariableUsages;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unassignedFields;
    /// <summary>
    /// Gets all module-scope fields that are not assigned.
    /// </summary>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnassignedFields()
    {
        if (_unassignedFields != null)
        {
            return _unassignedFields;
        }

        _unassignedFields = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
                        _fields.Where(context => context.Context.Parent.GetType() != typeof(VBParser.ConstSubStmtContext))
                        .Where(field => _assignments.Where(assignment => assignment.QualifiedName.Equals(field.QualifiedName))
                                .All(assignment => field.Context.GetText() != assignment.Context.GetText()))
                        );

        return _unassignedFields;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unassignedLocals;
    /// <summary>
    /// Gets all procedure-scope locals that are not assigned.
    /// </summary>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnassignedLocals()
    {
        if (_unassignedLocals != null)
        {
            return _unassignedLocals;
        }

        _unassignedLocals = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
            _locals.Where(context => context.Context.Parent.GetType() != typeof(VBParser.ConstSubStmtContext))
            .Where(local => _assignments.Where(assignment => assignment.QualifiedName.Equals(local.QualifiedName))
                .All(assignment => local.Context.GetText() != assignment.Context.GetText()))
            );

        return _unassignedLocals;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unassignedByRefParameters;
    /// <summary>
    /// Gets all unassigned ByRef parameters.
    /// </summary>
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnassignedByRefParameters()
    {
        if (_unassignedByRefParameters != null)
        {
            return _unassignedByRefParameters;
        }

        var byRefParams = 
            (from parameter in _parameters
            let byRef = ((VBParser.ArgContext) parameter.Context.Parent).BYREF()
            let byVal = ((VBParser.ArgContext) parameter.Context.Parent).BYVAL()
            where byRef != null || (byRef == null && byVal == null)
            select parameter).ToList();

        _unassignedByRefParameters = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
            _parameters.Where(parameter => byRefParams.Contains(parameter)
            && _assignments.Where(usage => usage.MemberName.Equals(parameter.MemberName))
                .All(usage => parameter.Context.GetText() != usage.Context.GetText()))
            );

        return _unassignedByRefParameters;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unusedGlobals;
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnusedGlobals()
    {
        if (_unusedGlobals != null)
        {
            return _unusedGlobals;
        }

        _unusedGlobals = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
                _globals.Where(context => _usages.Where(usage => usage.QualifiedName.Equals(context.QualifiedName))
                .All(usage => context.Context.GetText() != usage.Context.GetText()))
                );

        return _unusedGlobals;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unusedFields;
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnusedFields()
    {
        if (_unusedFields != null)
        {
            return _unusedFields;
        }

        _unusedFields = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
            _fields.Where(context =>
            _usages.Where(usage => usage.QualifiedName.Equals(context.QualifiedName))
                .All(usage => context.Context.GetText() != usage.Context.GetText()))
           );

        return _unusedFields;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unusedLocals;
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnusedLocals()
    {
        if (_unusedLocals != null)
        {
            return _unusedLocals;
        }

        _unusedLocals = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
            _locals.Where(context => 
            _usages.Where(usage => usage.MemberName.Equals(context.MemberName))
                .All(usage => context.Context.GetText() != usage.Context.GetText()))
            );
        return _unusedLocals;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> _unusedParameters;
    public IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> UnusedParameters()
    {
        if (_unusedParameters != null)
        {
            return _unusedParameters;
        }

        _unusedParameters = new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(
            _parameters.Where(context =>
            _usages.Where(usage => usage == context)
                .All(usage => context.Context.GetText() != usage.Context.GetText()))
            );
        return _unusedParameters;
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> GetGlobals()
    {
        var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();

        var modules = _parseResult.Where(e => e.Component.Type == vbext_ComponentType.vbext_ct_StdModule);
        foreach (var module in modules)
        {
            var scope = module;
            var listener = new DeclarationSectionListener(module.QualifiedName);
            var declarations = module.ParseTree
                .GetContexts<DeclarationSectionListener, ParserRuleContext>(listener)
                .Select(declaration => declaration.Context)
                                        .OfType<VBParser.VariableStmtContext>()
                .Where(declaration => IsGlobal(declaration.Visibility()))
                .SelectMany(declaration => declaration.VariableListStmt().VariableSubStmt())
                .Select(identifier => identifier.AmbiguousIdentifier().ToQualifiedContext(scope.QualifiedName));

            result.AddRange(declarations);
        }

        return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> 
        GetFields(IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> globals)
    {
        var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();
        foreach (var module in _parseResult)
        {
            var listener = new DeclarationSectionListener(module.QualifiedName);
            var declarations = module.ParseTree
                .GetContexts<DeclarationSectionListener, ParserRuleContext>(listener)
                .Where(field => globals.All(global => global.QualifiedName.ModuleName == field.QualifiedName.ModuleName 
                                                   && global.Context.GetText() != field.Context.GetText()))
                .ToList();

            result.AddRange(declarations.Select(declaration => declaration.Context)
                                        .OfType<VBParser.VariableSubStmtContext>()
                                        .Select(context => 
                    context.AmbiguousIdentifier().ToQualifiedContext(module.QualifiedName)));

            //result.AddRange(declarations.Select(declaration => declaration.Context)
            //                            .OfType<VBParser.TypeStmtContext>()
            //                            .Select(context => 
            //        context.AmbiguousIdentifier().ToQualifiedContext(module.QualifiedName)));
        }

        return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> GetLocals()
    {
        var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();
        foreach (var module in _parseResult)
        {
            var HashSetener = new LocalDeclarationListener(module.QualifiedName);
            result.AddRange(module.ParseTree
                .GetContexts<LocalDeclarationListener, VBParser.AmbiguousIdentifierContext>(HashSetener));
        }

        return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> GetParameters()
    {
        var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();
        foreach (var module in _parseResult)
        {
            var listener = new ParameterListener(module.QualifiedName);
            result.AddRange(module.ParseTree
                .GetContexts<ParameterListener, VBParser.AmbiguousIdentifierContext>(listener));
        }

        return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> GetAssignments()
    {
        var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();
        foreach (var module in _parseResult)
        {
            var HashSetener = new VariableAssignmentListener(module.QualifiedName);
            result.AddRange(module.ParseTree
                .GetContexts<VariableAssignmentListener, VBParser.AmbiguousIdentifierContext>(HashSetener)
                .Where(identifier => !IsConstant(identifier.Context) && !IsJoinedAssignemntDeclaration(identifier.Context)));
        }

        return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
    }

    private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> GetIdentifierUsages(IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> assignments)
    {
        var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();
        foreach (var module in _parseResult)
        {
            var listener = new VariableReferencesListener(module.QualifiedName);

            var usages = module.ParseTree.GetContexts<VariableReferencesListener, VBParser.AmbiguousIdentifierContext>(listener);
            result.AddRange(usages.Where(usage => assignments.Any(assignment => !usage.Equals(assignment))));
        }

        return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
    }

    private static bool IsConstant(VBParser.AmbiguousIdentifierContext context)
    {
        return context.Parent.Parent.GetType() == typeof(VBParser.ConstSubStmtContext);
    }

    private static bool IsJoinedAssignemntDeclaration(VBParser.AmbiguousIdentifierContext context)
    {
        var declaration = context.Parent as VBParser.VariableSubStmtContext;
        if (declaration == null)
        {
            return false;
        }

        var asTypeClause = declaration.AsTypeClause();
        if (asTypeClause == null)
        {
            return false;
        }

        return asTypeClause.NEW() == null;
    }

    private static bool IsGlobal(VBParser.VisibilityContext context)
    {
        return context != null && context.GetText() != Tokens.Private;
    }
}

So, instead of having each inspection re-walk the entire parse tree an arbitrary number of times to figure out what variables were unused, I'm doing that work once, and then passing the results to all inspections that want them.

That's great, but far from awesome - it takes about 15 seconds to inspect a large-ish project, and I'd like it to be faster... much faster.

I'm using a similar strategy in the RubberduckParser class, where a ConcurrentDictionary is caching the parse results - this means when the add-in needs to parse an entire project, only the modules that have changed are actually parsed; the unchanged ones have a cached parse tree waiting to be consumed:

public class RubberduckParser : IRubberduckParser
{
    private static readonly ConcurrentDictionary<QualifiedModuleName, VBComponentParseResult> ParseResultCache = 
        new ConcurrentDictionary<QualifiedModuleName, VBComponentParseResult>();

    public IParseTree Parse(string code)
    {
        var input = new AntlrInputStream(code);
        var lexer = new VBLexer(input);
        var tokens = new CommonTokenStream(lexer);
        var parser = new VBParser(tokens);

        var result = parser.StartRule();
        return result;
    }

    public IEnumerable<VBComponentParseResult> Parse(VBProject project)
    {
        var modules = project.VBComponents.Cast<VBComponent>();
        foreach(var module in modules)
        {
            yield return Parse(module);
        };
    }

    public VBComponentParseResult Parse(VBComponent component)
    {
        VBComponentParseResult cachedValue;
        var name = component.QualifiedName();
        if (ParseResultCache.TryGetValue(name, out cachedValue))
        {
            return cachedValue;
        }

        var parseTree = Parse(component.CodeModule.Lines());
        var comments = ParseComments(component);
        var result = new VBComponentParseResult(component, parseTree, comments);

        ParseResultCache.AddOrUpdate(name, module => result, (qName, module) => result);
        return result;
    }

    public IEnumerable<CommentNode> ParseComments(VBComponent component)
    {
        var code = component.CodeModule.Code();
        var qualifiedName = component.QualifiedName();

        var commentBuilder = new StringBuilder();
        var continuing = false;

        var startLine = 0;
        var startColumn = 0;

        for (var i = 0; i < code.Length; i++)
        {
            var line = code[i];                
            var index = 0;

            if (continuing || line.HasComment(out index))
            {
                startLine = continuing ? startLine : i;
                startColumn = continuing ? startColumn : index;

                var commentLength = line.Length - index;

                continuing = line.EndsWith("_");
                if (!continuing)
                {
                    commentBuilder.Append(line.Substring(index, commentLength).TrimStart());
                    var selection = new Selection(startLine + 1, startColumn + 1, i + 1, line.Length);

                    var result = new CommentNode(commentBuilder.ToString(), new QualifiedSelection(qualifiedName, selection));
                    commentBuilder.Clear();

                    yield return result;
                }
                else
                {
                    // ignore line continuations in comment text:
                    commentBuilder.Append(line.Substring(index, commentLength).TrimStart()); 
                }
            }
        }
    }
}

The RubberduckParser instance's lifetime is that of the application itself, so there's a potential cache bloat that could happen if a huge project is opened, parsed, and then closed... but then, cache invalidation is hard, right?

Is there a better way to do this? Anything sticks out? Hurts your eyes? Makes you want to run away or hide under your desk?

Note that performance is directly related to the number of code modules in the parse result - 30 seconds is for a VBA project that's much larger than average. I'd like to bring it down as much as possible.

Performance profiling reveals that most of the work is done iterating the inspection results, or in ANTLR parse tree generated methods.

\$\endgroup\$
3
\$\begingroup\$
//result.AddRange(declarations.Select(declaration => declaration.Context)
//                            .OfType<VBParser.TypeStmtContext>()
//                            .Select(context => 
//        context.AmbiguousIdentifier().ToQualifiedContext(module.QualifiedName)));  

Commented code is dead code and dead code should be deleted.


In

private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> 
    GetFields(IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> globals)  

You are calling ToList() on the IEnumerable and the again using Select() thow changing to an IEnumerable again which then is added to a List<T> by calling AddRange().

Because it is an IEnumerable and not an ICollection the call to AddRange() will again iterate over the elements of the IEnumerable to add them to the List<T>.

So refactoring to

private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> 
    GetFields(IEnumerable<QualifiedContext<VBParser.AmbiguousIdentifierContext>> globals)
{
    var result = new List<QualifiedContext<VBParser.AmbiguousIdentifierContext>>();
    foreach (var module in _parseResult)
    {
        var listener = new DeclarationSectionListener(module.QualifiedName);
        var fields = module.ParseTree
            .GetContexts<DeclarationSectionListener, ParserRuleContext>(listener)
            .Where(field => globals.All(global => global.QualifiedName.ModuleName == field.QualifiedName.ModuleName 
                                               && global.Context.GetText() != field.Context.GetText()))
            .Select(declaration => declaration.Context)
            .OfType<VBParser.VariableSubStmtContext>()
            .Select(context => context.AmbiguousIdentifier().ToQualifiedContext(module.QualifiedName))

        result.AddRange(fields);

    }

    return new HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>>(result);
}  

will be faster and better looking.


private HashSet<QualifiedContext<VBParser.AmbiguousIdentifierContext>> GetAssignments()  

The variable var HashSetener = new VariableAssignmentListener(module.QualifiedName); should at least be named using camelCase casing, but what the heck is a HashSetener ?

I would also suggest to introduce a bool IsValidAssignmentContext() to be called by the Where() like

private bool IsValidAssignmentContext(VBParser.AmbiguousIdentifierContext context)
{
    return !IsConstant(context) && !IsJoinedAssignemntDeclaration(context)
}  

and to be called

result.AddRange(module.ParseTree
            .GetContexts<VariableAssignmentListener, VBParser.AmbiguousIdentifierContext>(HashSetener)
            .Where(identifier => IsValidAssignmentContext(identifier.Context)));  
\$\endgroup\$
  • 3
    \$\begingroup\$ LOL! A HashSetener is what you get when you have a listener and then want to replace all List<T> with a HashSet<T> with find/replace-all ;) \$\endgroup\$ – Mathieu Guindon Mar 3 '15 at 12:02
  • \$\begingroup\$ lol. Yeah... HashSetener... My bad... \$\endgroup\$ – RubberDuck Mar 3 '15 at 12:40
2
\$\begingroup\$

Just an idea but maybe create a custom collection type to hold your variable classes and load the types into lookup dictionaries to make your property queries faster. This will obviously add some overhead at load, along with memory additions, but my guess is it will be worth it. Please let me know.

class Program
{
    static void Main(string[] args)
    {
        AmbigousIndentiferContextCollection locals = new AmbigousIndentiferContextCollection(null);
        AmbigousIndentiferContextCollection globals = new AmbigousIndentiferContextCollection(null);
        AmbigousIndentiferContextCollection fields = new AmbigousIndentiferContextCollection(null);
        AmbigousIndentiferContextCollection parameters = new AmbigousIndentiferContextCollection(null);
        AmbigousIndentiferContextCollection assignments = new AmbigousIndentiferContextCollection(null);
        AmbigousIndentiferContextCollection useages = new AmbigousIndentiferContextCollection(null);

        HashSet<AmbigousIndentifierContext> _unassignedGlobals = new HashSet<AmbigousIndentifierContext>();
        var unassignedGlobals = globals.GetItemsNotWithParentType(typeof(object)).Where(global => assignments.GetItemsWithQualifiedName(global.QualifiedName).All(
            assignment => assignment.Text != global.Text));
        foreach (var unassignedGlobal in unassignedGlobals)
        {
            if (assignments.GetItemsNotWithQualifiedName(unassignedGlobal.QualifiedName).All(assignment => unassignedGlobal.Text != assignment.Text))
                _unassignedGlobals.Add(unassignedGlobal);
        }
    }
}

public class AmbigousIndentifierContext
{
    public Type ParentType { get; set; }
    public string QualifiedName { get; set; }
    public string Text { get; set; }
    public string MemberName { get; set; }
}

public class AmbigousIndentiferContextCollection : IEnumerable<AmbigousIndentifierContext>
{
    private HashSet<AmbigousIndentifierContext> Collection { get; set; }
    private Dictionary<Type, List<AmbigousIndentifierContext>> TypeMapping { get; set; }
    private Dictionary<string, List<AmbigousIndentifierContext>> QualifiedNameMapping { get; set; }
    private Dictionary<string, List<AmbigousIndentifierContext>> TextMapping { get; set; }
    private Dictionary<string, List<AmbigousIndentifierContext>> MemberNameMapping { get; set; }
    public AmbigousIndentiferContextCollection(IEnumerable<AmbigousIndentifierContext> contexts)
    {
        Collection = new HashSet<AmbigousIndentifierContext>();
        QualifiedNameMapping = new Dictionary<string, List<AmbigousIndentifierContext>>();
        TypeMapping = new Dictionary<Type, List<AmbigousIndentifierContext>>();
        TextMapping = new Dictionary<string, List<AmbigousIndentifierContext>>();
        MemberNameMapping = new Dictionary<string, List<AmbigousIndentifierContext>>();
        foreach (var context in contexts)
        {
            Add(context);
        }
    }
    public void Add(AmbigousIndentifierContext context)
    {
        Collection.Add(context);
        List<AmbigousIndentifierContext> typeMapping;
        if (!TypeMapping.TryGetValue(context.ParentType, out typeMapping))
        {
            typeMapping = new List<AmbigousIndentifierContext>();
            TypeMapping[context.ParentType] = typeMapping;
        }
        typeMapping.Add(context);
        List<AmbigousIndentifierContext> qualifiedNameMapping;
        if (!QualifiedNameMapping.TryGetValue(context.QualifiedName, out qualifiedNameMapping))
        {
            qualifiedNameMapping = new List<AmbigousIndentifierContext>();
            QualifiedNameMapping[context.QualifiedName] = qualifiedNameMapping;
        }
        qualifiedNameMapping.Add(context);
        List<AmbigousIndentifierContext> textMapping;
        if (!TextMapping.TryGetValue(context.Text, out textMapping))
        {
            textMapping = new List<AmbigousIndentifierContext>();
            TextMapping[context.Text] = textMapping;
        }
        textMapping.Add(context);
        List<AmbigousIndentifierContext> memberNameMapping;
        if (!MemberNameMapping.TryGetValue(context.MemberName, out memberNameMapping))
        {
            memberNameMapping = new List<AmbigousIndentifierContext>();
            MemberNameMapping[context.MemberName] = memberNameMapping;
        }
        memberNameMapping.Add(context);
    }
    public IEnumerable<AmbigousIndentifierContext> GetItemsWithParentType(Type type)
    {
        List<AmbigousIndentifierContext> types;
        if(TypeMapping.TryGetValue(type, out types))
        {
            foreach(var item in types)
            {
                yield return item;
            }
        }
    }
    public IEnumerable<AmbigousIndentifierContext> GetItemsNotWithParentType(Type type)
    {
        foreach (var dicItem in TypeMapping.Where(i => i.Key != type))
        {
            foreach (var item in dicItem.Value)
            {
                yield return item;
            }
        }
    }
    public IEnumerable<AmbigousIndentifierContext> GetItemsWithQualifiedName(string qualifiedName)
    {
        List<AmbigousIndentifierContext> qualifiedNames;
        if (QualifiedNameMapping.TryGetValue(qualifiedName, out qualifiedNames))
        {
            foreach (var item in qualifiedNames)
            {
                yield return item;
            }
        }
    }
    public IEnumerable<AmbigousIndentifierContext> GetItemsNotWithQualifiedName(string qualifiedName)
    {
        foreach (var dicItem in QualifiedNameMapping.Where(i => i.Key != qualifiedName))
        {
            foreach (var item in dicItem.Value)
            {
                yield return item;
            }
        }
    }
    public IEnumerable<AmbigousIndentifierContext> GetItemsWithText(string text)
    {
        List<AmbigousIndentifierContext> texts;
        if(TextMapping.TryGetValue(text, out texts))
        {
            foreach(var item in texts)
            {
                yield return item;
            }
        }
    }
    public IEnumerable<AmbigousIndentifierContext> GetItemsWithMemberName(string memberName)
    {
        List<AmbigousIndentifierContext> memberNames;
        if (MemberNameMapping.TryGetValue(memberName, out memberNames))
        {
            foreach (var item in memberNames)
            {
                yield return item;
            }
        }
    }

    public IEnumerator<AmbigousIndentifierContext> GetEnumerator()
    {
        foreach (var item in Collection)
        {
            yield return item;
        }
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return this.GetEnumerator();
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.