Add ImplicitContainingWorkbookReferenceInspection and ImplicitContainingWorksheetReferenceInspection

These two find unqualified references inside the corresponding document modules to certain members that point to the ActiveWorkbook or ActiveSheet when used unqualified outside the appropriate document modules. That the reference is actually to the containing document can be surprising. Because of that, they should be qualified with Me.

This also removes the corresponding false-positives for the ImplicitActiveSheetReferenceInspection and ImplicitActiveWorkbookReferenceInspection.

Technically, both the containing and active workbook/worksheet types now share a common base inspection handling (most of) the selection of the declarations whose references can be suspicious.

Author: MDoerner

Rubberduck.CodeAnalysis/Inspections/Abstract/ImplicitSheetReferenceInspectionBase.cs

@@ -0,0 +1,46 @@
+using System.Collections.Generic;
+using System.Linq;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Parsing.VBA.DeclarationCaching;
+
+namespace Rubberduck.CodeAnalysis.Inspections.Abstract
+{
+    internal abstract class ImplicitSheetReferenceInspectionBase : IdentifierReferenceInspectionFromDeclarationsBase
+    {
+        public ImplicitSheetReferenceInspectionBase(IDeclarationFinderProvider declarationFinderProvider)
+            : base(declarationFinderProvider)
+        { }
+
+        protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
+        {
+            var excel = finder.Projects
+                .SingleOrDefault(item => !item.IsUserDefined
+                                         && item.IdentifierName == "Excel");
+            if (excel == null)
+            {
+                return Enumerable.Empty<Declaration>();
+            }
+
+            var globalModules = GlobalObjectClassNames
+                .Select(className => finder.FindClassModule(className, excel, true))
+                .OfType<ModuleDeclaration>();
+
+            return globalModules
+                .SelectMany(moduleClass => moduleClass.Members)
+                .Where(declaration => TargetMemberNames.Contains(declaration.IdentifierName)
+                                      && declaration.DeclarationType.HasFlag(DeclarationType.Member)
+                                      && declaration.AsTypeName == "Range");
+        }
+
+        private static readonly string[] GlobalObjectClassNames =
+        {
+            "Global", "_Global"
+        };
+
+        private static readonly string[] TargetMemberNames =
+        {
+            "Cells", "Range", "Columns", "Rows"
+        };
+    }
+}

Rubberduck.CodeAnalysis/Inspections/Abstract/ImplicitWorkbookReferenceInspectionBase.cs

@@ -0,0 +1,46 @@
+using System.Collections.Generic;
+using System.Linq;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Parsing.VBA.DeclarationCaching;
+
+namespace Rubberduck.CodeAnalysis.Inspections.Abstract
+{
+    internal abstract class ImplicitWorkbookReferenceInspectionBase : IdentifierReferenceInspectionFromDeclarationsBase
+    {
+        internal ImplicitWorkbookReferenceInspectionBase(IDeclarationFinderProvider declarationFinderProvider)
+            : base(declarationFinderProvider)
+        { }
+
+        private static readonly string[] InterestingMembers =
+        {
+            "Worksheets", "Sheets", "Names"
+        };
+
+        private static readonly string[] InterestingClasses =
+        {
+            "_Global", "_Application", "Global", "Application"
+        };
+
+        protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
+        {
+            var excel = finder.Projects
+                .SingleOrDefault(project => project.IdentifierName == "Excel" && !project.IsUserDefined);
+            if (excel == null)
+            {
+                return Enumerable.Empty<Declaration>();
+            }
+
+            var relevantClasses = InterestingClasses
+                .Select(className => finder.FindClassModule(className, excel, true))
+                .OfType<ModuleDeclaration>();
+
+            var relevantProperties = relevantClasses
+                .SelectMany(classDeclaration => classDeclaration.Members)
+                .OfType<PropertyGetDeclaration>()
+                .Where(member => InterestingMembers.Contains(member.IdentifierName));
+
+            return relevantProperties;
+        }
+    }
+}

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitActiveSheetReferenceInspection.cs

@@ -1,5 +1,4 @@
-using System.Collections.Generic;
-using System.Linq;
+using System.Linq;
 using Rubberduck.CodeAnalysis.Inspections.Abstract;
 using Rubberduck.CodeAnalysis.Inspections.Attributes;
 using Rubberduck.Parsing.Symbols;
@@ -10,7 +9,7 @@
 namespace Rubberduck.CodeAnalysis.Inspections.Concrete
 {
     /// <summary>
-    /// Locates unqualified Worksheet.Range/Cells/Columns/Rows member calls that implicitly refer to ActiveSheet.
+    /// Locates unqualified Worksheet.Range/Cells/Columns/Rows member calls inside worksheet modules.
     /// </summary>
     /// <reference name="Excel" />
     /// <why>
@@ -42,43 +41,18 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
     /// </module>
     /// </example>
     [RequiredLibrary("Excel")]
-    internal sealed class ImplicitActiveSheetReferenceInspection : IdentifierReferenceInspectionFromDeclarationsBase
+    internal sealed class ImplicitActiveSheetReferenceInspection : ImplicitSheetReferenceInspectionBase
     {
         public ImplicitActiveSheetReferenceInspection(IDeclarationFinderProvider declarationFinderProvider)
             : base(declarationFinderProvider)
         {}
 
-        protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
+        protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
         {
-            var excel = finder.Projects
-                .SingleOrDefault(item => !item.IsUserDefined
-                                         && item.IdentifierName == "Excel");
-            if (excel == null)
-            {
-                return Enumerable.Empty<Declaration>();
-            }
-
-            var globalModules = GlobalObjectClassNames
-                .Select(className => finder.FindClassModule(className, excel, true))
-                .OfType<ModuleDeclaration>();
-
-            return globalModules
-                .SelectMany(moduleClass => moduleClass.Members)
-                .Where(declaration => TargetMemberNames.Contains(declaration.IdentifierName) 
-                                      && declaration.DeclarationType.HasFlag(DeclarationType.Member)
-                                      && declaration.AsTypeName == "Range");
+            return !(Declaration.GetModuleParent(reference.ParentNonScoping) is DocumentModuleDeclaration document)
+                || !document.SupertypeNames.Contains("Worksheet");
         }
 
-        private static readonly string[] GlobalObjectClassNames =
-        {
-            "Global", "_Global"
-        };
-
-        private static readonly string[] TargetMemberNames =
-        {
-            "Cells", "Range", "Columns", "Rows"
-        };
-
         protected override string ResultDescription(IdentifierReference reference)
         {
             return string.Format(

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitActiveWorkbookReferenceInspection.cs

@@ -40,41 +40,22 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
     /// </module>
     /// </example>
     [RequiredLibrary("Excel")]
-    internal sealed class ImplicitActiveWorkbookReferenceInspection : IdentifierReferenceInspectionFromDeclarationsBase
+    internal sealed class ImplicitActiveWorkbookReferenceInspection : ImplicitWorkbookReferenceInspectionBase
     {
         public ImplicitActiveWorkbookReferenceInspection(IDeclarationFinderProvider declarationFinderProvider)
             : base(declarationFinderProvider)
         {}
 
-        private static readonly string[] InterestingMembers =
+        private static readonly List<string> _alwaysActiveWorkbookReferenceParents = new List<string>
         {
-            "Worksheets", "Sheets", "Names"
+            "_Application", "Application"
         };
 
-        private static readonly string[] InterestingClasses =
+        protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
         {
-            "_Global", "_Application", "Global", "Application"
-        };
-
-        protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
-        {
-            var excel = finder.Projects
-                .SingleOrDefault(project => project.IdentifierName == "Excel" && !project.IsUserDefined);
-            if (excel == null)
-            {
-                return Enumerable.Empty<Declaration>();
-            }
-
-            var relevantClasses = InterestingClasses
-                .Select(className => finder.FindClassModule(className, excel, true))
-                .OfType<ModuleDeclaration>();
-
-            var relevantProperties = relevantClasses
-                .SelectMany(classDeclaration => classDeclaration.Members)
-                .OfType<PropertyGetDeclaration>()
-                .Where(member => InterestingMembers.Contains(member.IdentifierName));
-
-            return relevantProperties;
+            return !(Declaration.GetModuleParent(reference.ParentNonScoping) is DocumentModuleDeclaration document)
+                   || !document.SupertypeNames.Contains("Workbook")
+                   || _alwaysActiveWorkbookReferenceParents.Contains(reference.Declaration.ParentDeclaration.IdentifierName);
         }
 
         protected override string ResultDescription(IdentifierReference reference)

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitContainingWorkbookReferenceInspection.cs

@@ -0,0 +1,72 @@
+using System.Collections.Generic;
+using System.Linq;
+using Rubberduck.CodeAnalysis.Inspections.Abstract;
+using Rubberduck.CodeAnalysis.Inspections.Attributes;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Parsing.VBA.DeclarationCaching;
+using Rubberduck.Resources.Inspections;
+
+namespace Rubberduck.CodeAnalysis.Inspections.Concrete
+{
+    /// <summary>
+    /// Locates unqualified Workbook.Worksheets/Sheets/Names member calls inside workbook document modules that implicitly refer to the containing workbook.
+    /// </summary>
+    /// <reference name="Excel" />
+    /// <why>
+    /// Implicit references inside a workbook document module can be mistakes for implicit references to the active workbook, which is the behavior in all other modules 
+    /// By explicitly qualifying these member calls with Me, the ambiguity can be resolved.
+    /// </why>
+    /// <example hasResult="true">
+    /// <module name="ThisWorkbook" type="Document Module">
+    /// <![CDATA[
+    /// Private Sub Example()
+    ///     Dim summarySheet As Worksheet
+    ///     Set summarySheet = Worksheets("Summary") ' unqualified Worksheets is implicitly querying the containing workbook's Worksheets collection.
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// </example>
+    /// <example hasResult="false">
+    /// <module name="ThisWorkbook" type="Document Module">
+    /// <![CDATA[
+    /// Private Sub Example()
+    ///     Dim summarySheet As Worksheet
+    ///     Set summarySheet = Me.Worksheets("Summary")
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// </example>
+    [RequiredLibrary("Excel")]
+    internal sealed class ImplicitContainingWorkbookReferenceInspection : ImplicitWorkbookReferenceInspectionBase
+    {
+        public ImplicitContainingWorkbookReferenceInspection(IDeclarationFinderProvider declarationFinderProvider)
+            : base(declarationFinderProvider)
+        { }
+
+        private static readonly List<string> _alwaysActiveWorkbookReferenceParents = new List<string>
+        {
+            "_Application", "Application"
+        };
+
+        protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
+        {
+            return base.ObjectionableDeclarations(finder)
+                .Where(declaration => !_alwaysActiveWorkbookReferenceParents.Contains(declaration.ParentDeclaration.IdentifierName));
+        }
+
+        protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
+        {
+            return Declaration.GetModuleParent(reference.ParentNonScoping) is DocumentModuleDeclaration document
+                   && document.SupertypeNames.Contains("Workbook");
+        }
+
+        protected override string ResultDescription(IdentifierReference reference)
+        {
+            var referenceText = reference.Context.GetText();
+            return string.Format(
+                InspectionResults.ImplicitContainingWorkbookReferenceInspection, 
+                referenceText);
+        }
+    }
+}

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitContainingWorksheetReferenceInspection.cs

@@ -0,0 +1,59 @@
+using System.Linq;
+using Rubberduck.CodeAnalysis.Inspections.Abstract;
+using Rubberduck.CodeAnalysis.Inspections.Attributes;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Parsing.VBA.DeclarationCaching;
+using Rubberduck.Resources.Inspections;
+
+namespace Rubberduck.CodeAnalysis.Inspections.Concrete
+{
+    /// <summary>
+    /// Locates unqualified Worksheet.Range/Cells/Columns/Rows member calls inside worksheet modules that implicitly refer to the containing sheet.
+    /// </summary>
+    /// <reference name="Excel" />
+    /// <why>
+    /// Implicit references inside a worksheet document module can be mistakes for implicit references to the active worksheet, which is the behavior in all other places.
+    /// By explicitly qualifying these member calls with Me, the ambiguity can be resolved.
+    /// </why>
+    /// <example hasResult="true">
+    /// <module name="Sheet1" type="Document Module">
+    /// <![CDATA[
+    /// Private Sub Example()
+    ///     Dim foo As Range
+    ///     Set foo = Range("A1") ' Worksheet.Range implicitly from containing worksheet
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// </example>
+    /// <example hasResult="false">
+    /// <module name="Sheet1" type="Document Module">
+    /// <![CDATA[
+    /// Private Sub Example()
+    ///     Dim foo As Range
+    ///     Set foo = Me.Range("A1")
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// </example>
+    [RequiredLibrary("Excel")]
+    internal sealed class ImplicitContainingWorksheetReferenceInspection : ImplicitSheetReferenceInspectionBase
+    {
+        public ImplicitContainingWorksheetReferenceInspection(IDeclarationFinderProvider declarationFinderProvider)
+            : base(declarationFinderProvider)
+        {}
+
+        protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
+        {
+            return Declaration.GetModuleParent(reference.ParentNonScoping) is DocumentModuleDeclaration document
+                   && document.SupertypeNames.Contains("Worksheet");
+        }
+
+        protected override string ResultDescription(IdentifierReference reference)
+        {
+            return string.Format(
+                InspectionResults.ImplicitContainingWorksheetReferenceInspection,
+                reference.Declaration.IdentifierName);
+        }
+    }
+}