search

rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.92k stars 302 forks source link

Code Metrics faceplanting #4814

Closed bclothier closed 5 years ago

bclothier commented 5 years ago

Rubberduck version information Version 2.4.0.4532 OS: Microsoft Windows NT 10.0.17763.0, x64 Host Product: Microsoft Office x64 Host Version: 16.0.11231.20174 Host Executable: MSACCESS.EXE

Description After a lengthy session, I managed to arrive to a state where code metrics would not run correctly, throwing an exception. Because the code metrics is run as part of parsing, the exception runs up all the parsing stack, resulting in a parser state of Unexpected error

To Reproduce Not sure yet. Will update.

Expected behavior Code metrics should not throw exceptions to the parser. Ever. Let it faceplant without pulling down everyone down.

Logfile

2019-02-21 07:18:54.1670;ERROR-2.4.0.4532;Rubberduck.Parsing.VBA.ParseCoordinator;Unexpected exception thrown in parsing run. (thread 47).;System.InvalidOperationException: Sequence contains no elements
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at Rubberduck.CodeAnalysis.CodeMetrics.CyclomaticComplexityListener.ExitFunctionStmt(FunctionStmtContext context) in C:\projects\rubberduck\Rubberduck.CodeAnalysis\CodeMetrics\CyclomaticComplexityMetric.cs:line 62
   at Rubberduck.Parsing.Grammar.VBAParser.FunctionStmtContext.ExitRule(IParseTreeListener listener) in C:\projects\rubberduck\Rubberduck.Parsing\obj\Release\net46\VBAParser.cs:line 8858
   at Rubberduck.Parsing.VBA.CombinedParseTreeListener.ExitEveryRule(ParserRuleContext ctx) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\CombinedParseTreeListener.cs:line 30
   at Antlr4.Runtime.Tree.ParseTreeWalker.Walk(IParseTreeListener listener, IParseTree t)
   at Rubberduck.CodeAnalysis.CodeMetrics.CodeMetricsAnalyst.TraverseModuleTree(IParseTree parseTree, DeclarationFinder declarationFinder, QualifiedModuleName moduleName) in C:\projects\rubberduck\Rubberduck.CodeAnalysis\CodeMetrics\CodeMetricsAnalyst.cs:line 54
   at Rubberduck.CodeAnalysis.CodeMetrics.CodeMetricsAnalyst.<>c__DisplayClass2_0.<GetMetrics>b__0(KeyValuePair`2 moduleTree) in C:\projects\rubberduck\Rubberduck.CodeAnalysis\CodeMetrics\CodeMetricsAnalyst.cs:line 31
   at System.Linq.Enumerable.<SelectManyIterator>d__17`2.MoveNext()
   at Rubberduck.CodeAnalysis.CodeMetrics.CodeMetricsAnalyst.<GetMetrics>d__2.MoveNext() in C:\projects\rubberduck\Rubberduck.CodeAnalysis\CodeMetrics\CodeMetricsAnalyst.cs:line 31
   at System.Linq.Lookup`2.Create[TSource](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at System.Linq.GroupedEnumerable`3.GetEnumerator()
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at Rubberduck.CodeAnalysis.CodeMetrics.CodeMetricsViewModel.Synchronize(IEnumerable`1 declarations) in C:\projects\rubberduck\Rubberduck.Core\CodeAnalysis\CodeMetrics\CodeMetricsViewModel.cs:line 66
   at Rubberduck.CodeAnalysis.CodeMetrics.CodeMetricsViewModel.OnStateChanged(Object sender, ParserStateEventArgs e) in C:\projects\rubberduck\Rubberduck.Core\CodeAnalysis\CodeMetrics\CodeMetricsViewModel.cs:line 61
   at System.EventHandler`1.Invoke(Object sender, TEventArgs e)
   at Rubberduck.Parsing.VBA.RubberduckParserState.OnStateChanged(Object requestor, CancellationToken token, ParserState state, ParserState oldStatus) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\RubberduckParserState.cs:line 402
   at Rubberduck.Parsing.VBA.RubberduckParserState.SetStatusAndFireStateChanged(Object requestor, ParserState status, CancellationToken token) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\RubberduckParserState.cs:line 617
   at Rubberduck.Parsing.VBA.ParserStateManagerBase.SetStatusAndFireStateChanged(Object requestor, ParserState status, CancellationToken token) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\ParserStateManagerBase.cs:line 52
   at Rubberduck.Parsing.VBA.ParseCoordinator.ExecuteCommonParseActivities(IReadOnlyCollection`1 toParse, IReadOnlyCollection`1 toReresolveReferencesInput, IReadOnlyCollection`1 newProjectIds, CancellationToken token) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\ParseCoordinator.cs:line 336
   at Rubberduck.Parsing.VBA.ParseCoordinator.ParseAllInternal(Object requestor, CancellationToken token) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\ParseCoordinator.cs:line 511
   at Rubberduck.Parsing.VBA.ParseCoordinator.ParseAll(Object requestor) in C:\projects\rubberduck\Rubberduck.Parsing\VBA\ParseCoordinator.cs:line 415

Additional context Restarting the program and reparsing worked. This may be due to having had parsing using bad code module. Furthermore, I was changing the name via the properties toolwindows. The parsing starts as soon as I type a letter and get cancelled when I type the next. It's conceivable that while I was renaming, it got confused about what's what and code metrics picks it up. Once it arrives into that state, reparsing does not apparently help; only restarting.

retailcoder commented 5 years ago

Sounds like CodeMetricsAnalyst could try/catch its work and wrap any exception inside some CodeMetricsException (with the original exception as the InnerException), and then we could catch that in OnStateChanged (?), log it, and not break the entire parser state.

Wouldn't fix it, but would help segregating state-change handlers' exceptions from the rest of the parse run.

MDoerner commented 5 years ago

It might be a good idea to wrap the execution of the event handlers in RubberduckParserState.OnStateChanged into a try catch block and then log and swallow all exceptions there, with the exception of OperationCanceledException.

comintern commented 5 years ago

I'm almost positive the failing line of code is this one: https://github.com/rubberduck-vba/Rubberduck/blob/next/Rubberduck.Core/Navigation/CodeExplorer/CodeExplorerItemViewModel.cs#L44

I suspect what is happening is that a Declaration is passing the equity test there that shouldn't be. That would prevent the node from being removed from the active tree, and since the context on the Declaration is the one that is being evaluated against the now current parse tree, the context search fails in the listener.

Ref #4714