diff --git a/RetailCoder.VBE/Common/DeclarationExtensions.cs b/RetailCoder.VBE/Common/DeclarationExtensions.cs index 0e2ba12bfc..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(); @@ -451,6 +467,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/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; diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs index be99fcf2f5..9a8fe79f37 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs @@ -21,39 +21,86 @@ 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 interfaceScopes = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers).Select(s => s.Scope); - var formEventHandlerScopes = State.FindFormEventHandlers() - .Select(handler => handler.Scope); + issues.AddRange(GetResults(declarations, interfaceDeclarationMembers)); - var eventScopes = declarations.Where(item => - !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event) - .Select(e => e.Scope).Concat(State.AllDeclarations.FindBuiltInEventHandlers().Select(e => e.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); + + issues.AddRange(GetResults(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); - - var issues = declarations.Where(declaration => + + 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 - && !interfaceMembers.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, State, issue, issue.Context, issue.QualifiedName))); return issues; } + private IEnumerable GetResults(List declarations, List declarationMembers) + { + foreach (var declaration in declarationMembers) + { + var declarationParameters = + 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 members = declarationMembers.Any(a => a.DeclarationType == DeclarationType.Event) + ? declarations.FindHandlersForEvent(declaration).Select(s => s.Item2).ToList() + : declarations.FindInterfaceImplementationMembers(declaration).ToList(); + + foreach (var member in members) + { + var parameters = + declarations.Where(d => d.DeclarationType == DeclarationType.Parameter && + d.ParentDeclaration == member) + .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. @@ -74,28 +121,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) + if (declaration.References.Any(reference => reference.IsAssignment)) { - 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; - } + return true; } } } diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs index b7cb0e2c53..e1b1e0cd71 100644 --- a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs @@ -1,7 +1,10 @@ using System.Collections.Generic; +using System.Linq; using Antlr4.Runtime; +using Rubberduck.Common; using Rubberduck.Parsing.Grammar; using Rubberduck.Parsing.Symbols; +using Rubberduck.Parsing.VBA; using Rubberduck.VBEditor; namespace Rubberduck.Inspections @@ -10,12 +13,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 +33,76 @@ 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 (_target.ParentDeclaration.DeclarationType == DeclarationType.Event || + _state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration)) + { + FixMethods(); + } + else + { + FixMethod((VBAParser.ArgContext)Context, Selection); + } + } + + private void FixMethods() + { + 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(); + + foreach (var member in members) + { + 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/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/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)); } } 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")] 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 450a41216f..dba7e02f49 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() @@ -244,6 +333,303 @@ Sub Foo(arg1 As String) Assert.IsFalse(inspectionResults.Any()); } + [TestMethod] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_InterfaceMember_SingleParam() + { + //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) + .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_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 + 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(); + + 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_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] + [TestCategory("Inspections")] + public void ParameterCanBeByVal_EventMember_SingleParam() + { + //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) +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, 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) + .AddComponent("Class3", 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 + 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, 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_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) + .AddComponent("Class3", 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() @@ -403,6 +789,136 @@ public void ParameterCanBeByVal_QuickFixWorks_PassedByRef_MultilineParameter() Assert.AreEqual(expectedCode, module.Lines()); } + [TestMethod] + [TestCategory("Inspections")] + 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_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()