Add UseOfUnboundBangNotationInspection

MDoerner · MDoerner · commit 0d803d28e31e · 2019-09-21T23:28:00.000+02:00
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UseOfRecursiveBangNotationInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UseOfRecursiveBangNotationInspection.cs
@@ -26,7 +26,7 @@ namespace Rubberduck.Inspections.Concrete
     /// <![CDATA[
     /// Public Function MyName(ByVal rst As ADODB.Recordset) As Variant
     ///     With rst
-    ///         !Name.Value
+    ///         MyName = !Name.Value
     ///     End With
     /// End Function
     /// ]]>
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UseOfUnboundBangNotationInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UseOfUnboundBangNotationInspection.cs
@@ -0,0 +1,85 @@
+﻿using System.Collections.Generic;
+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;
+using Rubberduck.VBEditor;
+
+namespace Rubberduck.Inspections.Concrete
+{
+    /// <summary>
+    /// Identifies the use of bang notation, formally known as dictionary access expression, for which the default member is not known at compile time.
+    /// </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 the default member cannot be determined at compile time.  
+    /// </why>
+    /// <example hasResult="true">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As Object) As Variant
+    ///     MyName = rst!Name.Value
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="true">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As Variant) 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!Name.Value
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As Object) As Variant
+    ///     MyName = rst("Name").Value
+    /// End Function
+    /// ]]>
+    /// </example>
+    /// <example hasResult="false">
+    /// <![CDATA[
+    /// Public Function MyName(ByVal rst As Variant) As Variant
+    ///     With rst
+    ///         MyName = .Fields.Item("Name").Value
+    ///     End With
+    /// End Function
+    /// ]]>
+    /// </example>
+    public sealed class UseOfUnboundBangNotationInspection : IdentifierReferenceInspectionBase
+    {
+        public UseOfUnboundBangNotationInspection(RubberduckParserState state)
+            : base(state)
+        {
+            Severity = CodeInspectionSeverity.Warning;
+        }
+
+        protected override IEnumerable<IdentifierReference> ReferencesInModule(QualifiedModuleName module)
+        {
+            return DeclarationFinderProvider.DeclarationFinder.UnboundDefaultMemberAccesses(module);
+        }
+
+        protected override bool IsResultReference(IdentifierReference reference)
+        {
+            return reference.IsIndexedDefaultMemberAccess
+                   && reference.Context is VBAParser.DictionaryAccessContext
+                   && !reference.IsIgnoringInspectionResultFor(AnnotationName);
+        }
+
+        protected override string ResultDescription(IdentifierReference reference)
+        {
+            var expression = reference.IdentifierName;
+            return string.Format(InspectionResults.UseOfRecursiveBangNotationInspection, expression);
+        }
+    }
+}
diff --git a/Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs b/Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs
diff --git a/Rubberduck.Resources/Inspections/InspectionInfo.de.resx b/Rubberduck.Resources/Inspections/InspectionInfo.de.resx
@@ -409,4 +409,7 @@ Falls der Parameter 'null' sein kann, bitte dieses Auftreten ignorieren. 'null'
   <data name="UseOfRecursiveBangNotationInspection" xml:space="preserve">
     <value>Die Ausrufezeichennotation erweckt den Eindruck, dass es sich um einen Zugriff handelt, der Typchecks unterliegt. Allerdings handelt es sich lediglich um einen Zugriff auf den Standardmember des Objekts, auf das sie angewendet wird, bei dem das Argument als Zeichenkette übergeben wird. Dies ist besonders verwirrend, wenn der Standardmember des Objekts selber keine Zeichenkette akzeptiert und deshalb ein rekursiver Zugriff auf den parameterlosen Standardmember erfolgen muss.</value>
   </data>
+  <data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
+    <value>Die Ausrufezeichennotation erweckt den Eindruck, dass es sich um einen Zugriff handelt, der Typchecks unterliegt. Allerdings handelt es sich lediglich um einen Zugriff auf den Standardmember des Objekts, auf das sie angewendet wird, bei dem das Argument als Zeichenkette übergeben wird. Dies ist besonders verwirrend, wenn der Standardmember nicht zum Zeitpunkt des Kompilierens ermittelt werden kann.</value>
+  </data>
 </root>
diff --git a/Rubberduck.Resources/Inspections/InspectionInfo.resx b/Rubberduck.Resources/Inspections/InspectionInfo.resx
@@ -409,4 +409,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu
   <data name="UseOfRecursiveBangNotationInspection" xml:space="preserve">
     <value>Bang notation, formally known as dictionary access expression, looks like it is strongly typed. However, it is actually a stringly  typed access to the paramterized default member of the object it is used on. 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. </value>
   </data>
+  <data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
+    <value>Bang notation, formally known as dictionary access expression, looks like it is strongly typed. However, it is actually a stringly  typed access to the paramterized default member of the object it is used on. This is especially misleading the default member cannot be determined at compile time.</value>
+  </data>
 </root>
diff --git a/Rubberduck.Resources/Inspections/InspectionNames.Designer.cs b/Rubberduck.Resources/Inspections/InspectionNames.Designer.cs
diff --git a/Rubberduck.Resources/Inspections/InspectionNames.de.resx b/Rubberduck.Resources/Inspections/InspectionNames.de.resx
@@ -393,4 +393,7 @@
   <data name="UseOfRecursiveBangNotationInspection" xml:space="preserve">
     <value>Verwendung rekursiver Ausrufezeichennotation</value>
   </data>
+  <data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
+    <value>Verwendung nicht gebundener Ausrufezeichennotation</value>
+  </data>
 </root>
diff --git a/Rubberduck.Resources/Inspections/InspectionNames.resx b/Rubberduck.Resources/Inspections/InspectionNames.resx
@@ -413,4 +413,7 @@
   <data name="UseOfRecursiveBangNotationInspection" xml:space="preserve">
     <value>Use of recursive bang notation</value>
   </data>
+  <data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
+    <value>Use of unbound bang notation</value>
+  </data>
 </root>
diff --git a/Rubberduck.Resources/Inspections/InspectionResults.Designer.cs b/Rubberduck.Resources/Inspections/InspectionResults.Designer.cs
diff --git a/Rubberduck.Resources/Inspections/InspectionResults.de.resx b/Rubberduck.Resources/Inspections/InspectionResults.de.resx
@@ -434,4 +434,7 @@ In Memoriam, 1972-2018</value>
   <data name="UseOfRecursiveBangNotationInspection" xml:space="preserve">
     <value>Der Ausdruck '{0}' verwendet rekursive Ausrufezeichennotation.</value>
   </data>
+  <data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
+    <value>Der Ausdruck '{0}' verwendet nicht gebundene Ausrufezeichennotation.</value>
+  </data>
 </root>
diff --git a/Rubberduck.Resources/Inspections/InspectionResults.resx b/Rubberduck.Resources/Inspections/InspectionResults.resx
@@ -466,4 +466,8 @@ In memoriam, 1972-2018</value>
     <value>The expression '{0}' contains a use of recursive bang notation.</value>
     <comment>{0} expression</comment>
   </data>
+  <data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
+    <value>The expression '{0}' contains a use of unbound bang notation.</value>
+    <comment>{0} expression</comment>
+  </data>
 </root>
diff --git a/RubberduckTests/Inspections/UseOfUnboundBangNotationInspectionTests.cs b/RubberduckTests/Inspections/UseOfUnboundBangNotationInspectionTests.cs
@@ -0,0 +1,192 @@
+﻿using System.Linq;
+using NUnit.Framework;
+using Rubberduck.Inspections.Concrete;
+using Rubberduck.Parsing.Inspections.Abstract;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.VBEditor.SafeComWrappers;
+using RubberduckTests.Mocks;
+
+namespace RubberduckTests.Inspections
+{
+    [TestFixture]
+    public class UseOfUnboundBangNotationInspectionTests : InspectionTestsBase
+    {
+        [Test]
+        [Category("Inspections")]
+        [TestCase("Class1", 0)]
+        [TestCase("Object", 1)]
+        [TestCase("Variant", 1)]
+        public void DictionaryAccessExpression(string typeName, int expectedNumberOfResults)
+        {
+            var class1Code = @"
+Public Function Foo(bar As String) As Class2
+Attribute Foo.VB_UserMemId = 0
+End Function
+";
+
+            var class2Code = @"
+Public Function Baz(bar As String) As Class2
+Attribute Baz.VB_UserMemId = 0
+End Function
+";
+
+            var moduleCode = $@"
+Private Function Foo() As Class2 
+    Dim cls As {typeName}
+    Set Foo = cls!newClassObject
+End Function
+";
+
+            var vbe = MockVbeBuilder.BuildFromModules(
+                ("Class1", class1Code, ComponentType.ClassModule),
+                ("Class2", class2Code, ComponentType.ClassModule),
+                ("Module1", moduleCode, ComponentType.StandardModule));
+
+            var inspectionResults = InspectionResults(vbe.Object);
+
+            Assert.AreEqual(expectedNumberOfResults, inspectionResults.Count());
+        }
+
+        [Test]
+        [Category("Inspections")]
+        [TestCase("Class1", 0)]
+        [TestCase("Object", 1)]
+        [TestCase("Variant", 1)]
+        public void WithDictionaryAccessExpression(string typeName, int expectedNumberOfResults)
+        {
+            var class1Code = @"
+Public Function Foo(bar As String) As Class2
+Attribute Foo.VB_UserMemId = 0
+End Function
+";
+
+            var class2Code = @"
+Public Function Baz(bar As String) As Class2
+Attribute Baz.VB_UserMemId = 0
+End Function
+";
+
+            var moduleCode = $@"
+Private Function Foo() As Class2 
+    Dim bar As {typeName}
+    With bar
+        Set Foo = !newClassObject
+    End With
+End Function
+";
+
+            var vbe = MockVbeBuilder.BuildFromModules(
+                ("Class1", class1Code, ComponentType.ClassModule),
+                ("Class2", class2Code, ComponentType.ClassModule),
+                ("Module1", moduleCode, ComponentType.StandardModule));
+
+            var inspectionResults = InspectionResults(vbe.Object);
+
+            Assert.AreEqual(expectedNumberOfResults, inspectionResults.Count());
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void ChainedUnboundDictionaryAccessExpression_OneResultEach()
+        {
+            var class1Code = @"
+Public Function Foo(bar As String) As Object
+Attribute Foo.VB_UserMemId = 0
+End Function
+";
+
+            var class2Code = @"
+Public Function Baz(bar As String) As Object
+Attribute Baz.VB_UserMemId = 0
+End Function
+";
+
+            var moduleCode = @"
+Private Function Foo() As Class2 
+    Dim cls As Object
+    Set Foo = cls!newClassObject!whatever
+End Function
+";
+
+            var vbe = MockVbeBuilder.BuildFromModules(
+                ("Class1", class1Code, ComponentType.ClassModule),
+                ("Class2", class2Code, ComponentType.ClassModule),
+                ("Module1", moduleCode, ComponentType.StandardModule));
+
+            var inspectionResults = InspectionResults(vbe.Object);
+
+            Assert.AreEqual(2, inspectionResults.Count());
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void IndexedDefaultMemberAccessExpression_NoResult()
+        {
+            var class1Code = @"
+Public Function Foo(bar As String) As Class2
+Attribute Foo.VB_UserMemId = 0
+End Function
+";
+
+            var class2Code = @"
+Public Function Baz(bar As String) As Class2
+Attribute Baz.VB_UserMemId = 0
+End Function
+";
+
+            var moduleCode = @"
+Private Function Foo() As Class2 
+    Dim cls As Object
+    Set Foo = cls(""newClassObject"")
+End Function
+";
+
+            var vbe = MockVbeBuilder.BuildFromModules(
+                ("Class1", class1Code, ComponentType.ClassModule),
+                ("Class2", class2Code, ComponentType.ClassModule),
+                ("Module1", moduleCode, ComponentType.StandardModule));
+
+            var inspectionResults = InspectionResults(vbe.Object);
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void RecursiveUnboundDictionaryAccessExpression_OneResult()
+        {
+            var class1Code = @"
+Public Function Foo() As Object
+Attribute Foo.VB_UserMemId = 0
+End Function
+";
+
+            var class2Code = @"
+Public Function Baz(bar As String) As Class2
+Attribute Baz.VB_UserMemId = 0
+End Function
+";
+
+            var moduleCode = @"
+Private Function Foo() As Class2 
+    Dim cls As New Class1
+    Set Foo = cls!newClassObject
+End Function
+";
+
+            var vbe = MockVbeBuilder.BuildFromModules(
+                ("Class1", class1Code, ComponentType.ClassModule),
+                ("Class2", class2Code, ComponentType.ClassModule),
+                ("Module1", moduleCode, ComponentType.StandardModule));
+
+            var inspectionResults = InspectionResults(vbe.Object);
+
+            Assert.AreEqual(1, inspectionResults.Count());
+        }
+
+        protected override IInspection InspectionUnderTest(RubberduckParserState state)
+        {
+            return new UseOfUnboundBangNotationInspection(state);
+        }
+    }
+}
