Merge pull request rubberduck-vba#1720 from Hosch250/InspectionBugs

Inspection bugs

RetailCoder.VBE/Common/DeclarationExtensions.cs

@@ -222,12 +222,40 @@ public static IEnumerable<Declaration> FindEventHandlers(this IEnumerable<Declar
 
         public static IEnumerable<Declaration> FindBuiltInEventHandlers(this IEnumerable<Declaration> declarations)
         {
-            var handlerNames = declarations.Where(declaration => declaration.IsBuiltIn && declaration.DeclarationType == DeclarationType.Event)
+            var declarationList = declarations.ToList();
+
+            var handlerNames = declarationList.Where(declaration => declaration.IsBuiltIn && declaration.DeclarationType == DeclarationType.Event)
                                            .Select(e => e.ParentDeclaration.IdentifierName + "_" + e.IdentifierName);
 
-            return declarations.Where(declaration => !declaration.IsBuiltIn
+            // class module built-in events
+            var classModuleHandlers = declarationList.Where(item =>
+                        item.DeclarationType == DeclarationType.Procedure &&
+                        item.ParentDeclaration.DeclarationType == DeclarationType.ClassModule &&
+                        (item.IdentifierName == "Class_Initialize" || item.IdentifierName == "Class_Terminate"));
+
+            // user form built-in events
+            var userFormHandlers = declarationList.Where(item =>
+                item.DeclarationType == DeclarationType.Procedure &&
+                item.ParentDeclaration.DeclarationType == DeclarationType.ClassModule &&
+                item.QualifiedName.QualifiedModuleName.Component.Type == vbext_ComponentType.vbext_ct_MSForm &&
+                new[]
+                {
+                    "UserForm_Activate", "UserForm_AddControl", "UserForm_BeforeDragOver", "UserForm_BeforeDropOrPaste",
+                    "UserForm_Click", "UserForm_DblClick", "UserForm_Deactivate", "UserForm_Error",
+                    "UserForm_Initialize", "UserForm_KeyDown", "UserForm_KeyPress", "UserForm_KeyUp", "UserForm_Layout",
+                    "UserForm_MouseDown", "UserForm_MouseMove", "UserForm_MouseUp", "UserForm_QueryClose",
+                    "UserForm_RemoveControl", "UserForm_Resize", "UserForm_Scroll", "UserForm_Terminate",
+                    "UserForm_Zoom"
+                }.Contains(item.IdentifierName));
+
+            var handlers = declarationList.Where(declaration => !declaration.IsBuiltIn
                                                      && declaration.DeclarationType == DeclarationType.Procedure
-                                                     && handlerNames.Contains(declaration.IdentifierName));
+                                                     && handlerNames.Contains(declaration.IdentifierName)).ToList();
+
+            handlers.AddRange(classModuleHandlers);
+            handlers.AddRange(userFormHandlers);
+
+            return handlers;
         }
 
         /// <summary>

RetailCoder.VBE/Inspections/ImplicitByRefParameterInspection.cs

@@ -30,6 +30,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
                     // ParamArray parameters do not allow an explicit "ByRef" parameter mechanism.               
                     && !((ParameterDeclaration)item).IsParamArray
                     && !interfaceMembers.Select(m => m.Scope).Contains(item.ParentScope)
+                    && !UserDeclarations.FindBuiltInEventHandlers().Contains(item.ParentDeclaration)
                 let arg = item.Context as VBAParser.ArgContext
                 where arg != null && arg.BYREF() == null && arg.BYVAL() == null
                 select new QualifiedContext<VBAParser.ArgContext>(item.QualifiedName, arg))

RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs

@@ -49,7 +49,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
 
             var eventScopes = declarations.Where(item =>
                 !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event)
-                .Select(e => e.Scope);
+                .Select(e => e.Scope).Concat(declarations.FindBuiltInEventHandlers().Select(e => e.Scope));
 
             var declareScopes = declarations.Where(item =>
                     item.DeclarationType == DeclarationType.LibraryFunction

RetailCoder.VBE/Inspections/ParameterNotUsedInspection.cs

@@ -2,13 +2,11 @@
 using System.Linq;
 using Microsoft.Vbe.Interop;
 using Rubberduck.Common;
-using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
 using Rubberduck.Refactorings.RemoveParameters;
 using Rubberduck.UI;
 using Rubberduck.UI.Refactorings;
-using Rubberduck.VBEditor.VBEInterfaces.RubberduckCodePane;
 
 namespace Rubberduck.Inspections
 {
@@ -38,8 +36,9 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
             var builtInHandlers = declarations.FindBuiltInEventHandlers();
 
             var parameters = declarations.Where(parameter => parameter.DeclarationType == DeclarationType.Parameter
-                && !(parameter.Context.Parent.Parent is VBAParser.EventStmtContext)
-                && !(parameter.Context.Parent.Parent is VBAParser.DeclareStmtContext));
+                && parameter.ParentDeclaration.DeclarationType != DeclarationType.Event
+                && parameter.ParentDeclaration.DeclarationType != DeclarationType.LibraryFunction
+                && parameter.ParentDeclaration.DeclarationType != DeclarationType.LibraryProcedure);
 
             var unused = parameters.Where(parameter => !parameter.References.Any()).ToList();
             var quickFixRefactoring =

RetailCoder.VBE/Inspections/ProcedureCanBeWrittenAsFunctionInspection.cs

@@ -71,7 +71,8 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
                     if (declaration == null) { return false; }  // rather be safe than sorry
 
                     return UserDeclarations.Where(item => item.IsWithEvents)
-                            .All(withEvents => UserDeclarations.FindEventProcedures(withEvents) == null);
+                            .All(withEvents => UserDeclarations.FindEventProcedures(withEvents) == null) &&
+                            !UserDeclarations.FindBuiltInEventHandlers().Contains(declaration);
                 });
 
             return ParseTreeResults.ArgListsWithOneByRefParam

RetailCoder.VBE/Inspections/ProcedureNotUsedInspection.cs

@@ -42,6 +42,8 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
                 handlers.AddRange(forms.SelectMany(form => declarations.FindFormEventHandlers(form)));
             }
 
+            handlers.AddRange(declarations.FindBuiltInEventHandlers());
+
             var items = declarations
                 .Where(item => !IsIgnoredDeclaration(declarations, item, handlers, classes, modules)
                             && !item.IsInspectionDisabled(AnnotationName)).ToList();

RubberduckTests/Inspections/ImplicitByRefParameterInspectionTests.cs

@@ -173,6 +173,36 @@ Sub IClass1_Foo(arg1 As Integer)
             Assert.AreEqual(1, inspectionResults.Count());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ImplicitByRefParameter_DoesNotReturnResult_BuiltInEvent()
+        {
+            //Input
+            const string inputCode =
+@"Private Sub UserForm_Zoom(Percent As Integer)
+
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
+                .AddComponent("Class1", vbext_ComponentType.vbext_ct_MSForm, inputCode)
+                .Build();
+            var vbe = builder.AddProject(project).Build();
+
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState());
+
+            parser.Parse();
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ImplicitByRefParameterInspection(parser.State);
+            var inspectionResults = inspection.GetInspectionResults();
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ImplicitByRefParameter_QuickFixWorks_PassByRef()

RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs

@@ -139,6 +139,32 @@ public void ParameterCanByByVal_DoesNotReturnResult_PassedByRefAndAssigned()
             Assert.AreEqual(0, inspectionResults.Count());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ParameterCanByByVal_DoesNotReturnResult_BuiltInEventParam()
+        {
+            const string inputCode =
+@"Sub Foo(ByRef arg1 As String)
+    arg1 = ""test""
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            VBComponent component;
+            var vbe = builder.BuildFromSingleStandardModule(inputCode, out component);
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState());
+
+            parser.Parse();
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ParameterCanBeByValInspection(parser.State);
+            var inspectionResults = inspection.GetInspectionResults();
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ParameterCanByByVal_ReturnsResult_SomeParams()

RubberduckTests/Inspections/ParameterNotUsedInspectionTests.cs

@@ -92,6 +92,32 @@ public void ParameterUsed_DoesNotReturnResult()
             Assert.AreEqual(0, inspectionResults.Count());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ParameterUsed_BuiltInEventHandlerParameter_DoesNotReturnResult()
+        {
+            const string inputCode =
+@"Private Sub UserForm_BeforeDropOrPaste(ByVal Cancel As MSForms.ReturnBoolean, ByVal Control As MSForms.Control, ByVal Action As MSForms.fmAction, ByVal Data As MSForms.DataObject, ByVal X As Single, ByVal Y As Single, ByVal Effect As MSForms.ReturnEffect, ByVal Shift As Integer)
+
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            VBComponent component;
+            var vbe = builder.BuildFromSingleModule(inputCode, vbext_ComponentType.vbext_ct_MSForm, out component, new Rubberduck.VBEditor.Selection());
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState());
+
+            parser.Parse();
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
+            var inspectionResults = inspection.GetInspectionResults();
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ParameterNotUsed_ReturnsResult_SomeParamsUsed()

RubberduckTests/Inspections/ProcedureNotUsedInspectionTests.cs

@@ -194,6 +194,36 @@ public void ProcedureNotUsed_DoesNotReturnResult_EventImplementation()
             Assert.AreEqual(0, inspectionResults.Count());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ProcedureNotUsed_DoesNotReturnResult_BuiltInEventImplementation()
+        {
+            //Input
+            const string inputCode =
+@"Private Sub UserForm_BeforeDropOrPaste(ByVal Cancel As MSForms.ReturnBoolean, ByVal Control As MSForms.Control, ByVal Action As MSForms.fmAction, ByVal Data As MSForms.DataObject, ByVal X As Single, ByVal Y As Single, ByVal Effect As MSForms.ReturnEffect, ByVal Shift As Integer)
+
+End Sub";
+
+            //Arrange
+            var builder = new MockVbeBuilder();
+            var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
+                .AddComponent("Form", vbext_ComponentType.vbext_ct_MSForm, inputCode)
+                .Build();
+            var vbe = builder.AddProject(project).Build();
+
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState());
+
+            parser.Parse();
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ProcedureNotUsedInspection(parser.State);
+            var inspectionResults = inspection.GetInspectionResults();
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ProcedureNotUsed_NoResultForClassInitialize()

RubberduckTests/Inspections/ProcedureShouldBeFunctionInspectionTests.cs

@@ -290,6 +290,42 @@ public void ProcedureShouldBeFunction_DoesNotReturnResult_EventImplementation()
             Assert.AreEqual(0, inspectionResults.Count());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ProcedureShouldBeFunction_DoesNotReturnResult_BuiltInEventImplementation()
+        {
+            //Input
+            const string inputCode =
+@"Private Sub UserForm_Zoom(Percent As Integer)
+
+End Sub";
+
+            //Arrange
+            var settings = new Mock<IGeneralConfigService>();
+            var config = GetTestConfig();
+            settings.Setup(x => x.LoadConfiguration()).Returns(config);
+
+            var builder = new MockVbeBuilder();
+            var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
+                .AddComponent("Class1", vbext_ComponentType.vbext_ct_MSForm, inputCode)
+                .Build();
+            var vbe = builder.AddProject(project).Build();
+
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState());
+
+            parser.Parse();
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ProcedureCanBeWrittenAsFunctionInspection(parser.State);
+            var inspector = new Inspector(settings.Object, new IInspection[] { inspection });
+
+            var inspectionResults = inspector.FindIssuesAsync(parser.State, CancellationToken.None).Result;
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ProcedureShouldBeFunction_QuickFixWorks()