Merge branch 'next' into 5803_MissingSetter

Author: retailcoder

Rubberduck.CodeAnalysis/Inspections/Concrete/SuspiciousPredeclaredInstanceAccessInspection.cs

@@ -0,0 +1,99 @@
+using Rubberduck.CodeAnalysis.Inspections.Abstract;
+using Rubberduck.Parsing;
+using Rubberduck.Parsing.Grammar;
+using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.VBA;
+using Rubberduck.Parsing.VBA.DeclarationCaching;
+using Rubberduck.Resources.Inspections;
+using Tokens = Rubberduck.Resources.Tokens;
+
+namespace Rubberduck.CodeAnalysis.Inspections.Concrete
+{
+    /// <summary>
+    /// This inspection warns about references to the default instance of a class, inside that class.
+    /// </summary>
+    /// <why>
+    /// While a stateful default instance might be intentional, when it isn't it's easily a source of bugs.
+    /// Use the Me qualifier to explicitly refer to the current instance and eliminate any ambiguity.
+    /// Global state accidentally stored in a class' default instance is not shared by all other instances of that class.
+    /// </why>
+    /// <example hasResult="true">
+    /// <module name="Module1" type="Standard Module">
+    /// <![CDATA[
+    /// Option Explicit
+    /// 
+    /// Public Sub Test1()
+    ///     UserForm1.Show ' the default instance is being shown
+    /// End Sub
+    /// 
+    /// Public Sub Test2()
+    ///     With New UserForm1
+    ///         .Show ' a new instance is being shown
+    ///     End With
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// <module name="UserForm1" type="UserForm Module">
+    /// <![CDATA[
+    /// Option Explicit
+    /// Private ClickCount As Long
+    /// 
+    /// Private Sub CommandButton1_Click()
+    ///     ClickCount = ClickCount + 1
+    ///     UserForm1.TextBox1.Text = ClickCount ' only TextBox1 on the default instance is affected
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// </example>
+    /// <example hasResult="false">
+    /// <module name="Module1" type="Standard Module">
+    /// <![CDATA[
+    /// Option Explicit
+    /// 
+    /// Public Sub Test1()
+    ///     UserForm1.Show ' the default instance is being shown
+    /// End Sub
+    /// 
+    /// Public Sub Test2()
+    ///     With New UserForm1
+    ///         .Show ' a new instance is being shown
+    ///     End With
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// <module name="UserForm1" type="UserForm Module">
+    /// <![CDATA[
+    /// Option Explicit
+    /// Private ClickCount As Long
+    /// 
+    /// Private Sub CommandButton1_Click()
+    ///     ClickCount = ClickCount + 1
+    ///     Me.TextBox1.Text = ClickCount ' always works as expected
+    /// End Sub
+    /// ]]>
+    /// </module>
+    /// </example>
+    internal sealed class SuspiciousPredeclaredInstanceAccessInspection : IdentifierReferenceInspectionBase
+    {
+        public SuspiciousPredeclaredInstanceAccessInspection(IDeclarationFinderProvider declarationFinderProvider) 
+            : base(declarationFinderProvider)
+        {
+        }
+
+        protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
+        {
+            return 
+                reference.Declaration is ClassModuleDeclaration module && 
+                module.HasPredeclaredId &&
+                reference.ParentScoping.ParentDeclaration.Equals(module) &&
+                reference.Context.TryGetAncestor<VBAParser.MemberAccessExprContext>(out var expression) &&
+                reference.IdentifierName != Tokens.Me && expression.lExpression()?.GetText() == reference.IdentifierName;
+        }
+
+        protected override string ResultDescription(IdentifierReference reference)
+        {
+            reference.Context.TryGetAncestor<VBAParser.MemberAccessExprContext>(out var expression);
+            return string.Format(InspectionResults.SuspiciousPredeclaredInstanceAccessInspection, reference.IdentifierName, expression.GetText());
+        }
+    }
+}

Rubberduck.CodeAnalysis/QuickFixes/Concrete/ReplaceQualifierWithMeQuickFix.cs

@@ -0,0 +1,67 @@
+using Rubberduck.CodeAnalysis.Inspections;
+using Rubberduck.CodeAnalysis.Inspections.Concrete;
+using Rubberduck.CodeAnalysis.QuickFixes.Abstract;
+using Rubberduck.Parsing.Rewriter;
+using Rubberduck.Resources;
+
+namespace Rubberduck.CodeAnalysis.QuickFixes.Concrete
+{
+    /// <summary>
+    /// Replaces an explicit qualifier with 'Me'.
+    /// </summary>
+    /// <inspections>
+    /// <inspection name="SuspiciousPredeclaredInstanceAccessInspection" />
+    /// </inspections>
+    /// <canfix multiple="true" procedure="true" module="true" project="true" all="true" />
+    /// <example>
+    /// <before>
+    /// <![CDATA[
+    /// Option Explicit
+    /// Private ClickCount As Long
+    /// 
+    /// Private Sub CommandButton1_Click()
+    ///     ClickCount = ClickCount + 1
+    ///     ' works fine as long as the current instance is the default instance
+    ///     UserForm1.TextBox1.Text = ClickCount
+    /// End Sub
+    /// ]]>
+    /// </before>
+    /// <after>
+    /// <![CDATA[
+    /// Option Explicit
+    /// Private ClickCount As Long
+    /// 
+    /// Private Sub CommandButton1_Click()
+    ///     ClickCount = ClickCount + 1
+    ///     ' works fine regardless of which instance we're in
+    ///     Me.TextBox1.Text = ClickCount
+    /// End Sub
+    /// ]]>
+    /// </after>
+    /// </example>
+    internal class ReplaceQualifierWithMeQuickFix : QuickFixBase
+    {
+        public ReplaceQualifierWithMeQuickFix()
+            :base(typeof(SuspiciousPredeclaredInstanceAccessInspection))
+        {}
+
+        public override void Fix(IInspectionResult result, IRewriteSession rewriteSession)
+        {
+            var rewriter = rewriteSession.CheckOutModuleRewriter(result.QualifiedSelection.QualifiedName);
+
+            var context = result.Context;
+            rewriter.Replace(context.Start, Tokens.Me);
+        }
+
+        public override string Description(IInspectionResult result)
+        {
+            return Resources.Inspections.QuickFixes.ReplaceQualifierWithMeQuickFix;
+        }
+
+        public override bool CanFixMultiple => true;
+        public override bool CanFixInProcedure => true;
+        public override bool CanFixInModule => true;
+        public override bool CanFixInProject => true;
+        public override bool CanFixAll => true;
+    }
+}

Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs

@@ -460,4 +460,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu
   <data name="ReadOnlyPropertyAssignmentInspection" xml:space="preserve">
     <value>In general, the VBE editor catches this type of error and will not compile.  However, there are a few scenarios where the error is overlooked by the compiler and an error is generated at runtime.  To avoid a runtime error, implement the missing Property or Subroutine. </value>
   </data>
+  <data name="SuspiciousPredeclaredInstanceAccessInspection" xml:space="preserve">
+    <value>While a stateful default instance might be intentional, it is a common source of bugs and should be avoided. Use the 'Me' qualifier to explicitly refer to the current instance and eliminate any ambiguity.</value>
+  </data>
 </root>

Rubberduck.Resources/Inspections/InspectionInfo.resx

@@ -460,4 +460,7 @@
   <data name="ReadOnlyPropertyAssignmentInspection" xml:space="preserve">
     <value>Read-Only Property assignment</value>
   </data>
+  <data name="SuspiciousPredeclaredInstanceAccessInspection" xml:space="preserve">
+    <value>Suspicious access to a predeclared instance</value>
+  </data>
 </root>

Rubberduck.Resources/Inspections/InspectionNames.Designer.cs

@@ -540,4 +540,8 @@ In memoriam, 1972-2018</value>
     <value>Attempt to assign Read-Only Property '{0}'</value>
     <comment>{0} Property name</comment>
   </data>
+  <data name="SuspiciousPredeclaredInstanceAccessInspection" xml:space="preserve">
+    <value>Identifier '{0}' in '{1}' is suspiciously referring to the default instance of that class type.</value>
+    <comment>{0} identifier name; {1} expression/context</comment>
+  </data>
 </root>

Rubberduck.Resources/Inspections/InspectionNames.resx

@@ -309,4 +309,7 @@
   <data name="AnnotateEntryPointQuickFix" xml:space="preserve">
     <value>Add @EntryPoint annotation</value>
   </data>
+  <data name="ReplaceQualifierWithMeQuickFix" xml:space="preserve">
+    <value>Replace qualifier with 'Me'</value>
+  </data>
 </root>