From bbf7eed3a1e4174fa322a5003f70bcefbbb55ab7 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 10:54:11 -0500 Subject: [PATCH 01/13] Fix ParameterCanBeByVal bugs --- .../ParameterCanBeByValInspection.cs | 20 +---- .../ParameterCanBeByValInspectionTests.cs | 89 +++++++++++++++++++ 2 files changed, 93 insertions(+), 16 deletions(-) diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index be99fcf2f5..0434ca9a3d 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -74,28 +74,16 @@ private static bool IsUsedAsByRefParam(IEnumerable declarations, De .ThenBy(arg => arg.Selection.StartColumn) .ToArray(); - for (var i = 0; i < calledProcedureArgs.Count(); i++) + foreach (var declaration in calledProcedureArgs) { - if (((VBAParser.ArgContext)calledProcedureArgs[i].Context).BYVAL() != null) + if (((VBAParser.ArgContext)declaration.Context).BYVAL() != null) { continue; } - foreach (var reference in item) + foreach (var reference in declaration.References) { - if (!(reference.Context is VBAParser.ArgContext)) - { - continue; - } - var context = ((dynamic)reference.Context.Parent).argsCall() as VBAParser.ArgContext; - if (context == null) - { - continue; - } - if (parameter.IdentifierName == context.GetText()) - { - return true; - } + if (reference.IsAssignment) { return true; } } } } diff --git a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs index 450a41216f..abc93ff728 100644 --- a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs +++ b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs @@ -218,6 +218,95 @@ public void GivenArrayParameter_ReturnsNoResult() Assert.AreEqual(0, results.Count); } + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_ReturnsResult_PassedToByRefProc_NoAssignment() + { + const string inputCode = +@"Sub DoSomething(foo As Integer) + DoSomethingElse foo +End Sub + +Sub DoSomethingElse(ByVal bar As Integer) +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + VBComponent component; + var vbe = builder.BuildFromSingleStandardModule(inputCode, out component); + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_DoesNotReturnResult_PassedToByRefProc_WithAssignment() + { + const string inputCode = +@"Sub DoSomething(foo As Integer) + DoSomethingElse foo +End Sub + +Sub DoSomethingElse(ByRef bar As Integer) + bar = 42 +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + VBComponent component; + var vbe = builder.BuildFromSingleStandardModule(inputCode, out component); + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_ReturnsResult_PassedToByValProc_WithAssignment() + { + const string inputCode = +@"Sub DoSomething(foo As Integer) + DoSomethingElse foo +End Sub + +Sub DoSomethingElse(ByVal bar As Integer) + bar = 42 +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + VBComponent component; + var vbe = builder.BuildFromSingleStandardModule(inputCode, out component); + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + [TestMethod] [TestCategory("Inspections")] public void ParameterCanBeByVal_Ignored_DoesNotReturnResult() From 4c4a4d8a7d101dceb139bccb753b0c7daaa63721 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 11:36:33 -0500 Subject: [PATCH 02/13] Mark Line Input, Input, and Get references as assignments --- .../Symbols/IdentifierReferenceResolver.cs | 8 +- RubberduckTests/Grammar/ResolverTests.cs | 75 ++++++++++++++++++- .../ParameterCanBeByValInspectionTests.cs | 68 +++++++++++++++++ 3 files changed, 146 insertions(+), 5 deletions(-) diff --git a/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs b/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs index 2fdc9124bf..73b7d68687 100644 --- a/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs +++ b/Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs @@ -163,7 +163,7 @@ private void ResolveDefault( "Default Context: Failed to resolve {0}. Binding as much as we can.", expression.GetText())); } - _boundExpressionVisitor.AddIdentifierReferences(boundExpression, _qualifiedModuleName, _currentScope, _currentParent, isAssignmentTarget, false); + _boundExpressionVisitor.AddIdentifierReferences(boundExpression, _qualifiedModuleName, _currentScope, _currentParent, isAssignmentTarget, hasExplicitLetStatement); } private void ResolveType(ParserRuleContext expression) @@ -444,7 +444,7 @@ private void ResolveRecordRange(VBAParser.RecordRangeContext recordRange) public void Resolve(VBAParser.LineInputStmtContext context) { ResolveDefault(context.markedFileNumber().expression()); - ResolveDefault(context.variableName().expression()); + ResolveDefault(context.variableName().expression(), isAssignmentTarget: true); } public void Resolve(VBAParser.WidthStmtContext context) @@ -496,7 +496,7 @@ public void Resolve(VBAParser.InputStmtContext context) ResolveDefault(context.markedFileNumber().expression()); foreach (var inputVariable in context.inputList().inputVariable()) { - ResolveDefault(inputVariable.expression()); + ResolveDefault(inputVariable.expression(), isAssignmentTarget: true); } } @@ -522,7 +522,7 @@ public void Resolve(VBAParser.GetStmtContext context) } if (context.variable() != null) { - ResolveDefault(context.variable().expression()); + ResolveDefault(context.variable().expression(), isAssignmentTarget: true); } } diff --git a/RubberduckTests/Grammar/ResolverTests.cs b/RubberduckTests/Grammar/ResolverTests.cs index 6d2480705b..415a416f9b 100644 --- a/RubberduckTests/Grammar/ResolverTests.cs +++ b/RubberduckTests/Grammar/ResolverTests.cs @@ -1848,6 +1848,26 @@ End Sub Assert.AreEqual(2, declaration.References.Count()); } + [TestMethod] + public void LineInputStmt_ReferenceIsAssignment() + { + // arrange + var code = @" +Public Sub Test() + Dim file As Integer, content + Line Input #file, content +End Sub +"; + // act + var state = Resolve(code); + + // assert + var declaration = state.AllUserDeclarations.Single(item => + item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "content"); + + Assert.IsTrue(declaration.References.Single().IsAssignment); + } + [TestMethod] public void WidthStmt_IsReferenceToLocalVariable() { @@ -1928,6 +1948,39 @@ End Sub Assert.AreEqual(2, declaration.References.Count()); } + [TestMethod] + public void InputStmt_ReferenceIsAssignment() + { + // arrange + var code = @" +Public Sub Test() + Dim str As String + Dim xCoord, yCoord, zCoord As Double + Input #1, str, xCoord, yCoord, zCoord +End Sub +"; + // act + var state = Resolve(code); + + // assert + var strDeclaration = state.AllUserDeclarations.Single(item => + item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "str"); + + var xCoordDeclaration = state.AllUserDeclarations.Single(item => + item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "xCoord"); + + var yCoordDeclaration = state.AllUserDeclarations.Single(item => + item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "yCoord"); + + var zCoordDeclaration = state.AllUserDeclarations.Single(item => + item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "zCoord"); + + Assert.IsTrue(strDeclaration.References.Single().IsAssignment); + Assert.IsTrue(xCoordDeclaration.References.Single().IsAssignment); + Assert.IsTrue(yCoordDeclaration.References.Single().IsAssignment); + Assert.IsTrue(zCoordDeclaration.References.Single().IsAssignment); + } + [TestMethod] public void PutStmt_IsReferenceToLocalVariable() { @@ -1955,7 +2008,7 @@ public void GetStmt_IsReferenceToLocalVariable() var code = @" Public Sub Test() Dim referenced As Integer - Get referenced,referenced,referenced + Get #referenced,referenced,referenced End Sub "; // act @@ -1968,6 +2021,26 @@ End Sub Assert.AreEqual(3, declaration.References.Count()); } + [TestMethod] + public void GetStmt_ReferenceIsAssignment() + { + // arrange + var code = @" +Public Sub Test() + Dim fileNumber As Integer, recordNumber, variable + Get #fileNumber, recordNumber, variable +End Sub +"; + // act + var state = Resolve(code); + + // assert + var variableDeclaration = state.AllUserDeclarations.Single(item => + item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "variable"); + + Assert.IsTrue(variableDeclaration.References.Single().IsAssignment); + } + [TestMethod] public void LineSpecialForm_IsReferenceToLocalVariable() { diff --git a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs index abc93ff728..7811ed94d3 100644 --- a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs +++ b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs @@ -333,6 +333,74 @@ Sub Foo(arg1 As String) Assert.IsFalse(inspectionResults.Any()); } + [TestMethod] + public void ParameterCanBeByVal_InterfaceMember() + { + //Input + const string inputCode1 = +@"Public Sub DoSomething(ByRef a As Integer) +End Sub"; + const string inputCode2 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByRef a As Integer) +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + public void ParameterCanBeByVal_Event() + { + //Input + const string inputCode1 = +@"Public Event Foo(ByVal arg1 As Integer, ByVal arg2 As String)"; + + const string inputCode2 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByVal arg1 As Integer, ByVal arg2 As String) +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + [TestMethod] [TestCategory("Inspections")] public void ParameterCanBeByVal_QuickFixWorks_SubNameStartsWithParamName() From 4ee1d441547ce079b90df1a800b168e2332c4c52 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 12:11:20 -0500 Subject: [PATCH 03/13] Fires for interface params where all can by byval --- .../Common/DeclarationExtensions.cs | 6 ++ .../ParameterCanBeByValInspection.cs | 53 ++++++++++-- .../ParameterCanBeByValInspectionTests.cs | 82 ++++++++++++++++++- 3 files changed, 132 insertions(+), 9 deletions(-) diff --git a/RetailCoder.VBE/Common/DeclarationExtensions.cs b/RetailCoder.VBE/Common/DeclarationExtensions.cs index 0e2ba12bfc..b9054508c2 100644 --- a/RetailCoder.VBE/Common/DeclarationExtensions.cs +++ b/RetailCoder.VBE/Common/DeclarationExtensions.cs @@ -451,6 +451,12 @@ public static IEnumerable FindInterfaceImplementationMembers(this I .Where(m => m.IdentifierName.EndsWith(interfaceMember)); } + public static IEnumerable FindInterfaceImplementationMembers(this IEnumerable declarations, Declaration interfaceDeclaration) + { + return FindInterfaceImplementationMembers(declarations) + .Where(m => m.IdentifierName == interfaceDeclaration.ComponentName + "_" + interfaceDeclaration.IdentifierName); + } + public static Declaration FindInterfaceMember(this IEnumerable declarations, Declaration implementation) { var members = FindInterfaceMembers(declarations); diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index 0434ca9a3d..c75c8dfdb2 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -21,11 +21,50 @@ public ParameterCanBeByValInspection(RubberduckParserState state) public override IEnumerable GetInspectionResults() { var declarations = UserDeclarations.ToList(); + var issues = new List(); - IEnumerable interfaceMembers = null; - interfaceMembers = declarations.FindInterfaceMembers() - .Concat(declarations.FindInterfaceImplementationMembers()) - .ToList(); + var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList(); + var interfaceImplementationMembers = declarations.FindInterfaceImplementationMembers().ToList(); + var allInterfaceMembers = interfaceImplementationMembers.Concat(interfaceDeclarationMembers); + + foreach (var member in interfaceDeclarationMembers) + { + var declarationParameters = + declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == member) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + var parametersAreByRef = declarationParameters.Select(s => true).ToList(); + + var implementations = declarations.FindInterfaceImplementationMembers(member).ToList(); + foreach (var implementation in implementations) + { + var parameters = + declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == implementation) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + for (var i = 0; i < parameters.Count; i++) + { + parametersAreByRef[i] = parametersAreByRef[i] && !IsUsedAsByRefParam(declarations, parameters[i]) && + ((VBAParser.ArgContext)parameters[i].Context).BYVAL() == null && + !parameters[i].References.Any(reference => reference.IsAssignment); + } + } + + for (var i = 0; i < declarationParameters.Count; i++) + { + if (parametersAreByRef[i]) + { + issues.Add(new ParameterCanBeByValInspectionResult(this, declarationParameters[i], + declarationParameters[i].Context, declarationParameters[i].QualifiedName)); + } + } + } var formEventHandlerScopes = State.FindFormEventHandlers() .Select(handler => handler.Scope); @@ -41,15 +80,15 @@ public override IEnumerable GetInspectionResults() var ignoredScopes = formEventHandlerScopes.Concat(eventScopes).Concat(declareScopes); - var issues = declarations.Where(declaration => + issues.AddRange(declarations.Where(declaration => !declaration.IsArray && !ignoredScopes.Contains(declaration.ParentScope) && declaration.DeclarationType == DeclarationType.Parameter - && !interfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope) + && !allInterfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope) && ((VBAParser.ArgContext)declaration.Context).BYVAL() == null && !IsUsedAsByRefParam(declarations, declaration) && !declaration.References.Any(reference => reference.IsAssignment)) - .Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName)); + .Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName))); return issues; } diff --git a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs index 7811ed94d3..9852f6380a 100644 --- a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs +++ b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs @@ -334,7 +334,7 @@ Sub Foo(arg1 As String) } [TestMethod] - public void ParameterCanBeByVal_InterfaceMember() + public void ParameterCanBeByVal_InterfaceMember_SingleParam() { //Input const string inputCode1 = @@ -351,6 +351,43 @@ Private Sub IClass1_DoSomething(ByRef a As Integer) var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) .AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + public void ParameterCanBeByVal_InterfaceMember_SingleParamUsedByRef() + { + //Input + const string inputCode1 = +@"Public Sub DoSomething(ByRef a As Integer) +End Sub"; + const string inputCode2 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByRef a As Integer) + a = 42 +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) .Build(); var vbe = builder.AddProject(project).Build(); @@ -366,7 +403,48 @@ Private Sub IClass1_DoSomething(ByRef a As Integer) Assert.IsFalse(inspectionResults.Any()); } - + + [TestMethod] + public void ParameterCanBeByVal_InterfaceMember_MultipleParams_OneCanBeByVal() + { + //Input + const string inputCode1 = +@"Public Sub DoSomething(ByRef a As Integer, ByRef b As Integer) +End Sub"; + const string inputCode2 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer) + b = 42 +End Sub"; + const string inputCode3 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer) +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode3) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual("a", inspectionResults.Single().Target.IdentifierName); + } + [TestMethod] public void ParameterCanBeByVal_Event() { From 7079946be5f94100cecaf336a5b03dfca48bb5a5 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 13:30:15 -0500 Subject: [PATCH 04/13] Quick fix works for interfaces --- .../ParameterCanBeByValInspection.cs | 4 +- .../ParameterCanBeByValInspectionResult.cs | 67 ++++++++++++++++--- .../ParameterCanBeByValInspectionTests.cs | 65 ++++++++++++++++++ 3 files changed, 125 insertions(+), 11 deletions(-) diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index c75c8dfdb2..3ac6b43db0 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -60,7 +60,7 @@ public override IEnumerable GetInspectionResults() { if (parametersAreByRef[i]) { - issues.Add(new ParameterCanBeByValInspectionResult(this, declarationParameters[i], + issues.Add(new ParameterCanBeByValInspectionResult(this, State, declarationParameters[i], declarationParameters[i].Context, declarationParameters[i].QualifiedName)); } } @@ -88,7 +88,7 @@ public override IEnumerable GetInspectionResults() && ((VBAParser.ArgContext)declaration.Context).BYVAL() == null && !IsUsedAsByRefParam(declarations, declaration) && !declaration.References.Any(reference => reference.IsAssignment)) - .Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName))); + .Select(issue => new ParameterCanBeByValInspectionResult(this, State, issue, issue.Context, issue.QualifiedName))); return issues; } diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs index b7cb0e2c53..03e796aa6c 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs @@ -1,7 +1,11 @@ using System.Collections.Generic; +using System.Linq; using Antlr4.Runtime; +using Rubberduck.Common; +using Rubberduck.Parsing; using Rubberduck.Parsing.Grammar; using Rubberduck.Parsing.Symbols; +using Rubberduck.Parsing.VBA; using Rubberduck.VBEditor; namespace Rubberduck.Inspections @@ -10,12 +14,12 @@ public class ParameterCanBeByValInspectionResult : InspectionResultBase { private readonly IEnumerable _quickFixes; - public ParameterCanBeByValInspectionResult(IInspection inspection, Declaration target, ParserRuleContext context, QualifiedMemberName qualifiedName) + public ParameterCanBeByValInspectionResult(IInspection inspection, RubberduckParserState state, Declaration target, ParserRuleContext context, QualifiedMemberName qualifiedName) : base(inspection, qualifiedName.QualifiedModuleName, context, target) { _quickFixes = new CodeInspectionQuickFix[] { - new PassParameterByValueQuickFix(Context, QualifiedSelection), + new PassParameterByValueQuickFix(state, Target, Context, QualifiedSelection), new IgnoreOnceQuickFix(Context, QualifiedSelection, inspection.AnnotationName) }; } @@ -30,21 +34,66 @@ public override string Description public class PassParameterByValueQuickFix : CodeInspectionQuickFix { - public PassParameterByValueQuickFix(ParserRuleContext context, QualifiedSelection selection) + private readonly RubberduckParserState _state; + private readonly Declaration _target; + + public PassParameterByValueQuickFix(RubberduckParserState state, Declaration target, ParserRuleContext context, QualifiedSelection selection) : base(context, selection, InspectionsUI.PassParameterByValueQuickFix) { + _state = state; + _target = target; } public override void Fix() { - var selection = Selection.Selection; - var selectionLength = ((VBAParser.ArgContext) Context).BYREF() == null ? 0 : 6; + if (!_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) + { + FixMethod((VBAParser.ArgContext) Context, Selection); + } + else + { + var declarationParameters = + _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == _target.ParentDeclaration) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + var parameterIndex = declarationParameters.IndexOf(_target); + + if (parameterIndex == -1) + { + return; // should only happen if the parse results are stale; prevents a crash in that case + } + + var implementations = _state.AllUserDeclarations.FindInterfaceImplementationMembers(_target.ParentDeclaration); + foreach (var member in implementations) + { + var parameters = + _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == member) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + FixMethod((VBAParser.ArgContext) parameters[parameterIndex].Context, + parameters[parameterIndex].QualifiedSelection); + } + + FixMethod((VBAParser.ArgContext) declarationParameters[parameterIndex].Context, + declarationParameters[parameterIndex].QualifiedSelection); + } + } + + private void FixMethod(VBAParser.ArgContext context, QualifiedSelection qualifiedSelection) + { + var selectionLength = context.BYREF() == null ? 0 : 6; - var module = Selection.QualifiedName.Component.CodeModule; - var lines = module.Lines[selection.StartLine, 1]; + var module = qualifiedSelection.QualifiedName.Component.CodeModule; + var lines = module.Lines[context.Start.Line, 1]; - var result = lines.Remove(selection.StartColumn - 1, selectionLength).Insert(selection.StartColumn - 1, Tokens.ByVal + ' '); - module.ReplaceLine(selection.StartLine, result); + var result = lines.Remove(context.Start.Column, selectionLength).Insert(context.Start.Column, Tokens.ByVal + ' '); + module.ReplaceLine(context.Start.Line, result); } } } diff --git a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs index 9852f6380a..a0679f8146 100644 --- a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs +++ b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs @@ -638,6 +638,71 @@ public void ParameterCanBeByVal_QuickFixWorks_PassedByRef_MultilineParameter() Assert.AreEqual(expectedCode, module.Lines()); } + [TestMethod] + public void ParameterCanBeByVal_InterfaceMember_MultipleParams_OneCanBeByVal_QuickFixWorks() + { + //Input + const string inputCode1 = +@"Public Sub DoSomething(ByRef a As Integer, ByRef b As Integer) +End Sub"; + const string inputCode2 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer) + b = 42 +End Sub"; + const string inputCode3 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer) +End Sub"; + + //Expected + const string expectedCode1 = +@"Public Sub DoSomething(ByVal a As Integer, ByRef b As Integer) +End Sub"; + const string expectedCode2 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByVal a As Integer, ByRef b As Integer) + b = 42 +End Sub"; + const string expectedCode3 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByVal a As Integer, ByRef b As Integer) +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode3) + .Build(); + + var module1 = project.Object.VBComponents.Item("IClass1").CodeModule; + var module2 = project.Object.VBComponents.Item("Class1").CodeModule; + var module3 = project.Object.VBComponents.Item("Class2").CodeModule; + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + inspectionResults.Single().QuickFixes.Single(s => s is PassParameterByValueQuickFix).Fix(); + + Assert.AreEqual(expectedCode1, module1.Lines()); + Assert.AreEqual(expectedCode2, module2.Lines()); + Assert.AreEqual(expectedCode3, module3.Lines()); + } + [TestMethod] [TestCategory("Inspections")] public void ParameterCanBeByVal_IgnoreQuickFixWorks() From 59e940bf1df44bd3170a6658a03c17dfd0622a1b Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 13:36:37 -0500 Subject: [PATCH 05/13] Clean up inspection/quick fix --- .../ParameterCanBeByValInspection.cs | 63 ++++++++++--------- .../ParameterCanBeByValInspectionResult.cs | 57 +++++++++-------- 2 files changed, 64 insertions(+), 56 deletions(-) diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index 3ac6b43db0..aff44aa098 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -24,9 +24,38 @@ public override IEnumerable GetInspectionResults() var issues = new List(); var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList(); - var interfaceImplementationMembers = declarations.FindInterfaceImplementationMembers().ToList(); - var allInterfaceMembers = interfaceImplementationMembers.Concat(interfaceDeclarationMembers); + var allInterfaceMembers = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers); + issues.AddRange(GetInterfaceResults(declarations, interfaceDeclarationMembers)); + var formEventHandlerScopes = State.FindFormEventHandlers() + .Select(handler => handler.Scope); + + var eventScopes = declarations.Where(item => + !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event) + .Select(e => e.Scope).Concat(State.AllDeclarations.FindBuiltInEventHandlers().Select(e => e.Scope)); + + var declareScopes = declarations.Where(item => + item.DeclarationType == DeclarationType.LibraryFunction + || item.DeclarationType == DeclarationType.LibraryProcedure) + .Select(e => e.Scope); + + var ignoredScopes = formEventHandlerScopes.Concat(eventScopes).Concat(declareScopes); + + issues.AddRange(declarations.Where(declaration => + !declaration.IsArray + && !ignoredScopes.Contains(declaration.ParentScope) + && declaration.DeclarationType == DeclarationType.Parameter + && !allInterfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope) + && ((VBAParser.ArgContext)declaration.Context).BYVAL() == null + && !IsUsedAsByRefParam(declarations, declaration) + && !declaration.References.Any(reference => reference.IsAssignment)) + .Select(issue => new ParameterCanBeByValInspectionResult(this, State, issue, issue.Context, issue.QualifiedName))); + + return issues; + } + + private IEnumerable GetInterfaceResults(List declarations, List interfaceDeclarationMembers) + { foreach (var member in interfaceDeclarationMembers) { var declarationParameters = @@ -60,37 +89,11 @@ public override IEnumerable GetInspectionResults() { if (parametersAreByRef[i]) { - issues.Add(new ParameterCanBeByValInspectionResult(this, State, declarationParameters[i], - declarationParameters[i].Context, declarationParameters[i].QualifiedName)); + yield return new ParameterCanBeByValInspectionResult(this, State, declarationParameters[i], + declarationParameters[i].Context, declarationParameters[i].QualifiedName); } } } - - var formEventHandlerScopes = State.FindFormEventHandlers() - .Select(handler => handler.Scope); - - var eventScopes = declarations.Where(item => - !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event) - .Select(e => e.Scope).Concat(State.AllDeclarations.FindBuiltInEventHandlers().Select(e => e.Scope)); - - var declareScopes = declarations.Where(item => - item.DeclarationType == DeclarationType.LibraryFunction - || item.DeclarationType == DeclarationType.LibraryProcedure) - .Select(e => e.Scope); - - var ignoredScopes = formEventHandlerScopes.Concat(eventScopes).Concat(declareScopes); - - issues.AddRange(declarations.Where(declaration => - !declaration.IsArray - && !ignoredScopes.Contains(declaration.ParentScope) - && declaration.DeclarationType == DeclarationType.Parameter - && !allInterfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope) - && ((VBAParser.ArgContext)declaration.Context).BYVAL() == null - && !IsUsedAsByRefParam(declarations, declaration) - && !declaration.References.Any(reference => reference.IsAssignment)) - .Select(issue => new ParameterCanBeByValInspectionResult(this, State, issue, issue.Context, issue.QualifiedName))); - - return issues; } private static bool IsUsedAsByRefParam(IEnumerable declarations, Declaration parameter) diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs index 03e796aa6c..3aef0ac926 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs @@ -46,43 +46,48 @@ public PassParameterByValueQuickFix(RubberduckParserState state, Declaration tar public override void Fix() { - if (!_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) + if (_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) { - FixMethod((VBAParser.ArgContext) Context, Selection); + FixInterfaceMethods(); } else { - var declarationParameters = + FixMethod((VBAParser.ArgContext)Context, Selection); + } + } + + private void FixInterfaceMethods() + { + var declarationParameters = _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && declaration.ParentDeclaration == _target.ParentDeclaration) .OrderBy(o => o.Selection.StartLine) .ThenBy(t => t.Selection.StartColumn) .ToList(); - var parameterIndex = declarationParameters.IndexOf(_target); - - if (parameterIndex == -1) - { - return; // should only happen if the parse results are stale; prevents a crash in that case - } - - var implementations = _state.AllUserDeclarations.FindInterfaceImplementationMembers(_target.ParentDeclaration); - foreach (var member in implementations) - { - var parameters = - _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == member) - .OrderBy(o => o.Selection.StartLine) - .ThenBy(t => t.Selection.StartColumn) - .ToList(); - - FixMethod((VBAParser.ArgContext) parameters[parameterIndex].Context, - parameters[parameterIndex].QualifiedSelection); - } - - FixMethod((VBAParser.ArgContext) declarationParameters[parameterIndex].Context, - declarationParameters[parameterIndex].QualifiedSelection); + var parameterIndex = declarationParameters.IndexOf(_target); + + if (parameterIndex == -1) + { + return; // should only happen if the parse results are stale; prevents a crash in that case } + + var implementations = _state.AllUserDeclarations.FindInterfaceImplementationMembers(_target.ParentDeclaration); + foreach (var member in implementations) + { + var parameters = + _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == member) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + FixMethod((VBAParser.ArgContext)parameters[parameterIndex].Context, + parameters[parameterIndex].QualifiedSelection); + } + + FixMethod((VBAParser.ArgContext)declarationParameters[parameterIndex].Context, + declarationParameters[parameterIndex].QualifiedSelection); } private void FixMethod(VBAParser.ArgContext context, QualifiedSelection qualifiedSelection) From b94d23fa29242ee9710fd0f5ef3bc33293bfe09b Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 14:04:09 -0500 Subject: [PATCH 06/13] Correctly fires on events --- .../Common/DeclarationExtensions.cs | 16 ++ .../ParameterCanBeByValInspection.cs | 70 ++++++-- .../ParameterCanBeByValInspectionResult.cs | 1 - .../ParameterCanBeByValInspectionTests.cs | 154 +++++++++++++++++- 4 files changed, 224 insertions(+), 17 deletions(-) diff --git a/RetailCoder.VBE/Common/DeclarationExtensions.cs b/RetailCoder.VBE/Common/DeclarationExtensions.cs index b9054508c2..0ecfcc13ac 100644 --- a/RetailCoder.VBE/Common/DeclarationExtensions.cs +++ b/RetailCoder.VBE/Common/DeclarationExtensions.cs @@ -262,6 +262,22 @@ public static IEnumerable FindEventHandlers(this IEnumerable FindUserEventHandlers(this IEnumerable declarations) + { + var declarationList = declarations.ToList(); + + var userEvents = + declarationList.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList(); + + var handlers = new List(); + foreach (var @event in userEvents) + { + handlers.AddRange(declarations.FindHandlersForEvent(@event).Select(s => s.Item2)); + } + + return handlers; + } + public static IEnumerable FindBuiltInEventHandlers(this IEnumerable declarations) { var declarationList = declarations.ToList(); diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index aff44aa098..f2e457d201 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -24,28 +24,30 @@ public override IEnumerable GetInspectionResults() var issues = new List(); var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList(); - var allInterfaceMembers = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers); + var interfaceScopes = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers).Select(s => s.Scope); + issues.AddRange(GetInterfaceResults(declarations, interfaceDeclarationMembers)); - var formEventHandlerScopes = State.FindFormEventHandlers() - .Select(handler => handler.Scope); + var eventMembers = declarations.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList(); + var formEventHandlerScopes = State.FindFormEventHandlers().Select(handler => handler.Scope); + var eventHandlerScopes = State.AllDeclarations.FindBuiltInEventHandlers().Concat(declarations.FindUserEventHandlers()).Select(e => e.Scope); + var eventScopes = eventMembers.Select(s => s.Scope) + .Concat(formEventHandlerScopes) + .Concat(eventHandlerScopes); - var eventScopes = declarations.Where(item => - !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event) - .Select(e => e.Scope).Concat(State.AllDeclarations.FindBuiltInEventHandlers().Select(e => e.Scope)); + issues.AddRange(GetEventResults(declarations, eventMembers)); var declareScopes = declarations.Where(item => item.DeclarationType == DeclarationType.LibraryFunction || item.DeclarationType == DeclarationType.LibraryProcedure) .Select(e => e.Scope); - - var ignoredScopes = formEventHandlerScopes.Concat(eventScopes).Concat(declareScopes); - + issues.AddRange(declarations.Where(declaration => !declaration.IsArray - && !ignoredScopes.Contains(declaration.ParentScope) + && !declareScopes.Contains(declaration.ParentScope) + && !eventScopes.Contains(declaration.ParentScope) + && !interfaceScopes.Contains(declaration.ParentScope) && declaration.DeclarationType == DeclarationType.Parameter - && !allInterfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope) && ((VBAParser.ArgContext)declaration.Context).BYVAL() == null && !IsUsedAsByRefParam(declarations, declaration) && !declaration.References.Any(reference => reference.IsAssignment)) @@ -96,6 +98,48 @@ private IEnumerable GetInterfaceResults(Lis } } + private IEnumerable GetEventResults(List declarations, List eventDeclarationMembers) + { + foreach (var member in eventDeclarationMembers) + { + var declarationParameters = + declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == member) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + var parametersAreByRef = declarationParameters.Select(s => true).ToList(); + + var handlers = declarations.FindHandlersForEvent(member).Select(s => s.Item2).ToList(); + foreach (var handler in handlers) + { + var parameters = + declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == handler) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + for (var i = 0; i < parameters.Count; i++) + { + parametersAreByRef[i] = parametersAreByRef[i] && !IsUsedAsByRefParam(declarations, parameters[i]) && + ((VBAParser.ArgContext)parameters[i].Context).BYVAL() == null && + !parameters[i].References.Any(reference => reference.IsAssignment); + } + } + + for (var i = 0; i < declarationParameters.Count; i++) + { + if (parametersAreByRef[i]) + { + yield return new ParameterCanBeByValInspectionResult(this, State, declarationParameters[i], + declarationParameters[i].Context, declarationParameters[i].QualifiedName); + } + } + } + } + private static bool IsUsedAsByRefParam(IEnumerable declarations, Declaration parameter) { // find the procedure calls in the procedure of the parameter. @@ -123,9 +167,9 @@ private static bool IsUsedAsByRefParam(IEnumerable declarations, De continue; } - foreach (var reference in declaration.References) + if (declaration.References.Any(reference => reference.IsAssignment)) { - if (reference.IsAssignment) { return true; } + return true; } } } diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs index 3aef0ac926..3672ae49db 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs @@ -2,7 +2,6 @@ using System.Linq; using Antlr4.Runtime; using Rubberduck.Common; -using Rubberduck.Parsing; using Rubberduck.Parsing.Grammar; using Rubberduck.Parsing.Symbols; using Rubberduck.Parsing.VBA; diff --git a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs index a0679f8146..07a6abecab 100644 --- a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs +++ b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs @@ -334,6 +334,7 @@ Sub Foo(arg1 As String) } [TestMethod] + [TestCategory("Inspections")] public void ParameterCanBeByVal_InterfaceMember_SingleParam() { //Input @@ -369,6 +370,43 @@ Private Sub IClass1_DoSomething(ByRef a As Integer) } [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_InterfaceMember_SingleByValParam() + { + //Input + const string inputCode1 = +@"Public Sub DoSomething(ByVal a As Integer) +End Sub"; + const string inputCode2 = +@"Implements IClass1 + +Private Sub IClass1_DoSomething(ByVal a As Integer) +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [TestCategory("Inspections")] public void ParameterCanBeByVal_InterfaceMember_SingleParamUsedByRef() { //Input @@ -405,6 +443,7 @@ Private Sub IClass1_DoSomething(ByRef a As Integer) } [TestMethod] + [TestCategory("Inspections")] public void ParameterCanBeByVal_InterfaceMember_MultipleParams_OneCanBeByVal() { //Input @@ -446,16 +485,88 @@ Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer) } [TestMethod] - public void ParameterCanBeByVal_Event() + [TestCategory("Inspections")] + public void ParameterCanBeByVal_EventMember_SingleParam() { //Input const string inputCode1 = -@"Public Event Foo(ByVal arg1 As Integer, ByVal arg2 As String)"; +@"Public Event Foo(ByRef arg1 As Integer)"; const string inputCode2 = @"Private WithEvents abc As Class1 -Private Sub abc_Foo(ByVal arg1 As Integer, ByVal arg2 As String) +Private Sub abc_Foo(ByRef arg1 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_ClassModule, inputCode1) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual(1, inspectionResults.Count()); + } + + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_EventMember_SingleByValParam() + { + //Input + const string inputCode1 = +@"Public Event Foo(ByVal arg1 As Integer)"; + + const string inputCode2 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByVal arg1 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_ClassModule, inputCode1) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.IsFalse(inspectionResults.Any()); + } + + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_EventMember_SingleParamUsedByRef() + { + //Input + const string inputCode1 = +@"Public Event Foo(ByRef arg1 As Integer)"; + + const string inputCode2 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByRef arg1 As Integer) + arg1 = 42 End Sub"; //Arrange @@ -479,6 +590,42 @@ Private Sub abc_Foo(ByVal arg1 As Integer, ByVal arg2 As String) Assert.IsFalse(inspectionResults.Any()); } + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_EventMember_MultipleParams_OneCanBeByVal() + { + //Input + const string inputCode1 = +@"Public Event Foo(ByRef arg1 As Integer, ByRef arg2 As Integer)"; + + const string inputCode2 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByRef arg1 As Integer, ByRef arg2 As Integer) + arg1 = 42 +End Sub"; + + //Arrange + var builder = new MockVbeBuilder(); + var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) + .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .Build(); + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + Assert.AreEqual("arg2", inspectionResults.Single().Target.IdentifierName); + } + [TestMethod] [TestCategory("Inspections")] public void ParameterCanBeByVal_QuickFixWorks_SubNameStartsWithParamName() @@ -639,6 +786,7 @@ public void ParameterCanBeByVal_QuickFixWorks_PassedByRef_MultilineParameter() } [TestMethod] + [TestCategory("Inspections")] public void ParameterCanBeByVal_InterfaceMember_MultipleParams_OneCanBeByVal_QuickFixWorks() { //Input From 9b2ddce013116838e99a90de6f7a5797a6ef78b2 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 14:12:28 -0500 Subject: [PATCH 07/13] Event quick fix works --- .../ParameterCanBeByValInspectionResult.cs | 40 ++++++++++- .../ParameterCanBeByValInspectionTests.cs | 68 +++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs index 3672ae49db..4a35840eca 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs @@ -45,7 +45,11 @@ public PassParameterByValueQuickFix(RubberduckParserState state, Declaration tar public override void Fix() { - if (_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) + if (_target.ParentDeclaration.DeclarationType == DeclarationType.Event) + { + FixEventMethods(); + } + else if (_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) { FixInterfaceMethods(); } @@ -65,7 +69,6 @@ private void FixInterfaceMethods() .ToList(); var parameterIndex = declarationParameters.IndexOf(_target); - if (parameterIndex == -1) { return; // should only happen if the parse results are stale; prevents a crash in that case @@ -89,6 +92,39 @@ private void FixInterfaceMethods() declarationParameters[parameterIndex].QualifiedSelection); } + private void FixEventMethods() + { + var declarationParameters = + _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == _target.ParentDeclaration) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + var parameterIndex = declarationParameters.IndexOf(_target); + if (parameterIndex == -1) + { + return; // should only happen if the parse results are stale; prevents a crash in that case + } + + var handlers = _state.AllUserDeclarations.FindHandlersForEvent(_target.ParentDeclaration).Select(s => s.Item2).ToList(); + foreach (var handler in handlers) + { + var parameters = + _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && + declaration.ParentDeclaration == handler) + .OrderBy(o => o.Selection.StartLine) + .ThenBy(t => t.Selection.StartColumn) + .ToList(); + + FixMethod((VBAParser.ArgContext)parameters[parameterIndex].Context, + parameters[parameterIndex].QualifiedSelection); + } + + FixMethod((VBAParser.ArgContext)declarationParameters[parameterIndex].Context, + declarationParameters[parameterIndex].QualifiedSelection); + } + private void FixMethod(VBAParser.ArgContext context, QualifiedSelection qualifiedSelection) { var selectionLength = context.BYREF() == null ? 0 : 6; diff --git a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs index 07a6abecab..dba7e02f49 100644 --- a/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs +++ b/RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs @@ -503,6 +503,7 @@ Private Sub abc_Foo(ByRef arg1 As Integer) var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class3", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) .Build(); var vbe = builder.AddProject(project).Build(); @@ -538,6 +539,7 @@ Private Sub abc_Foo(ByVal arg1 As Integer) var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class3", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) .Build(); var vbe = builder.AddProject(project).Build(); @@ -574,6 +576,7 @@ Private Sub abc_Foo(ByRef arg1 As Integer) var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class3", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) .Build(); var vbe = builder.AddProject(project).Build(); @@ -610,6 +613,7 @@ Private Sub abc_Foo(ByRef arg1 As Integer, ByRef arg2 As Integer) var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none) .AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1) .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class3", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) .Build(); var vbe = builder.AddProject(project).Build(); @@ -851,6 +855,70 @@ Private Sub IClass1_DoSomething(ByVal a As Integer, ByRef b As Integer) Assert.AreEqual(expectedCode3, module3.Lines()); } + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_EVentMember_MultipleParams_OneCanBeByVal_QuickFixWorks() + { + //Input + const string inputCode1 = +@"Public Event Foo(ByRef a As Integer, ByRef b As Integer)"; + const string inputCode2 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByRef a As Integer, ByRef b As Integer) + a = 42 +End Sub"; + const string inputCode3 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByRef a As Integer, ByRef b As Integer) +End Sub"; + + //Expected + const string expectedCode1 = +@"Public Event Foo(ByRef a As Integer, ByVal b As Integer)"; + const string expectedCode2 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByRef a As Integer, ByVal b As Integer) + a = 42 +End Sub"; + const string expectedCode3 = +@"Private WithEvents abc As Class1 + +Private Sub abc_Foo(ByRef a As Integer, ByVal b 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_ClassModule, inputCode1) + .AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2) + .AddComponent("Class3", vbext_ComponentType.vbext_ct_ClassModule, inputCode3) + .Build(); + + var module1 = project.Object.VBComponents.Item("Class1").CodeModule; + var module2 = project.Object.VBComponents.Item("Class2").CodeModule; + var module3 = project.Object.VBComponents.Item("Class3").CodeModule; + var vbe = builder.AddProject(project).Build(); + + var mockHost = new Mock(); + mockHost.SetupAllProperties(); + var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock().Object)); + + parser.Parse(new CancellationTokenSource()); + if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); } + + var inspection = new ParameterCanBeByValInspection(parser.State); + var inspectionResults = inspection.GetInspectionResults(); + + inspectionResults.Single().QuickFixes.Single(s => s is PassParameterByValueQuickFix).Fix(); + + Assert.AreEqual(expectedCode1, module1.Lines()); + Assert.AreEqual(expectedCode2, module2.Lines()); + Assert.AreEqual(expectedCode3, module3.Lines()); + } + [TestMethod] [TestCategory("Inspections")] public void ParameterCanBeByVal_IgnoreQuickFixWorks() From 6039b16a02d0bda7c40cca8d9a394695a904fdfd Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 14:23:53 -0500 Subject: [PATCH 08/13] Refactor and reduce duplicated code. --- .../ParameterCanBeByValInspection.cs | 63 ++++--------------- .../ParameterCanBeByValInspectionResult.cs | 53 ++++------------ 2 files changed, 23 insertions(+), 93 deletions(-) diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index f2e457d201..9a8fe79f37 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -26,7 +26,7 @@ public override IEnumerable GetInspectionResults() var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList(); var interfaceScopes = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers).Select(s => s.Scope); - issues.AddRange(GetInterfaceResults(declarations, interfaceDeclarationMembers)); + issues.AddRange(GetResults(declarations, interfaceDeclarationMembers)); var eventMembers = declarations.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList(); var formEventHandlerScopes = State.FindFormEventHandlers().Select(handler => handler.Scope); @@ -35,7 +35,7 @@ public override IEnumerable GetInspectionResults() .Concat(formEventHandlerScopes) .Concat(eventHandlerScopes); - issues.AddRange(GetEventResults(declarations, eventMembers)); + issues.AddRange(GetResults(declarations, eventMembers)); var declareScopes = declarations.Where(item => item.DeclarationType == DeclarationType.LibraryFunction @@ -56,67 +56,28 @@ public override IEnumerable GetInspectionResults() return issues; } - private IEnumerable GetInterfaceResults(List declarations, List interfaceDeclarationMembers) + private IEnumerable GetResults(List declarations, List declarationMembers) { - foreach (var member in interfaceDeclarationMembers) + foreach (var declaration in declarationMembers) { var declarationParameters = - declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == member) + declarations.Where(d => d.DeclarationType == DeclarationType.Parameter && + d.ParentDeclaration == declaration) .OrderBy(o => o.Selection.StartLine) .ThenBy(t => t.Selection.StartColumn) .ToList(); var parametersAreByRef = declarationParameters.Select(s => true).ToList(); - var implementations = declarations.FindInterfaceImplementationMembers(member).ToList(); - foreach (var implementation in implementations) - { - var parameters = - declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == implementation) - .OrderBy(o => o.Selection.StartLine) - .ThenBy(t => t.Selection.StartColumn) - .ToList(); - - for (var i = 0; i < parameters.Count; i++) - { - parametersAreByRef[i] = parametersAreByRef[i] && !IsUsedAsByRefParam(declarations, parameters[i]) && - ((VBAParser.ArgContext)parameters[i].Context).BYVAL() == null && - !parameters[i].References.Any(reference => reference.IsAssignment); - } - } - - for (var i = 0; i < declarationParameters.Count; i++) - { - if (parametersAreByRef[i]) - { - yield return new ParameterCanBeByValInspectionResult(this, State, declarationParameters[i], - declarationParameters[i].Context, declarationParameters[i].QualifiedName); - } - } - } - } - - private IEnumerable GetEventResults(List declarations, List eventDeclarationMembers) - { - foreach (var member in eventDeclarationMembers) - { - var declarationParameters = - declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == member) - .OrderBy(o => o.Selection.StartLine) - .ThenBy(t => t.Selection.StartColumn) - .ToList(); - - var parametersAreByRef = declarationParameters.Select(s => true).ToList(); + var members = declarationMembers.Any(a => a.DeclarationType == DeclarationType.Event) + ? declarations.FindHandlersForEvent(declaration).Select(s => s.Item2).ToList() + : declarations.FindInterfaceImplementationMembers(declaration).ToList(); - var handlers = declarations.FindHandlersForEvent(member).Select(s => s.Item2).ToList(); - foreach (var handler in handlers) + foreach (var member in members) { var parameters = - declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == handler) + declarations.Where(d => d.DeclarationType == DeclarationType.Parameter && + d.ParentDeclaration == member) .OrderBy(o => o.Selection.StartLine) .ThenBy(t => t.Selection.StartColumn) .ToList(); diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs index 4a35840eca..e1b1e0cd71 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs @@ -45,13 +45,10 @@ public PassParameterByValueQuickFix(RubberduckParserState state, Declaration tar public override void Fix() { - if (_target.ParentDeclaration.DeclarationType == DeclarationType.Event) + if (_target.ParentDeclaration.DeclarationType == DeclarationType.Event || + _state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) { - FixEventMethods(); - } - else if (_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) - { - FixInterfaceMethods(); + FixMethods(); } else { @@ -59,7 +56,7 @@ public override void Fix() } } - private void FixInterfaceMethods() + private void FixMethods() { var declarationParameters = _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && @@ -74,45 +71,17 @@ private void FixInterfaceMethods() return; // should only happen if the parse results are stale; prevents a crash in that case } - var implementations = _state.AllUserDeclarations.FindInterfaceImplementationMembers(_target.ParentDeclaration); - foreach (var member in implementations) - { - var parameters = - _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == member) - .OrderBy(o => o.Selection.StartLine) - .ThenBy(t => t.Selection.StartColumn) - .ToList(); - - FixMethod((VBAParser.ArgContext)parameters[parameterIndex].Context, - parameters[parameterIndex].QualifiedSelection); - } - - FixMethod((VBAParser.ArgContext)declarationParameters[parameterIndex].Context, - declarationParameters[parameterIndex].QualifiedSelection); - } - - private void FixEventMethods() - { - var declarationParameters = - _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == _target.ParentDeclaration) - .OrderBy(o => o.Selection.StartLine) - .ThenBy(t => t.Selection.StartColumn) - .ToList(); - - var parameterIndex = declarationParameters.IndexOf(_target); - if (parameterIndex == -1) - { - return; // should only happen if the parse results are stale; prevents a crash in that case - } + var members = _target.ParentDeclaration.DeclarationType == DeclarationType.Event + ? _state.AllUserDeclarations.FindHandlersForEvent(_target.ParentDeclaration) + .Select(s => s.Item2) + .ToList() + : _state.AllUserDeclarations.FindInterfaceImplementationMembers(_target.ParentDeclaration).ToList(); - var handlers = _state.AllUserDeclarations.FindHandlersForEvent(_target.ParentDeclaration).Select(s => s.Item2).ToList(); - foreach (var handler in handlers) + foreach (var member in members) { var parameters = _state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter && - declaration.ParentDeclaration == handler) + declaration.ParentDeclaration == member) .OrderBy(o => o.Selection.StartLine) .ThenBy(t => t.Selection.StartColumn) .ToList(); From 86220610a74ea4858b40d231b60a0bb8fdc7bc44 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 15:21:38 -0500 Subject: [PATCH 09/13] Check CanExecute before running hotkey commands --- RetailCoder.VBE/Common/RubberduckHooks.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/RetailCoder.VBE/Common/RubberduckHooks.cs b/RetailCoder.VBE/Common/RubberduckHooks.cs index a35374a7dd..2f9f67dc07 100644 --- a/RetailCoder.VBE/Common/RubberduckHooks.cs +++ b/RetailCoder.VBE/Common/RubberduckHooks.cs @@ -5,15 +5,12 @@ using System.Linq; using System.Runtime.InteropServices; using System.Windows.Forms; -using System.Windows.Input; using Microsoft.Vbe.Interop; using Rubberduck.Common.Hotkeys; using Rubberduck.Common.WinAPI; using Rubberduck.Settings; using Rubberduck.UI.Command; -using Rubberduck.UI.Command.Refactorings; using NLog; -using Rubberduck.UI; namespace Rubberduck.Common { @@ -21,12 +18,8 @@ public class RubberduckHooks : IRubberduckHooks { private readonly VBE _vbe; private readonly IntPtr _mainWindowHandle; - private readonly IntPtr _oldWndPointer; private readonly User32.WndProc _oldWndProc; - private User32.WndProc _newWndProc; private RawInput _rawinput; - private IRawDevice _kb; - private IRawDevice _mouse; private readonly IGeneralConfigService _config; private readonly IEnumerable _commands; private readonly IList _hooks = new List(); @@ -37,9 +30,8 @@ public RubberduckHooks(VBE vbe, IGeneralConfigService config, IEnumerable hotkey.IsEnabled)) { @@ -175,7 +165,7 @@ public void Detach() private void hook_MessageReceived(object sender, HookEventArgs e) { var hotkey = sender as IHotkey; - if (hotkey != null) + if (hotkey != null && hotkey.Command.CanExecute(null)) { hotkey.Command.Execute(null); return; From 133968c28ddf9c499554321c8185b7705968e71f Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 15:41:23 -0500 Subject: [PATCH 10/13] Revert "Check CanExecute before running hotkey commands" This reverts commit 86220610a74ea4858b40d231b60a0bb8fdc7bc44. --- RetailCoder.VBE/Common/RubberduckHooks.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/RetailCoder.VBE/Common/RubberduckHooks.cs b/RetailCoder.VBE/Common/RubberduckHooks.cs index 2f9f67dc07..a35374a7dd 100644 --- a/RetailCoder.VBE/Common/RubberduckHooks.cs +++ b/RetailCoder.VBE/Common/RubberduckHooks.cs @@ -5,12 +5,15 @@ using System.Linq; using System.Runtime.InteropServices; using System.Windows.Forms; +using System.Windows.Input; using Microsoft.Vbe.Interop; using Rubberduck.Common.Hotkeys; using Rubberduck.Common.WinAPI; using Rubberduck.Settings; using Rubberduck.UI.Command; +using Rubberduck.UI.Command.Refactorings; using NLog; +using Rubberduck.UI; namespace Rubberduck.Common { @@ -18,8 +21,12 @@ public class RubberduckHooks : IRubberduckHooks { private readonly VBE _vbe; private readonly IntPtr _mainWindowHandle; + private readonly IntPtr _oldWndPointer; private readonly User32.WndProc _oldWndProc; + private User32.WndProc _newWndProc; private RawInput _rawinput; + private IRawDevice _kb; + private IRawDevice _mouse; private readonly IGeneralConfigService _config; private readonly IEnumerable _commands; private readonly IList _hooks = new List(); @@ -30,8 +37,9 @@ public RubberduckHooks(VBE vbe, IGeneralConfigService config, IEnumerable hotkey.IsEnabled)) { @@ -165,7 +175,7 @@ public void Detach() private void hook_MessageReceived(object sender, HookEventArgs e) { var hotkey = sender as IHotkey; - if (hotkey != null && hotkey.Command.CanExecute(null)) + if (hotkey != null) { hotkey.Command.Execute(null); return; From 0b128f99f0d31acf3d5069e08b09ac61f5f93b0d Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 15:46:41 -0500 Subject: [PATCH 11/13] Redo hotkey CanExecute before Execute --- RetailCoder.VBE/Common/RubberduckHooks.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/RetailCoder.VBE/Common/RubberduckHooks.cs b/RetailCoder.VBE/Common/RubberduckHooks.cs index a35374a7dd..69eeb2457f 100644 --- a/RetailCoder.VBE/Common/RubberduckHooks.cs +++ b/RetailCoder.VBE/Common/RubberduckHooks.cs @@ -5,21 +5,17 @@ using System.Linq; using System.Runtime.InteropServices; using System.Windows.Forms; -using System.Windows.Input; using Microsoft.Vbe.Interop; using Rubberduck.Common.Hotkeys; using Rubberduck.Common.WinAPI; using Rubberduck.Settings; using Rubberduck.UI.Command; -using Rubberduck.UI.Command.Refactorings; using NLog; -using Rubberduck.UI; namespace Rubberduck.Common { public class RubberduckHooks : IRubberduckHooks { - private readonly VBE _vbe; private readonly IntPtr _mainWindowHandle; private readonly IntPtr _oldWndPointer; private readonly User32.WndProc _oldWndProc; @@ -34,7 +30,6 @@ public class RubberduckHooks : IRubberduckHooks public RubberduckHooks(VBE vbe, IGeneralConfigService config, IEnumerable commands) { - _vbe = vbe; _mainWindowHandle = (IntPtr)vbe.MainWindow.HWnd; _oldWndProc = WindowProc; _newWndProc = WindowProc; @@ -175,7 +170,7 @@ public void Detach() private void hook_MessageReceived(object sender, HookEventArgs e) { var hotkey = sender as IHotkey; - if (hotkey != null) + if (hotkey != null && hotkey.Command.CanExecute(null)) { hotkey.Command.Execute(null); return; From 8c84ed4a24a707fdb1cf7ec97380ef338d7830f0 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 16:43:57 -0500 Subject: [PATCH 12/13] Potential fix for #2054 --- .../Symbols/ReferencedDeclarationsCollector.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs b/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs index ce1f657f81..c47d03103f 100644 --- a/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs +++ b/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs @@ -220,7 +220,7 @@ public List GetDeclarationsForReference(Reference reference) break; case DeclarationType.ClassModule: var module = new ClassModuleDeclaration(typeQualifiedMemberName, projectDeclaration, typeName, true, new List(), attributes); - var implements = GetImplementedInterfaceNames(typeAttributes, info); + var implements = GetImplementedInterfaceNames(typeAttributes, info, module); foreach (var supertypeName in implements) { module.AddSupertype(supertypeName); @@ -575,7 +575,7 @@ private ParameterDeclaration CreateParameterDeclaration(IReadOnlyList me return new ParameterDeclaration(new QualifiedMemberName(typeQualifiedModuleName, paramName), memberDeclaration, paramInfo.Name, null, null, isOptional, paramInfo.IsByRef, paramInfo.IsArray); } - private IEnumerable GetImplementedInterfaceNames(TYPEATTR typeAttr, ITypeInfo info) + private IEnumerable GetImplementedInterfaceNames(TYPEATTR typeAttr, ITypeInfo info, Declaration module) { var output = new List(); for (var implIndex = 0; implIndex < typeAttr.cImplTypes; implIndex++) @@ -616,7 +616,7 @@ private IEnumerable GetImplementedInterfaceNames(TYPEATTR typeAttr, ITyp else { _comInformation.Add(typeAttributes.guid, - new ComInformation(typeAttributes, flags, implTypeInfo, implTypeName, new QualifiedModuleName(), null, 0)); + new ComInformation(typeAttributes, flags, implTypeInfo, implTypeName, module.QualifiedName.QualifiedModuleName, module, 0)); } } From 5c4f2f9746f4d127dd8057f1715e8298b5fd89c7 Mon Sep 17 00:00:00 2001 From: Abraham Hosch Date: Thu, 14 Jul 2016 16:54:36 -0500 Subject: [PATCH 13/13] Make the COM library say "Rubberduck Source Control 2.0" --- Rubberduck.SourceControl/Properties/AssemblyInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rubberduck.SourceControl/Properties/AssemblyInfo.cs b/Rubberduck.SourceControl/Properties/AssemblyInfo.cs index ef92122c40..59a868135e 100644 --- a/Rubberduck.SourceControl/Properties/AssemblyInfo.cs +++ b/Rubberduck.SourceControl/Properties/AssemblyInfo.cs @@ -5,7 +5,7 @@ // set of attributes. Change these attribute values to modify the information // associated with an assembly. [assembly: AssemblyTitle("Rubberduck.SourceControl")] -[assembly: AssemblyDescription("Rubberduck Source Control 1.0")] +[assembly: AssemblyDescription("Rubberduck Source Control 2.0")] [assembly: AssemblyConfiguration("")] [assembly: AssemblyCompany("Rubberduck")] [assembly: AssemblyProduct("Rubberduck.SourceControl")]