diff --git a/src/PowerShellEditorServices/Services/Symbols/Vistors/FindReferencesVisitor.cs b/src/PowerShellEditorServices/Services/Symbols/Vistors/FindReferencesVisitor.cs index 0dcbe7e0d..44b64c8f5 100644 --- a/src/PowerShellEditorServices/Services/Symbols/Vistors/FindReferencesVisitor.cs +++ b/src/PowerShellEditorServices/Services/Symbols/Vistors/FindReferencesVisitor.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Management.Automation.Language; +using Microsoft.PowerShell.EditorServices.Utility; namespace Microsoft.PowerShell.EditorServices.Services.Symbols { @@ -116,7 +117,7 @@ public override AstVisitAction VisitCommand(CommandAst commandAst) /// A visit action that continues the search for references public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst) { - (int startColumnNumber, int startLineNumber) = GetStartColumnAndLineNumbersFromAst(functionDefinitionAst); + (int startColumnNumber, int startLineNumber) = VisitorUtils.GetNameStartColumnAndLineNumbersFromAst(functionDefinitionAst); IScriptExtent nameExtent = new ScriptExtent() { @@ -167,39 +168,5 @@ public override AstVisitAction VisitVariableExpression(VariableExpressionAst var } return AstVisitAction.Continue; } - - // Computes where the start of the actual function name is. - private static (int, int) GetStartColumnAndLineNumbersFromAst(FunctionDefinitionAst ast) - { - int startColumnNumber = ast.Extent.StartColumnNumber; - int startLineNumber = ast.Extent.StartLineNumber; - int astOffset = ast.IsFilter ? "filter".Length : ast.IsWorkflow ? "workflow".Length : "function".Length; - string astText = ast.Extent.Text; - // The line offset represents the offset on the line that we're on where as - // astOffset is the offset on the entire text of the AST. - int lineOffset = astOffset; - for (; astOffset < astText.Length; astOffset++, lineOffset++) - { - if (astText[astOffset] == '\n') - { - // reset numbers since we are operating on a different line and increment the line number. - startColumnNumber = 0; - startLineNumber++; - lineOffset = 0; - } - else if (astText[astOffset] == '\r') - { - // Do nothing with carriage returns... we only look for line feeds since those - // are used on every platform. - } - else if (!char.IsWhiteSpace(astText[astOffset])) - { - // This is the start of the function name so we've found our start column and line number. - break; - } - } - - return (startColumnNumber + lineOffset, startLineNumber); - } } } diff --git a/src/PowerShellEditorServices/Services/Symbols/Vistors/FindSymbolVisitor.cs b/src/PowerShellEditorServices/Services/Symbols/Vistors/FindSymbolVisitor.cs index 0a1e1ec2a..bf2520a3c 100644 --- a/src/PowerShellEditorServices/Services/Symbols/Vistors/FindSymbolVisitor.cs +++ b/src/PowerShellEditorServices/Services/Symbols/Vistors/FindSymbolVisitor.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System.Management.Automation.Language; +using Microsoft.PowerShell.EditorServices.Utility; namespace Microsoft.PowerShell.EditorServices.Services.Symbols { @@ -57,22 +58,28 @@ public override AstVisitAction VisitCommand(CommandAst commandAst) /// or a decision to continue if it wasn't found public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst) { - int startColumnNumber = 1; + int startLineNumber = functionDefinitionAst.Extent.StartLineNumber; + int startColumnNumber = functionDefinitionAst.Extent.StartColumnNumber; + int endLineNumber = functionDefinitionAst.Extent.EndLineNumber; + int endColumnNumber = functionDefinitionAst.Extent.EndColumnNumber; if (!includeFunctionDefinitions) { - startColumnNumber = - functionDefinitionAst.Extent.Text.IndexOf( - functionDefinitionAst.Name) + 1; + // We only want the function name + (int startColumn, int startLine) = VisitorUtils.GetNameStartColumnAndLineNumbersFromAst(functionDefinitionAst); + startLineNumber = startLine; + startColumnNumber = startColumn; + endLineNumber = startLine; + endColumnNumber = startColumn + functionDefinitionAst.Name.Length; } IScriptExtent nameExtent = new ScriptExtent() { Text = functionDefinitionAst.Name, - StartLineNumber = functionDefinitionAst.Extent.StartLineNumber, - EndLineNumber = functionDefinitionAst.Extent.EndLineNumber, + StartLineNumber = startLineNumber, + EndLineNumber = endLineNumber, StartColumnNumber = startColumnNumber, - EndColumnNumber = startColumnNumber + functionDefinitionAst.Name.Length, + EndColumnNumber = endColumnNumber, File = functionDefinitionAst.Extent.File }; diff --git a/src/PowerShellEditorServices/Utility/VisitorUtils.cs b/src/PowerShellEditorServices/Utility/VisitorUtils.cs new file mode 100644 index 000000000..861d04acc --- /dev/null +++ b/src/PowerShellEditorServices/Utility/VisitorUtils.cs @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Management.Automation.Language; + +namespace Microsoft.PowerShell.EditorServices.Utility +{ + /// + /// General common utilities for AST visitors to prevent reimplementation. + /// + internal static class VisitorUtils + { + /// + /// Calculates the start line and column of the actual function name in a function definition AST. + /// + /// A FunctionDefinitionAst object in the script's AST + /// A tuple with start column and line for the function name + internal static (int startColumn, int startLine) GetNameStartColumnAndLineNumbersFromAst(FunctionDefinitionAst ast) + { + int startColumnNumber = ast.Extent.StartColumnNumber; + int startLineNumber = ast.Extent.StartLineNumber; + int astOffset = ast.IsFilter ? "filter".Length : ast.IsWorkflow ? "workflow".Length : "function".Length; + string astText = ast.Extent.Text; + // The line offset represents the offset on the line that we're on where as + // astOffset is the offset on the entire text of the AST. + int lineOffset = astOffset; + for (; astOffset < astText.Length; astOffset++, lineOffset++) + { + if (astText[astOffset] == '\n') + { + // reset numbers since we are operating on a different line and increment the line number. + startColumnNumber = 0; + startLineNumber++; + lineOffset = 0; + } + else if (astText[astOffset] == '\r') + { + // Do nothing with carriage returns... we only look for line feeds since those + // are used on every platform. + } + else if (!char.IsWhiteSpace(astText[astOffset])) + { + // This is the start of the function name so we've found our start column and line number. + break; + } + } + + return (startColumnNumber + lineOffset, startLineNumber); + } + } +} diff --git a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs index b713d900f..6789c4323 100644 --- a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs @@ -35,14 +35,21 @@ function FunctionWithExtraSpace FunctionNameOnDifferentLine + + function IndentedFunction { } IndentedFunction "; private static readonly ScriptBlockAst s_ast = (ScriptBlockAst)ScriptBlock.Create(s_scriptString).Ast; [Theory] + [InlineData(1, 15, "BasicFunction")] [InlineData(2, 3, "BasicFunction")] + [InlineData(4, 31, "FunctionWithExtraSpace")] [InlineData(7, 18, "FunctionWithExtraSpace")] + [InlineData(12, 22, "FunctionNameOnDifferentLine")] [InlineData(22, 13, "FunctionNameOnDifferentLine")] - public void CanFindSymbolAtPostion(int lineNumber, int columnNumber, string expectedName) + [InlineData(24, 30, "IndentedFunction")] + [InlineData(24, 52, "IndentedFunction")] + public void CanFindSymbolAtPosition(int lineNumber, int columnNumber, string expectedName) { SymbolReference reference = AstOperations.FindSymbolAtPosition(s_ast, lineNumber, columnNumber); Assert.NotNull(reference); @@ -50,8 +57,8 @@ public void CanFindSymbolAtPostion(int lineNumber, int columnNumber, string expe } [Theory] - [MemberData(nameof(FindReferencesOfSymbolAtPostionData))] - public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, Position[] positions) + [MemberData(nameof(FindReferencesOfSymbolAtPositionData))] + public void CanFindReferencesOfSymbolAtPosition(int lineNumber, int columnNumber, Range[] symbolRange) { SymbolReference symbol = AstOperations.FindSymbolAtPosition(s_ast, lineNumber, columnNumber); @@ -60,18 +67,25 @@ public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, int positionsIndex = 0; foreach (SymbolReference reference in references) { - Assert.Equal(positions[positionsIndex].Line, reference.ScriptRegion.StartLineNumber); - Assert.Equal(positions[positionsIndex].Character, reference.ScriptRegion.StartColumnNumber); + Assert.Equal(symbolRange[positionsIndex].Start.Line, reference.ScriptRegion.StartLineNumber); + Assert.Equal(symbolRange[positionsIndex].Start.Character, reference.ScriptRegion.StartColumnNumber); + Assert.Equal(symbolRange[positionsIndex].End.Line, reference.ScriptRegion.EndLineNumber); + Assert.Equal(symbolRange[positionsIndex].End.Character, reference.ScriptRegion.EndColumnNumber); positionsIndex++; } } - public static object[][] FindReferencesOfSymbolAtPostionData { get; } = new object[][] + public static object[][] FindReferencesOfSymbolAtPositionData { get; } = new object[][] { - new object[] { 2, 3, new[] { new Position(1, 10), new Position(2, 1) } }, - new object[] { 7, 18, new[] { new Position(4, 19), new Position(7, 3) } }, - new object[] { 22, 13, new[] { new Position(12, 8), new Position(22, 5) } }, + new object[] { 1, 15, new[] { new Range(1, 10, 1, 23), new Range(2, 1, 2, 14) } }, + new object[] { 2, 3, new[] { new Range(1, 10, 1, 23), new Range(2, 1, 2, 14) } }, + new object[] { 4, 31, new[] { new Range(4, 19, 4, 41), new Range(7, 3, 7, 25) } }, + new object[] { 7, 18, new[] { new Range(4, 19, 4, 41), new Range(7, 3, 7, 25) } }, + new object[] { 22, 13, new[] { new Range(12, 8, 12, 35), new Range(22, 5, 22, 32) } }, + new object[] { 12, 22, new[] { new Range(12, 8, 12, 35), new Range(22, 5, 22, 32) } }, + new object[] { 24, 30, new[] { new Range(24, 22, 24, 38), new Range(24, 44, 24, 60) } }, + new object[] { 24, 52, new[] { new Range(24, 22, 24, 38), new Range(24, 44, 24, 60) } }, }; } }