From ef55b69e4a6b7795b90f83e41cdc87b0d64f5620 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Wed, 21 Apr 2021 15:14:15 -0400 Subject: [PATCH] include whitespace in argument selection --- .../NonReturningFunctionInspection.cs | 3 +- .../Concrete/ParameterCanBeByValInspection.cs | 3 +- ...ocedureCanBeWrittenAsFunctionInspection.cs | 3 +- .../UnassignedVariableUsageInspection.cs | 3 +- .../Concrete/VariableNotAssignedInspection.cs | 3 +- .../DeclarationCaching/DeclarationFinder.cs | 54 +++++++++++-------- .../VBA/SelectedDeclarationProvider.cs | 40 +++++++++++--- Rubberduck.VBEEditor/Selection.cs | 5 ++ 8 files changed, 81 insertions(+), 33 deletions(-) diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs index 919794f4c8..22e82c57c4 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs @@ -71,7 +71,8 @@ private bool IsAssignedByRefArgument(Declaration enclosingProcedure, IdentifierR return false; } - var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argExpression, enclosingProcedure); + var argument = argExpression.GetAncestor(); + var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argument, enclosingProcedure); // note: not recursive, by design. return parameter != null diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/ParameterCanBeByValInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/ParameterCanBeByValInspection.cs index 2af67e9394..cb2d0b4929 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Concrete/ParameterCanBeByValInspection.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Concrete/ParameterCanBeByValInspection.cs @@ -142,7 +142,8 @@ private static bool IsPotentiallyAssignedByRefArgument(QualifiedModuleName modul return false; } - var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argExpression, module); + var argument = argExpression.GetAncestor(); + var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argument, module); if (parameter == null) { diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/ProcedureCanBeWrittenAsFunctionInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/ProcedureCanBeWrittenAsFunctionInspection.cs index dbd001bc1c..fcb4f63c7b 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Concrete/ProcedureCanBeWrittenAsFunctionInspection.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Concrete/ProcedureCanBeWrittenAsFunctionInspection.cs @@ -88,7 +88,8 @@ private static bool IsUsageAsAssignedToByRefArgument(IdentifierReference referen return false; } - var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argExpression, reference.QualifiedModuleName); + var argument = argExpression.GetAncestor(); + var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argument, reference.QualifiedModuleName); if (parameter == null) { diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnassignedVariableUsageInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnassignedVariableUsageInspection.cs index a8e03a9ae3..b3280565a6 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnassignedVariableUsageInspection.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnassignedVariableUsageInspection.cs @@ -155,7 +155,8 @@ private static bool IsAssignedByRefArgument(Declaration enclosingProcedure, Iden return false; } - var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argExpression, enclosingProcedure); + var argument = argExpression.GetAncestor(); + var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argument, enclosingProcedure); // note: not recursive, by design. return parameter != null diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/VariableNotAssignedInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/VariableNotAssignedInspection.cs index fe5abbb010..d964a9e55e 100644 --- a/Rubberduck.CodeAnalysis/Inspections/Concrete/VariableNotAssignedInspection.cs +++ b/Rubberduck.CodeAnalysis/Inspections/Concrete/VariableNotAssignedInspection.cs @@ -69,7 +69,8 @@ private static bool IsAssignedByRefArgument(Declaration enclosingProcedure, Iden return false; } - var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argExpression, enclosingProcedure); + var argument = argExpression.GetAncestor(); + var parameter = finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argument, enclosingProcedure); // note: not recursive, by design. return parameter != null diff --git a/Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs b/Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs index 437d4e5daa..fb50fdf8cb 100644 --- a/Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs +++ b/Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs @@ -602,24 +602,25 @@ public IEnumerable MatchName(string name) : Enumerable.Empty(); } - public ParameterDeclaration FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(VBAParser.ArgumentExpressionContext argumentExpression, Declaration enclosingProcedure) + public ParameterDeclaration FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(VBAParser.ArgumentContext argument, Declaration enclosingProcedure) { - return FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argumentExpression, enclosingProcedure.QualifiedModuleName); + return FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(argument, enclosingProcedure.QualifiedModuleName); } - public ParameterDeclaration FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(VBAParser.ArgumentExpressionContext argumentExpression, QualifiedModuleName module) + public ParameterDeclaration FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(VBAParser.ArgumentContext argument, QualifiedModuleName module) { //todo: Rename after making it work for more general cases. - - if (argumentExpression == null - || argumentExpression.GetDescendent() != null - || argumentExpression.BYVAL() != null) + var missingArgument = argument.missingArgument(); + var argumentExpression = argument.GetDescendent(); + if ((missingArgument == null && argumentExpression == null) + || argumentExpression?.GetDescendent() != null + || argumentExpression?.BYVAL() != null) { // not a simple argument, or argument is parenthesized and thus passed ByVal return null; } - var callingNonDefaultMember = CallingNonDefaultMember(argumentExpression, module); + var callingNonDefaultMember = CallingNonDefaultMember((ParserRuleContext)argumentExpression ?? missingArgument, module); if (callingNonDefaultMember == null) { // Either we could not resolve the call or there is a default member call involved. @@ -627,6 +628,7 @@ public ParameterDeclaration FindParameterOfNonDefaultMemberFromSimpleArgumentNot } var parameters = Parameters(callingNonDefaultMember); + var hasNamedArgs = argumentExpression?.GetAncestor()?.TryGetChildContext(out _) ?? false; ParameterDeclaration parameter; var namedArg = argumentExpression.GetAncestor(); @@ -639,35 +641,44 @@ public ParameterDeclaration FindParameterOfNonDefaultMemberFromSimpleArgumentNot else { // argument is positional: work out its index - var argumentList = argumentExpression.GetAncestor(); - var arguments = argumentList.children.Where(t => (t is VBAParser.ArgumentContext)).ToArray(); - - var parameterIndex = arguments - .Select((arg, index) => (arg: arg as ParserRuleContext, index)) - .SingleOrDefault(item => item.arg.ContainsTokenIndex(argumentExpression.Start.TokenIndex)).index; - + var argumentList = ((ParserRuleContext)argumentExpression ?? missingArgument).GetAncestor(); + var arguments = argumentList.children.Where(t => t is VBAParser.ArgumentContext).ToArray(); + var selection = argumentExpression?.GetSelection() ?? missingArgument.GetSelection(); + + var indexedArgs = arguments.Select((arg, index) => (arg: arg as ParserRuleContext, index)) + .Select(e => (arg: e.arg, e.index, selection:e.arg.GetSelection())) + .ToList(); + var indexedArg = indexedArgs.SingleOrDefault(item => item.selection.Contains(selection)); + if (indexedArg.arg == null) + { + return null; + } parameter = parameters .Select((param, index) => (param, index)) - .SingleOrDefault(item => item.index == parameterIndex).param; + .Single(item => item.index == indexedArg.index).param; } return parameter; } - public ModuleBodyElementDeclaration FindInvokedMemberFromArgumentExpressionContext(VBAParser.ArgumentExpressionContext argumentExpression, QualifiedModuleName module) + public ModuleBodyElementDeclaration FindInvokedMemberFromArgumentContext(VBAParser.ArgumentContext argument, QualifiedModuleName module) { - return CallingNonDefaultMember(argumentExpression, module); + var expression = (ParserRuleContext)argument.GetDescendent() + ?? argument.GetDescendent(); + return expression != null + ? CallingNonDefaultMember(expression, module) + : null; } - private ModuleBodyElementDeclaration CallingNonDefaultMember(VBAParser.ArgumentExpressionContext argumentExpression, QualifiedModuleName module) + private ModuleBodyElementDeclaration CallingNonDefaultMember(ParserRuleContext argumentExpressionOrMissingArgument, QualifiedModuleName module) { //todo: Make this work for default member calls. - var argumentList = argumentExpression.GetAncestor(); + var argumentList = argumentExpressionOrMissingArgument.GetAncestor(); var cannotHaveDefaultMemberCall = false; ParserRuleContext callingExpression; - switch (argumentList.Parent) + switch (argumentList?.Parent) { case VBAParser.CallStmtContext callStmt: cannotHaveDefaultMemberCall = true; @@ -680,7 +691,6 @@ private ModuleBodyElementDeclaration CallingNonDefaultMember(VBAParser.ArgumentE callingExpression = indexExpr.lExpression(); break; default: - //This should never happen. return null; } diff --git a/Rubberduck.Parsing/VBA/SelectedDeclarationProvider.cs b/Rubberduck.Parsing/VBA/SelectedDeclarationProvider.cs index c79a55b3fd..45d8df396c 100644 --- a/Rubberduck.Parsing/VBA/SelectedDeclarationProvider.cs +++ b/Rubberduck.Parsing/VBA/SelectedDeclarationProvider.cs @@ -107,19 +107,47 @@ private static Declaration SelectedDeclarationViaArgument(QualifiedSelection qua .Where(m => (m.DeclarationType.HasFlag(DeclarationType.Procedure) // includes PropertyLet and PropertySet and LibraryProcedure || m.DeclarationType.HasFlag(DeclarationType.Function)) // includes PropertyGet and LibraryFunction && !m.DeclarationType.HasFlag(DeclarationType.LibraryFunction) - && !m.DeclarationType.HasFlag(DeclarationType.LibraryProcedure)); + && !m.DeclarationType.HasFlag(DeclarationType.LibraryProcedure)); var enclosingProcedure = members.SingleOrDefault(m => m.Context.GetSelection().Contains(qualifiedSelection.Selection)); + if (enclosingProcedure == null) + { + return null; + } - var context = enclosingProcedure?.Context.GetDescendents() - .FirstOrDefault(m => m.GetSelection().ContainsFirstCharacter(qualifiedSelection.Selection)); + var allArguments = enclosingProcedure.Context.GetDescendents(); + var context = allArguments + .Where(arg => arg.missingArgument() == null) + .FirstOrDefault(m => + { + var isOnWhitespace = false; + if (m.TryGetPrecedingContext(out var whitespace)) + { + isOnWhitespace = whitespace.GetSelection().ContainsFirstCharacter(qualifiedSelection.Selection); + } + return isOnWhitespace || m.GetSelection().ContainsFirstCharacter(qualifiedSelection.Selection); + }); + + var skippedArg = allArguments + .Where(arg => arg.missingArgument() != null) + .FirstOrDefault(m => + { + var isOnWhitespace = false; + if (m.TryGetPrecedingContext(out var whitespace)) + { + isOnWhitespace = whitespace.GetSelection().ContainsFirstCharacter(qualifiedSelection.Selection); + } + return isOnWhitespace || m.GetSelection().ContainsFirstCharacter(qualifiedSelection.Selection); + }); + + context = context ?? skippedArg; if (context != null) { - return finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(context, enclosingProcedure); + return (Declaration)finder.FindParameterOfNonDefaultMemberFromSimpleArgumentNotPassedByValExplicitly(context, enclosingProcedure) + ?? finder.FindInvokedMemberFromArgumentContext(context, qualifiedSelection.QualifiedName); // fallback to the invoked procedure declaration } - // fallback to the invoked procedure declaration - return finder.FindInvokedMemberFromArgumentExpressionContext(context, qualifiedSelection.QualifiedName); + return null; } private static Declaration SelectedDeclarationViaReference(QualifiedSelection qualifiedSelection, DeclarationFinder finder) diff --git a/Rubberduck.VBEEditor/Selection.cs b/Rubberduck.VBEEditor/Selection.cs index 2823a8bf3e..6bbd33a2b9 100644 --- a/Rubberduck.VBEEditor/Selection.cs +++ b/Rubberduck.VBEEditor/Selection.cs @@ -46,6 +46,11 @@ public bool ContainsFirstCharacter(Selection selection) return Contains(new Selection(selection.StartLine, selection.StartColumn, selection.StartLine, selection.StartColumn)); } + public Selection ExtendLeft(int positions = 1) + { + return new Selection(StartLine, Math.Max(StartColumn - positions, 1), EndLine, EndColumn); + } + public bool Contains(Selection selection) { // single line comparison