Merge branch 'next' into EnhanceFunctionReturnValueDiscardedInspectionForBuiltInFunctions

Author: MDoerner

Rubberduck.CodeAnalysis/Inspections/Concrete/AssignmentNotUsedInspection.cs

@@ -1,10 +1,12 @@
 using System.Collections.Generic;
 using System.Linq;
+using Antlr4.Runtime;
 using Rubberduck.CodeAnalysis.Inspections.Abstract;
 using Rubberduck.CodeAnalysis.Inspections.Extensions;
 using Rubberduck.Inspections.CodePathAnalysis;
 using Rubberduck.Inspections.CodePathAnalysis.Extensions;
 using Rubberduck.Inspections.CodePathAnalysis.Nodes;
+using Rubberduck.Parsing;
 using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
@@ -67,7 +69,7 @@ private IEnumerable<IdentifierReference> UnusedAssignments(Declaration localVari
             return UnusedAssignmentReferences(tree);
         }
 
-        public static List<IdentifierReference> UnusedAssignmentReferences(INode node)
+        private static List<IdentifierReference> UnusedAssignmentReferences(INode node)
         {
             var nodes = new List<IdentifierReference>();
 
@@ -98,7 +100,8 @@ public static List<IdentifierReference> UnusedAssignmentReferences(INode node)
 
         protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
         {
-            return !IsAssignmentOfNothing(reference);
+            return !(IsAssignmentOfNothing(reference)
+                        || IsPotentiallyUsedViaJump(reference, finder));
         }
 
         private static bool IsAssignmentOfNothing(IdentifierReference reference)
@@ -108,6 +111,142 @@ private static bool IsAssignmentOfNothing(IdentifierReference reference)
                 && setStmtContext.expression().GetText().Equals(Tokens.Nothing);
         }
 
+        /// <summary>
+        /// Filters false positive result references due to GoTo and Resume statements.  e.g., 
+        /// An ErrorHandler block that branches execution to a location where the asignment may be used. 
+        /// </summary>
+        /// <remarks>
+        /// Filters Assignment references that meet the following conditions:
+        /// 1. Precedes a GoTo or Resume statement that branches execution to a line before the 
+        ///     assignment reference, and
+        /// 2. A non-assignment reference is present on a line that is:
+        ///     a) At or below the start of the execution branch, and 
+        ///     b) Above the next ExitStatement line (if one exists) or the end of the procedure
+        /// </remarks>
+        private static bool IsPotentiallyUsedViaJump(IdentifierReference resultCandidate, DeclarationFinder finder)
+        {
+            if (!resultCandidate.Declaration.References.Any(rf => !rf.IsAssignment)) { return false; }
+
+            var labelIdLineNumberPairs = finder.DeclarationsWithType(DeclarationType.LineLabel)
+                                                .Where(label => resultCandidate.ParentScoping.Equals(label.ParentDeclaration))
+                                                .Select(lbl => (lbl.IdentifierName, lbl.Context.Start.Line));
+
+            return GotoPotentiallyUsesVariable(resultCandidate, labelIdLineNumberPairs) 
+                || ResumePotentiallyUsesVariable(resultCandidate, labelIdLineNumberPairs);
+        }
+
+        private static bool GotoPotentiallyUsesVariable(IdentifierReference resultCandidate, IEnumerable<(string, int)> labelIdLineNumberPairs)
+        {
+            if (TryGetRelevantJumpContext<VBAParser.GoToStmtContext>(resultCandidate, out var gotoStmt))
+            {
+                return IsPotentiallyUsedAssignment(gotoStmt, resultCandidate, labelIdLineNumberPairs);
+            }
+
+            return false;
+        }
+
+        private static bool ResumePotentiallyUsesVariable(IdentifierReference resultCandidate, IEnumerable<(string IdentifierName, int Line)> labelIdLineNumberPairs)
+        {
+            if (TryGetRelevantJumpContext<VBAParser.ResumeStmtContext>(resultCandidate, out var resumeStmt))
+            {
+                return IsPotentiallyUsedAssignment(resumeStmt, resultCandidate, labelIdLineNumberPairs);
+            }
+
+            return false;
+        }
+
+        private static bool TryGetRelevantJumpContext<T>(IdentifierReference resultCandidate, out T ctxt) where T : ParserRuleContext //, IEnumerable<T> stmtContexts, int targetLine, int? targetColumn = null) where T : ParserRuleContext
+        {
+            ctxt = resultCandidate.ParentScoping.Context.GetDescendents<T>()
+                                    .Where(sc => sc.Start.Line > resultCandidate.Context.Start.Line
+                                                    || (sc.Start.Line == resultCandidate.Context.Start.Line
+                                                            && sc.Start.Column > resultCandidate.Context.Start.Column))
+                                    .OrderBy(sc => sc.Start.Line - resultCandidate.Context.Start.Line)
+                                    .ThenBy(sc => sc.Start.Column - resultCandidate.Context.Start.Column)
+                                    .FirstOrDefault();
+            return ctxt != null;
+        }
+
+        private static bool IsPotentiallyUsedAssignment<T>(T jumpContext, IdentifierReference resultCandidate, IEnumerable<(string, int)> labelIdLineNumberPairs) //, int executionBranchLine)
+        {
+            int? executionBranchLine = null;
+            if (jumpContext is VBAParser.GoToStmtContext gotoCtxt)
+            {
+                executionBranchLine = DetermineLabeledExecutionBranchLine(gotoCtxt.expression().GetText(), labelIdLineNumberPairs);
+            }
+            else
+            {
+                executionBranchLine = DetermineResumeStmtExecutionBranchLine(jumpContext as VBAParser.ResumeStmtContext, resultCandidate, labelIdLineNumberPairs);
+            }
+
+            return executionBranchLine.HasValue
+                ?   AssignmentIsUsedPriorToExitStmts(resultCandidate, executionBranchLine.Value)
+                :   false;
+        }
+
+        private static bool AssignmentIsUsedPriorToExitStmts(IdentifierReference resultCandidate, int executionBranchLine)
+        {
+            if (resultCandidate.Context.Start.Line < executionBranchLine) { return false; }
+
+            var procedureExitStmtCtxts = resultCandidate.ParentScoping.Context.GetDescendents<VBAParser.ExitStmtContext>()
+                                    .Where(exitCtxt => exitCtxt.EXIT_DO() == null
+                                            && exitCtxt.EXIT_FOR() == null);
+
+            var nonAssignmentCtxts = resultCandidate.Declaration.References
+                                            .Where(rf => !rf.IsAssignment)
+                                            .Select(rf => rf.Context);
+
+            var sortedContextsAfterBranch = nonAssignmentCtxts.Concat(procedureExitStmtCtxts)
+                        .Where(ctxt => ctxt.Start.Line >= executionBranchLine)
+                        .OrderBy(ctxt => ctxt.Start.Line)
+                        .ThenBy(ctxt => ctxt.Start.Column);
+
+            return !(sortedContextsAfterBranch.FirstOrDefault() is VBAParser.ExitStmtContext);
+        }
+
+        private static int? DetermineResumeStmtExecutionBranchLine(VBAParser.ResumeStmtContext resumeStmt, IdentifierReference resultCandidate, IEnumerable<(string IdentifierName, int Line)> labelIdLineNumberPairs) //where T: ParserRuleContext
+        {
+            var onErrorGotoLabelToLineNumber = resultCandidate.ParentScoping.Context.GetDescendents<VBAParser.OnErrorStmtContext>()
+                    .Where(errorStmtCtxt => !errorStmtCtxt.expression().GetText().Equals("0"))
+                    .ToDictionary(k => k.expression()?.GetText() ?? "No Label", v => v.Start.Line);
+
+            var errorHandlerLabelsAndLines = labelIdLineNumberPairs
+                                                    .Where(pair => onErrorGotoLabelToLineNumber.ContainsKey(pair.IdentifierName));
+
+            //Labels must be located at the start of a line.
+            //If the resultCandidate line precedes all error handling related labels, 
+            //a Resume statement cannot be invoked (successfully) for the resultCandidate
+            if (!errorHandlerLabelsAndLines.Any(s => s.Line <= resultCandidate.Context.Start.Line))
+            {
+                return null;
+            }
+
+            var expression = resumeStmt.expression()?.GetText();
+
+            //For Resume and Resume Next, expression() is null
+            if (string.IsNullOrEmpty(expression))
+            {
+                //Get errorHandlerLabel for the Resume statement
+                string errorHandlerLabel = errorHandlerLabelsAndLines
+                                                .Where(pair => resumeStmt.Start.Line >= pair.Line)
+                                                .OrderBy(pair => resumeStmt.Start.Line - pair.Line)
+                                                .Select(pair => pair.IdentifierName)
+                                                .FirstOrDefault();
+
+                //Since the execution branch line for Resume and Resume Next statements 
+                //is indeterminant by static analysis, the On***GoTo statement
+                //is used as the execution branch line
+                return onErrorGotoLabelToLineNumber[errorHandlerLabel];
+            }
+            //Resume <label>
+            return DetermineLabeledExecutionBranchLine(expression, labelIdLineNumberPairs);
+        }
+
+        private static int DetermineLabeledExecutionBranchLine(string expression, IEnumerable<(string IdentifierName, int Line)> IDandLinePairs)
+                        => int.TryParse(expression, out var parsedLineNumber)
+                                        ? parsedLineNumber
+                                        : IDandLinePairs.Single(v => v.IdentifierName.Equals(expression)).Line;
+
         protected override string ResultDescription(IdentifierReference reference)
         {
             return Description;

Rubberduck.Core/UI/Command/MenuItems/CommandMenuItemBase.cs

@@ -54,7 +54,7 @@ public virtual bool EvaluateCanExecute(RubberduckParserState state)
         public virtual bool HiddenWhenDisabled => false;
         public virtual bool IsVisible => true;
         public virtual bool BeginGroup => false;
-        public virtual int DisplayOrder => default(int);
+        public virtual int DisplayOrder => default;
         public virtual Image Image => null;
         public virtual Image Mask => null;
     }

Rubberduck.Core/UI/Command/MenuItems/ParentMenus/ParentMenuItemBase.cs

@@ -62,7 +62,7 @@ public virtual Func<string> ToolTipText
         }
 
         public virtual bool BeginGroup => false;
-        public virtual int DisplayOrder => default(int);
+        public virtual int DisplayOrder => default;
 
         public void Localize()
         {
@@ -201,8 +201,7 @@ private ICommandBarControl InitializeChildControl(ICommandMenuItem item)
 
         private void child_Click(object sender, CommandBarButtonClickEventArgs e)
         {
-            var item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => e.Tag.EndsWith(menu.GetType().Name)) as ICommandMenuItem;
-            if (item == null)
+            if (!(_items.Select(kvp => kvp.Key).SingleOrDefault(menu => e.Tag.EndsWith(menu.GetType().Name)) is ICommandMenuItem item))
             {
                 return;
             }