Merge pull request #5164 from MDoerner/UseOfBangInspection

Adding "use of bang notation" inspection

Author: retailcoder

Rubberduck.CodeAnalysis/Inspections/Abstract/IdentifierReferenceInspectionBase.cs

@@ -0,0 +1,66 @@
+using System.Collections.Generic;
+using System.Linq;
+using Rubberduck.Inspections.Inspections.Extensions;
+using Rubberduck.Inspections.Results;
+using Rubberduck.Parsing.Inspections.Abstract;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.VBEditor;
+
+namespace Rubberduck.Inspections.Abstract
+{
+    public abstract class IdentifierReferenceInspectionBase : InspectionBase
+    {
+        protected readonly IDeclarationFinderProvider DeclarationFinderProvider;
+
+        public IdentifierReferenceInspectionBase(RubberduckParserState state)
+            : base(state)
+        {
+            DeclarationFinderProvider = state;
+        }
+
+        protected abstract bool IsResultReference(IdentifierReference reference);
+        protected abstract string ResultDescription(IdentifierReference reference);
+
+        protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
+        {
+            var results = new List<IInspectionResult>();
+            foreach (var moduleDeclaration in State.DeclarationFinder.UserDeclarations(DeclarationType.Module))
+            {
+                if (moduleDeclaration == null || moduleDeclaration.IsIgnoringInspectionResultFor(AnnotationName))
+                {
+                    continue;
+                }
+
+                var module = moduleDeclaration.QualifiedModuleName;
+                results.AddRange(DoGetInspectionResults(module));
+            }
+
+            return results;
+        }
+
+        private IEnumerable<IInspectionResult> DoGetInspectionResults(QualifiedModuleName module)
+        {
+            var objectionableReferences = ReferencesInModule(module)
+                .Where(IsResultReference);
+
+            return objectionableReferences
+                .Select(reference => InspectionResult(reference, DeclarationFinderProvider))
+                .ToList();
+        }
+
+        protected virtual IEnumerable<IdentifierReference> ReferencesInModule(QualifiedModuleName module)
+        {
+            return DeclarationFinderProvider.DeclarationFinder.IdentifierReferences(module);
+        }
+
+        protected virtual IInspectionResult InspectionResult(IdentifierReference reference, IDeclarationFinderProvider declarationFinderProvider)
+        {
+            return new IdentifierReferenceInspectionResult(
+                this,
+                ResultDescription(reference),
+                declarationFinderProvider,
+                reference);
+        }
+    }
+}

Rubberduck.CodeAnalysis/Inspections/Concrete/DefaultMemberRequiredInspection.cs

@@ -1,13 +1,11 @@
 using System.Collections.Generic;
-using System.Linq;
 using Rubberduck.Inspections.Abstract;
 using Rubberduck.Inspections.Inspections.Extensions;
-using Rubberduck.Inspections.Results;
 using Rubberduck.Parsing.Inspections;
-using Rubberduck.Parsing.Inspections.Abstract;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
 using Rubberduck.Resources.Inspections;
+using Rubberduck.VBEditor;
 
 namespace Rubberduck.CodeAnalysis.Inspections.Concrete
 {
@@ -55,43 +53,26 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
     /// End Sub
     /// ]]>
     /// </example>
-    public class DefaultMemberRequiredInspection : InspectionBase
+    public class DefaultMemberRequiredInspection : IdentifierReferenceInspectionBase
     {
-        private readonly IDeclarationFinderProvider _declarationFinderProvider;
-
         public DefaultMemberRequiredInspection(RubberduckParserState state)
             : base(state)
         {
-            _declarationFinderProvider = state;
-
             //This will most likely cause a runtime error. The exceptions are rare and should be refactored or made explicit with an @Ignore annotation.
             Severity = CodeInspectionSeverity.Error;
         }
 
-        protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
-        {
-            var finder = _declarationFinderProvider.DeclarationFinder;
-
-            var failedIndexedDefaultMemberAccesses = finder.FailedIndexedDefaultMemberAccesses();
-            return failedIndexedDefaultMemberAccesses
-                .Where(failedIndexedDefaultMemberAccess => !IsIgnored(failedIndexedDefaultMemberAccess))
-                .Select(failedIndexedDefaultMemberAccess => InspectionResult(failedIndexedDefaultMemberAccess, _declarationFinderProvider));
-        }
-
-        private bool IsIgnored(IdentifierReference assignment)
+        protected override IEnumerable<IdentifierReference> ReferencesInModule(QualifiedModuleName module)
         {
-            return assignment.IsIgnoringInspectionResultFor(AnnotationName);
+            return DeclarationFinderProvider.DeclarationFinder.FailedIndexedDefaultMemberAccesses(module);
         }
 
-        private IInspectionResult InspectionResult(IdentifierReference failedCoercion, IDeclarationFinderProvider declarationFinderProvider)
+        protected override bool IsResultReference(IdentifierReference failedIndexedDefaultMemberAccess)
         {
-            return new IdentifierReferenceInspectionResult(this,
-                ResultDescription(failedCoercion),
-                declarationFinderProvider,
-                failedCoercion);
+            return !failedIndexedDefaultMemberAccess.IsIgnoringInspectionResultFor(AnnotationName);
         }
 
-        private string ResultDescription(IdentifierReference failedIndexedDefaultMemberAccess)
+        protected override string ResultDescription(IdentifierReference failedIndexedDefaultMemberAccess)
         {
             var expression = failedIndexedDefaultMemberAccess.IdentifierName;
             var typeName = failedIndexedDefaultMemberAccess.Declaration?.FullAsTypeName;

Rubberduck.CodeAnalysis/Inspections/Concrete/ProcedureRequiredInspection.cs

@@ -1,13 +1,11 @@
 using System.Collections.Generic;
-using System.Linq;
 using Rubberduck.Inspections.Abstract;
 using Rubberduck.Inspections.Inspections.Extensions;
-using Rubberduck.Inspections.Results;
 using Rubberduck.Parsing.Inspections;
-using Rubberduck.Parsing.Inspections.Abstract;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
 using Rubberduck.Resources.Inspections;
+using Rubberduck.VBEditor;
 
 namespace Rubberduck.CodeAnalysis.Inspections.Concrete
 {
@@ -53,43 +51,26 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
     /// End Sub
     /// ]]>
     /// </example>
-    public class ProcedureRequiredInspection : InspectionBase
+    public class ProcedureRequiredInspection : IdentifierReferenceInspectionBase
     {
-        private readonly IDeclarationFinderProvider _declarationFinderProvider;
-
         public ProcedureRequiredInspection(RubberduckParserState state)
             : base(state)
         {
-            _declarationFinderProvider = state;
-
             //This will most likely cause a runtime error. The exceptions are rare and should be refactored or made explicit with an @Ignore annotation.
             Severity = CodeInspectionSeverity.Error;
         }
 
-        protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
-        {
-            var finder = _declarationFinderProvider.DeclarationFinder;
-
-            var failedProcedureCoercions = finder.FailedProcedureCoercions();
-            return failedProcedureCoercions
-                .Where(failedCoercion => !IsIgnored(failedCoercion))
-                .Select(failedCoercion => InspectionResult(failedCoercion, _declarationFinderProvider));
-        }
-
-        private bool IsIgnored(IdentifierReference assignment)
+        protected override IEnumerable<IdentifierReference> ReferencesInModule(QualifiedModuleName module)
         {
-            return assignment.IsIgnoringInspectionResultFor(AnnotationName);
+            return DeclarationFinderProvider.DeclarationFinder.FailedProcedureCoercions(module);
         }
 
-        private IInspectionResult InspectionResult(IdentifierReference failedCoercion, IDeclarationFinderProvider declarationFinderProvider)
+        protected override bool IsResultReference(IdentifierReference failedCoercion)
         {
-            return new IdentifierReferenceInspectionResult(this,
-                ResultDescription(failedCoercion),
-                declarationFinderProvider,
-                failedCoercion);
+            return !failedCoercion.IsIgnoringInspectionResultFor(AnnotationName);
         }
 
-        private string ResultDescription(IdentifierReference failedCoercion)
+        protected override string ResultDescription(IdentifierReference failedCoercion)
         {
             var expression = failedCoercion.IdentifierName;
             var typeName = failedCoercion.Declaration?.FullAsTypeName;

Rubberduck.CodeAnalysis/Inspections/Concrete/UseOfBangNotationInspection.cs

@@ -0,0 +1,78 @@
+using Rubberduck.Inspections.Abstract;
+using Rubberduck.Resources.Inspections;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Inspections.Inspections.Extensions;
+using Rubberduck.Parsing.Grammar;
+using Rubberduck.Parsing.Inspections;
+
+namespace Rubberduck.Inspections.Concrete
+{
+    /// <summary>
+    /// Identifies the use of bang notation, formally known as dictionary access expression.
+    /// </summary>
+    /// <why>
+    /// A dictionary access expression looks like a strongly typed call, but it actually is a stringly typed access to the parameterized default member of the object. 
+    /// </why>
+    /// <example hasResult="true">
+    /// <![CDATA[
+    /// Public Sub DoSomething(ByVal wkb As Excel.Workbook)
+    ///     wkb.Worksheets!MySheet.Range("A1").Value = 42
+    /// End Sub
+    /// ]]>
+    /// </example>
+    /// <example hasResult="true">
+    /// <![CDATA[
+    /// Public Sub DoSomething(ByVal wkb As Excel.Workbook)
+    ///     With wkb.Worksheets
+    ///         !MySheet.Range("A1").Value = 42
+    ///     End With
+    /// End Sub
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Sub DoSomething(ByVal wkb As Excel.Workbook)
+    ///     wkb.Worksheets("MySheet").Range("A1").Value = 42
+    /// End Sub
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Sub DoSomething(ByVal wkb As Excel.Workbook)
+    ///     wkb.Worksheets.Item("MySheet").Range("A1").Value = 42
+    /// End Sub
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Sub DoSomething(ByVal wkb As Excel.Workbook)
+    ///     With wkb.Worksheets
+    ///         .Item("MySheet").Range("A1").Value = 42
+    ///     End With
+    /// End Sub
+    /// ]]>
+    /// </example>
+    public sealed class UseOfBangNotationInspection : IdentifierReferenceInspectionBase
+    {
+        public UseOfBangNotationInspection(RubberduckParserState state)
+            : base(state)
+        {
+            Severity = CodeInspectionSeverity.Suggestion;
+        }
+
+        protected override bool IsResultReference(IdentifierReference reference)
+        {
+            return reference.IsIndexedDefaultMemberAccess
+                   && reference.DefaultMemberRecursionDepth == 1
+                   && reference.Context is VBAParser.DictionaryAccessContext
+                   && !reference.IsIgnoringInspectionResultFor(AnnotationName);
+        }
+
+        protected override string ResultDescription(IdentifierReference reference)
+        {
+            var expression = reference.IdentifierName;
+            return string.Format(InspectionResults.UseOfBangNotationInspection, expression);
+        }
+    }
+}

Rubberduck.CodeAnalysis/Inspections/Concrete/UseOfRecursiveBangNotationInspection.cs

@@ -0,0 +1,86 @@
+using Rubberduck.Inspections.Abstract;
+using Rubberduck.Resources.Inspections;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Inspections.Inspections.Extensions;
+using Rubberduck.Parsing.Grammar;
+using Rubberduck.Parsing.Inspections;
+
+namespace Rubberduck.Inspections.Concrete
+{
+    /// <summary>
+    /// Identifies the use of bang notation, formally known as dictionary access expression, for which a recursive default member resolution is necessary.
+    /// </summary>
+    /// <why>
+    /// A dictionary access expression looks like a strongly typed call, but it actually is a stringly typed access to the parameterized default member of the object.
+    /// This is especially misleading if the parameterized default member is not on the object itself and can only be reached by calling the parameterless default member first.  
+    /// </why>
+    /// <example hasResult="true">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
+    ///     MyName = rst!Name.Value
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="true">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
+    ///     With rst
+    ///         MyName = !Name.Value
+    ///     End With
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
+    ///     MyName = rst.Fields.Item("Name").Value
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
+    ///     MyName = rst("Name").Value
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
+    ///     MyName = rst.Fields!Name.Value 'see "UseOfBangNotation" inspection
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
+    ///     With rst
+    ///         MyName = .Fields.Item("Name").Value
+    ///     End With
+    /// End Function
+    /// ]]>
+    /// </example>
+    public sealed class UseOfRecursiveBangNotationInspection : IdentifierReferenceInspectionBase
+    {
+        public UseOfRecursiveBangNotationInspection(RubberduckParserState state)
+            : base(state)
+        {
+            Severity = CodeInspectionSeverity.Suggestion;
+        }
+
+        protected override bool IsResultReference(IdentifierReference reference)
+        {
+            return reference.IsIndexedDefaultMemberAccess
+                   && reference.DefaultMemberRecursionDepth > 1
+                   && reference.Context is VBAParser.DictionaryAccessContext
+                   && !reference.IsIgnoringInspectionResultFor(AnnotationName);
+        }
+
+        protected override string ResultDescription(IdentifierReference reference)
+        {
+            var expression = reference.IdentifierName;
+            return string.Format(InspectionResults.UseOfRecursiveBangNotationInspection, expression);
+        }
+    }
+}