From 8f21fdf0c142f1b69c21ac4514769f6e68bc414f Mon Sep 17 00:00:00 2001 From: Frode Flaten <3436158+fflaten@users.noreply.github.com> Date: Sun, 14 Aug 2022 17:37:07 +0000 Subject: [PATCH 1/5] move helper to VistiorUtils class --- .../Symbols/Vistors/FindReferencesVisitor.cs | 37 +------------- .../Utility/VisitorUtils.cs | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+), 35 deletions(-) create mode 100644 src/PowerShellEditorServices/Utility/VisitorUtils.cs 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/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); + } + } +} From 2761a358220a6efbb96807358fd011392f687432 Mon Sep 17 00:00:00 2001 From: Frode Flaten <3436158+fflaten@users.noreply.github.com> Date: Sun, 14 Aug 2022 17:41:07 +0000 Subject: [PATCH 2/5] find function name if indented or after newline --- .../Symbols/Vistors/FindSymbolVisitor.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) 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 }; From cca6cdb8c420b4181158860f0962247df81705ca Mon Sep 17 00:00:00 2001 From: Frode Flaten <3436158+fflaten@users.noreply.github.com> Date: Sun, 14 Aug 2022 17:41:24 +0000 Subject: [PATCH 3/5] extend tests to cover function definition name --- .../Services/Symbols/AstOperationsTests.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs index b713d900f..ef12ac96b 100644 --- a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs @@ -35,13 +35,20 @@ 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")] + [InlineData(24, 30, "IndentedFunction")] + [InlineData(24, 52, "IndentedFunction")] public void CanFindSymbolAtPostion(int lineNumber, int columnNumber, string expectedName) { SymbolReference reference = AstOperations.FindSymbolAtPosition(s_ast, lineNumber, columnNumber); @@ -69,9 +76,14 @@ public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, public static object[][] FindReferencesOfSymbolAtPostionData { get; } = new object[][] { + new object[] { 1, 15, new[] { new Position(1, 10), new Position(2, 1) } }, new object[] { 2, 3, new[] { new Position(1, 10), new Position(2, 1) } }, + new object[] { 4, 31, new[] { new Position(4, 19), new Position(7, 3) } }, 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[] { 12, 22, new[] { new Position(12, 8), new Position(22, 5) } }, + new object[] { 24, 30, new[] { new Position(24, 22), new Position(24, 44) } }, + new object[] { 24, 52, new[] { new Position(24, 22), new Position(24, 44) } }, }; } } From 6ea278f6f17fa7e4dc504ff417657c971f1706bf Mon Sep 17 00:00:00 2001 From: Frode Flaten <3436158+fflaten@users.noreply.github.com> Date: Sun, 14 Aug 2022 17:54:46 +0000 Subject: [PATCH 4/5] extend test to include end position --- .../Services/Symbols/AstOperationsTests.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs index ef12ac96b..1aebd7f44 100644 --- a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs @@ -58,7 +58,7 @@ public void CanFindSymbolAtPostion(int lineNumber, int columnNumber, string expe [Theory] [MemberData(nameof(FindReferencesOfSymbolAtPostionData))] - public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, Position[] positions) + public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, Range[] symbolRange) { SymbolReference symbol = AstOperations.FindSymbolAtPosition(s_ast, lineNumber, columnNumber); @@ -67,8 +67,10 @@ 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++; } @@ -76,14 +78,14 @@ public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, public static object[][] FindReferencesOfSymbolAtPostionData { get; } = new object[][] { - new object[] { 1, 15, new[] { new Position(1, 10), new Position(2, 1) } }, - new object[] { 2, 3, new[] { new Position(1, 10), new Position(2, 1) } }, - new object[] { 4, 31, new[] { new Position(4, 19), new Position(7, 3) } }, - 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[] { 12, 22, new[] { new Position(12, 8), new Position(22, 5) } }, - new object[] { 24, 30, new[] { new Position(24, 22), new Position(24, 44) } }, - new object[] { 24, 52, new[] { new Position(24, 22), new Position(24, 44) } }, + 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) } }, }; } } From 150c218ecf923ceb9228d60e3027316a77992ccf Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andschwa@users.noreply.github.com> Date: Mon, 15 Aug 2022 13:55:10 -0700 Subject: [PATCH 5/5] Fix `postion` to `position` typo --- .../Services/Symbols/AstOperationsTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs index 1aebd7f44..6789c4323 100644 --- a/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Services/Symbols/AstOperationsTests.cs @@ -49,7 +49,7 @@ function IndentedFunction { } IndentedFunction [InlineData(22, 13, "FunctionNameOnDifferentLine")] [InlineData(24, 30, "IndentedFunction")] [InlineData(24, 52, "IndentedFunction")] - public void CanFindSymbolAtPostion(int lineNumber, int columnNumber, string expectedName) + public void CanFindSymbolAtPosition(int lineNumber, int columnNumber, string expectedName) { SymbolReference reference = AstOperations.FindSymbolAtPosition(s_ast, lineNumber, columnNumber); Assert.NotNull(reference); @@ -57,8 +57,8 @@ public void CanFindSymbolAtPostion(int lineNumber, int columnNumber, string expe } [Theory] - [MemberData(nameof(FindReferencesOfSymbolAtPostionData))] - public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, Range[] symbolRange) + [MemberData(nameof(FindReferencesOfSymbolAtPositionData))] + public void CanFindReferencesOfSymbolAtPosition(int lineNumber, int columnNumber, Range[] symbolRange) { SymbolReference symbol = AstOperations.FindSymbolAtPosition(s_ast, lineNumber, columnNumber); @@ -76,7 +76,7 @@ public void CanFindReferencesOfSymbolAtPostion(int lineNumber, int columnNumber, } } - public static object[][] FindReferencesOfSymbolAtPostionData { get; } = new object[][] + public static object[][] FindReferencesOfSymbolAtPositionData { get; } = new object[][] { 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) } },