Merge pull request #4314 from comintern/ObjectVariableNotSetInspection

Performance tune-up for ObjectVariableNotSetInspection

Author: retailcoder

Rubberduck.CodeAnalysis/Inspections/Concrete/ObjectVariableNotSetInspection.cs

@@ -6,14 +6,13 @@
 using Rubberduck.Resources.Inspections;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
-using Rubberduck.VBEditor.SafeComWrappers;
 
 namespace Rubberduck.Inspections.Concrete
 {
     public sealed class ObjectVariableNotSetInspection : InspectionBase
     {
         public ObjectVariableNotSetInspection(RubberduckParserState state)
-            : base(state) {  }
+            : base(state) { }
 
         protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
         {
@@ -36,17 +35,11 @@ private IEnumerable<IdentifierReference> InterestingReferences()
                     continue;
                 }
 
-                foreach (var reference in moduleReferences.Value)
-                {
-                    if (!IsIgnoringInspectionResultFor(reference, AnnotationName) 
-                        && VariableRequiresSetAssignmentEvaluator.NeedsSetKeywordAdded(reference, State))
-                    {
-                        result.Add(reference);
-                    }
-                }
+                result.AddRange(moduleReferences.Value.Where(reference => !reference.IsSetAssignment
+                    && VariableRequiresSetAssignmentEvaluator.RequiresSetAssignment(reference, State)));
             }
 
-            return result;
+            return result.Where(reference => !IsIgnoringInspectionResultFor(reference, AnnotationName));
         }
     }
 }

Rubberduck.CodeAnalysis/Inspections/Concrete/VariableRequiresSetAssignmentEvaluator.cs

@@ -2,35 +2,13 @@
 using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 
 namespace Rubberduck.Inspections
 {
     public static class VariableRequiresSetAssignmentEvaluator
     {
-        public static IEnumerable<Declaration> GetDeclarationsPotentiallyRequiringSetAssignment(IEnumerable<Declaration> declarations)
-        {
-            //Reduce most of the declaration list with the easy ones
-            var relevantDeclarations = declarations.Where(dec => dec.AsTypeName == Tokens.Variant
-                                 || !SymbolList.ValueTypes.Contains(dec.AsTypeName)
-                                 &&(MayRequireAssignmentUsingSet(dec) || RequiresAssignmentUsingSet(dec)));
-
-            return relevantDeclarations;
-        }
-
-        /// <summary>
-        /// Determines whether the 'Set' keyword needs to be added in the context of the specified identifier reference.
-        /// </summary>
-        /// <param name="reference">The identifier reference to analyze</param>
-        /// <param name="state">The parser state</param>
-        public static bool NeedsSetKeywordAdded(IdentifierReference reference, RubberduckParserState state)
-        {
-            var setStmtContext = reference.Context.GetAncestor<VBAParser.SetStmtContext>();
-            return setStmtContext == null && RequiresSetAssignment(reference, state);
-        }
-
         /// <summary>
         /// Determines whether the 'Set' keyword is required (whether it's present or not) for the specified identifier reference.
         /// </summary>
@@ -44,20 +22,12 @@ public static bool RequiresSetAssignment(IdentifierReference reference, Rubberdu
                 return false;
             }
 
-            var setStmtContext = reference.Context.GetAncestor<VBAParser.SetStmtContext>();
-            if (setStmtContext != null)
+            if (reference.IsSetAssignment)
             {
                 // don't assume Set keyword is legit...
                 return reference.Declaration.IsObject;
             }
 
-            var letStmtContext = reference.Context.GetAncestor<VBAParser.LetStmtContext>();
-            if (letStmtContext == null)
-            {
-                // not an assignment
-                return false;
-            }
-
             var declaration = reference.Declaration;
             if (declaration.IsArray)
             {
@@ -66,8 +36,7 @@ public static bool RequiresSetAssignment(IdentifierReference reference, Rubberdu
             }
 
             var isObjectVariable = declaration.IsObject;
-            var isVariant = declaration.IsUndeclared || declaration.AsTypeName == Tokens.Variant;
-            if (!isObjectVariable && !isVariant)
+            if (!isObjectVariable && !(declaration.IsUndeclared || Tokens.Variant.Equals(declaration.AsTypeName)))
             {
                 return false;
             }
@@ -76,23 +45,24 @@ public static bool RequiresSetAssignment(IdentifierReference reference, Rubberdu
             {
                 // get the members of the returning type, a default member could make us lie otherwise
                 var classModule = declaration.AsTypeDeclaration as ClassModuleDeclaration;
-                if (classModule?.DefaultMember != null)
+                if (classModule?.DefaultMember == null)
                 {
-                    var parameters = (classModule.DefaultMember as IParameterizedDeclaration)?.Parameters.ToArray() ?? Enumerable.Empty<ParameterDeclaration>().ToArray();
-                    if (!parameters.Any() || parameters.All(p => p.IsOptional))
-                    {
-                        // assigned declaration has a default parameterless member, which is legally being assigned here.
-                        // might be a good idea to flag that default member assignment though...
-                        return false;
-                    }
+                    return true;
                 }
-
+                var parameters = (classModule.DefaultMember as IParameterizedDeclaration)?.Parameters;
                 // assign declaration is an object without a default parameterless (or with all parameters optional) member - LHS needs a 'Set' keyword.
-                return true;
+                return parameters != null && parameters.All(p => p.IsOptional);
             }
 
             // assigned declaration is a variant. we need to know about the RHS of the assignment.
 
+            var letStmtContext = reference.Context.GetAncestor<VBAParser.LetStmtContext>();
+            if (letStmtContext == null)
+            {
+                // not an assignment
+                return false;
+            }
+
             var expression = letStmtContext.expression();
             if (expression == null)
             {
@@ -126,69 +96,12 @@ public static bool RequiresSetAssignment(IdentifierReference reference, Rubberdu
                 return true;
             }
 
-            var accessibleDeclarations = state.DeclarationFinder.GetAccessibleDeclarations(reference.ParentScoping);
-            foreach (var accessibleDeclaration in accessibleDeclarations.Where(d => d.IdentifierName == expression.GetText()))
-            {
-                if (accessibleDeclaration.DeclarationType.HasFlag(DeclarationType.ClassModule) || accessibleDeclaration.AsTypeName == Tokens.Object)
-                {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
-        private static bool MayRequireAssignmentUsingSet(Declaration declaration)
-        {
-            if (declaration.AsTypeName == Tokens.Variant)
-            {
-                return true;
-            }
-
-            if (declaration.IsArray)
-            {
-                return false;
-            }
-
-            if (declaration.AsTypeDeclaration != null)
-            {
-                if ((ClassModuleDeclaration.HasDefaultMember(declaration.AsTypeDeclaration)
-                    || declaration.AsTypeDeclaration.DeclarationType == DeclarationType.Enumeration))
-                {
-                    return false;
-                }
-            }
-
-            if (SymbolList.ValueTypes.Contains(declaration.AsTypeName))
-            {
-                return false;
-            }
-            return true;
-        }
-
-        private static bool RequiresAssignmentUsingSet(Declaration declaration)
-        {
-            if (declaration.AsTypeDeclaration != null)
-            {
-                return declaration.AsTypeDeclaration.DeclarationType == DeclarationType.ClassModule
-                        && (((IsVariableOrParameter(declaration) 
-                                && !declaration.IsSelfAssigned)
-                            || (IsMemberWithReturnType(declaration)  
-                                && declaration.IsTypeSpecified)));
-            }
-            return false;
-        }
-
-        private static bool IsMemberWithReturnType(Declaration item)
-        {
-            return (item.DeclarationType == DeclarationType.Function
-                || item.DeclarationType == DeclarationType.PropertyGet);
-        }
-
-        private static bool IsVariableOrParameter(Declaration item)
-        {
-            return item.DeclarationType == DeclarationType.Variable
-                    || item.DeclarationType == DeclarationType.Parameter;
+            // is the reference referring to something else in scope that's a object?
+            var project = Declaration.GetProjectParent(reference.ParentScoping);
+            var module = Declaration.GetModuleParent(reference.ParentScoping);
+            return state.DeclarationFinder.MatchName(expression.GetText().ToLowerInvariant())
+                .Any(decl => (decl.DeclarationType.HasFlag(DeclarationType.ClassModule) || Tokens.Object.Equals(decl.AsTypeName))
+                && AccessibilityCheck.IsAccessible(project, module, reference.ParentScoping, decl));
         }
     }
 }