Merge pull request #4427 from Hosch250/features2355

retailcoder · web-flow · commit df82dc635a53 · 2018-11-06T10:27:55.000-05:00
Inspection for public members with underscores in names in class modules
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnderscoreInPublicClassModuleMemberInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnderscoreInPublicClassModuleMemberInspection.cs
@@ -0,0 +1,32 @@
+using System.Collections.Generic;
+using System.Linq;
+using Rubberduck.Inspections.Abstract;
+using Rubberduck.Inspections.Results;
+using Rubberduck.Parsing.Inspections.Abstract;
+using Rubberduck.Resources.Inspections;
+using Rubberduck.Parsing.VBA;
+
+namespace Rubberduck.Inspections.Concrete
+{
+    public sealed class UnderscoreInPublicClassModuleMemberInspection : InspectionBase
+    {
+        public UnderscoreInPublicClassModuleMemberInspection(RubberduckParserState state)
+            : base(state) { }
+
+        protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
+        {
+            var interfaceMembers = State.DeclarationFinder.FindAllInterfaceImplementingMembers().ToList();
+            var eventHandlers = State.DeclarationFinder.FindEventHandlers().ToList();
+
+            var names = State.DeclarationFinder.UserDeclarations(Parsing.Symbols.DeclarationType.Member)
+                .Where(w => w.ParentDeclaration.DeclarationType == Parsing.Symbols.DeclarationType.ClassModule)
+                .Where(w => !interfaceMembers.Contains(w) && !eventHandlers.Contains(w))
+                .Where(w => w.Accessibility == Parsing.Symbols.Accessibility.Public || w.Accessibility == Parsing.Symbols.Accessibility.Implicit)
+                .Where(w => w.IdentifierName.Contains('_'))
+                .ToList();
+
+            return names.Select(issue =>
+                new DeclarationInspectionResult(this, string.Format(InspectionResults.UnderscoreInPublicClassModuleMemberInspection, issue.IdentifierName), issue));
+        }
+    }
+}
diff --git a/Rubberduck.CodeAnalysis/QuickFixes/RenameDeclarationQuickFix.cs b/Rubberduck.CodeAnalysis/QuickFixes/RenameDeclarationQuickFix.cs
@@ -18,7 +18,7 @@ public sealed class RenameDeclarationQuickFix : QuickFixBase
         private readonly IMessageBox _messageBox;
 
         public RenameDeclarationQuickFix(IVBE vbe, RubberduckParserState state, IMessageBox messageBox)
-            : base(typeof(HungarianNotationInspection), typeof(UseMeaningfulNameInspection), typeof(DefaultProjectNameInspection))
+            : base(typeof(HungarianNotationInspection), typeof(UseMeaningfulNameInspection), typeof(DefaultProjectNameInspection), typeof(UnderscoreInPublicClassModuleMemberInspection))
         {
             _vbe = vbe;
             _state = state;
diff --git a/Rubberduck.Core/Properties/Settings.Designer.cs b/Rubberduck.Core/Properties/Settings.Designer.cs
diff --git a/Rubberduck.Core/Properties/Settings.settings b/Rubberduck.Core/Properties/Settings.settings
@@ -261,6 +261,7 @@
     &lt;CodeInspection Name="IsMissingOnInappropriateArgumentInspection" Severity="Warning" InspectionType="CodeQualityIssues" /&gt;
     &lt;CodeInspection Name="IsMissingWithNonArgumentParameterInspection" Severity="Warning" InspectionType="CodeQualityIssues" /&gt;
     &lt;CodeInspection Name="AssignmentNotUsedInspection" Severity="Suggestion" InspectionType="CodeQualityIssues" /&gt;
+    &lt;CodeInspection Name="UnderscoreInPublicClassModuleMemberInspection" Severity="Warning" InspectionType="CodeQualityIssues" /&gt;
   &lt;/CodeInspections&gt;
   &lt;WhitelistedIdentifiers /&gt;
   &lt;RunInspectionsOnSuccessfulParse&gt;true&lt;/RunInspectionsOnSuccessfulParse&gt;
diff --git a/Rubberduck.Core/app.config b/Rubberduck.Core/app.config
@@ -386,6 +386,8 @@
                 Severity="Warning" InspectionType="CodeQualityIssues" />
               <CodeInspection Name="AssignmentNotUsedInspection" Severity="Suggestion"
                 InspectionType="CodeQualityIssues" />
+              <CodeInspection Name="UnderscoreInPublicClassModuleMemberInspection" Severity="Warning"
+                InspectionType="CodeQualityIssues" />
             </CodeInspections>
             <WhitelistedIdentifiers />
             <RunInspectionsOnSuccessfulParse>true</RunInspectionsOnSuccessfulParse>
diff --git a/Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs b/Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs
diff --git a/Rubberduck.Resources/Inspections/InspectionInfo.resx b/Rubberduck.Resources/Inspections/InspectionInfo.resx
@@ -349,6 +349,9 @@ If the parameter can be null, ignore this inspection result; passing a null valu
   <data name="AssignmentNotUsedInspection" xml:space="preserve">
     <value>An assignment is immediately overridden by another assignment or is never referenced.</value>
   </data>
+  <data name="UnderscoreInPublicClassModuleMemberInspection" xml:space="preserve">
+    <value>A class module that contains members with underscores cannot be implemented by other classes. The underscore is used as a separator between the interface/object name and the implemented member name: having an underscore in the member name confuses the compiler, which then refuses to compile the project. Avoid underscores in public member names by following a 'PascalCase' naming convention.</value>
+  </data>
   <data name="ExcelMemberMayReturnNothingInspection" xml:space="preserve">
     <value>A procedure that returns an object may return 'Nothing'. That will cause a runtime error 91 - "Object variable or With block variable not set" on subsequent member access. Perform an 'Is Nothing' check after the 'Set' assignment to guard against runtime errors.</value>
   </data>
diff --git a/Rubberduck.Resources/Inspections/InspectionNames.Designer.cs b/Rubberduck.Resources/Inspections/InspectionNames.Designer.cs
diff --git a/Rubberduck.Resources/Inspections/InspectionNames.resx b/Rubberduck.Resources/Inspections/InspectionNames.resx
@@ -348,6 +348,9 @@
   <data name="AssignmentNotUsedInspection" xml:space="preserve">
     <value>Assignment is not used</value>
   </data>
+  <data name="UnderscoreInPublicClassModuleMemberInspection" xml:space="preserve">
+    <value>Underscore in public class module member</value>
+  </data>
   <data name="ExcelMemberMayReturnNothingInspection" xml:space="preserve">
     <value>Member access may return 'Nothing'</value>
   </data>
diff --git a/Rubberduck.Resources/Inspections/InspectionResults.Designer.cs b/Rubberduck.Resources/Inspections/InspectionResults.Designer.cs
diff --git a/Rubberduck.Resources/Inspections/InspectionResults.resx b/Rubberduck.Resources/Inspections/InspectionResults.resx
@@ -378,6 +378,9 @@
   <data name="AssignmentNotUsedInspection" xml:space="preserve">
     <value>An assignment is immediately overridden by another assignment or is never referenced.</value>
   </data>
+  <data name="UnderscoreInPublicClassModuleMemberInspection" xml:space="preserve">
+    <value>Public member name '{0}' contains an underscore.</value>
+  </data>
   <data name="ExcelMemberMayReturnNothingInspection" xml:space="preserve">
     <value>Result of '{0}' call is not tested for 'Nothing'.</value>
     <comment>{0} Member identifier</comment>
diff --git a/RubberduckTests/Inspections/UnderscoreInPublicClassModuleMemberInspectionTests.cs b/RubberduckTests/Inspections/UnderscoreInPublicClassModuleMemberInspectionTests.cs
@@ -0,0 +1,171 @@
+﻿using NUnit.Framework;
+using Rubberduck.Inspections.Concrete;
+using Rubberduck.VBEditor.SafeComWrappers;
+using RubberduckTests.Mocks;
+using System.Linq;
+using System.Threading;
+
+namespace RubberduckTests.Inspections
+{
+    [TestFixture]
+    public class UnderscoreInPublicClassModuleMemberInspectionTests
+    {
+        [Test]
+        [Category("Inspections")]
+        public void BasicExample_Sub()
+        {
+            const string inputCode =
+                @"Public Sub Test_This_Out()
+End Sub";
+            var vbe = MockVbeBuilder.BuildFromSingleModule(inputCode, ComponentType.ClassModule, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(1, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void BasicExample_Function()
+        {
+            const string inputCode =
+                @"Public Function Test_This_Out() As Integer
+End Function";
+            var vbe = MockVbeBuilder.BuildFromSingleModule(inputCode, ComponentType.ClassModule, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(1, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void BasicExample_Property()
+        {
+            const string inputCode =
+                @"Public Property Get Test_This_Out() As Integer
+End Property";
+            var vbe = MockVbeBuilder.BuildFromSingleModule(inputCode, ComponentType.ClassModule, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(1, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void StandardModule()
+        {
+            const string inputCode =
+                @"Public Sub Test_This_Out()
+End Sub";
+            var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(0, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void NoUnderscore()
+        {
+            const string inputCode =
+                @"Public Sub Foo()
+End Sub";
+            var vbe = MockVbeBuilder.BuildFromSingleModule(inputCode, ComponentType.ClassModule, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(0, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void FriendMember_WithUnderscore()
+        {
+            const string inputCode =
+                @"Friend Sub Test_This_Out()
+End Sub";
+            var vbe = MockVbeBuilder.BuildFromSingleModule(inputCode, ComponentType.ClassModule, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(0, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void Implicit_WithUnderscore()
+        {
+            const string inputCode =
+                @"Sub Test_This_Out()
+End Sub";
+            var vbe = MockVbeBuilder.BuildFromSingleModule(inputCode, ComponentType.ClassModule, out _);
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(1, inspectionResults.Count());
+            }
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void ImplementsInterface()
+        {
+            const string inputCode1 =
+       @"Public Sub Foo()
+End Sub";
+
+            //Expectation
+            const string inputCode2 =
+                @"Implements Class1
+
+Public Sub Class1_Foo()
+    Err.Raise 5 'TODO implement interface member
+End Sub
+";
+            var builder = new MockVbeBuilder();
+            var project = builder.ProjectBuilder("TestProject1", ProjectProtection.Unprotected)
+                .AddComponent("Class1", ComponentType.ClassModule, inputCode1)
+                .AddComponent("Class2", ComponentType.ClassModule, inputCode2)
+                .Build();
+            var vbe = builder.AddProject(project).Build();
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnderscoreInPublicClassModuleMemberInspection(state);
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+                Assert.AreEqual(0, inspectionResults.Count());
+            }
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ public sealed class RenameDeclarationQuickFix : QuickFixBase`
`18`	`18`	`private readonly IMessageBox _messageBox;`
`19`	`19`
`20`	`20`	`public RenameDeclarationQuickFix(IVBE vbe, RubberduckParserState state, IMessageBox messageBox)`
`21`		`- : base(typeof(HungarianNotationInspection), typeof(UseMeaningfulNameInspection), typeof(DefaultProjectNameInspection))`
	`21`	`+ : base(typeof(HungarianNotationInspection), typeof(UseMeaningfulNameInspection), typeof(DefaultProjectNameInspection), typeof(UnderscoreInPublicClassModuleMemberInspection))`
`22`	`22`	`{`
`23`	`23`	`_vbe = vbe;`
`24`	`24`	`_state = state;`