From 7419af8d50108a0a8e88d26b908a63c244653bcd Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 18 May 2017 18:18:57 -0700 Subject: [PATCH 1/4] Fix finding variable definitions --- .../Language/FindDeclartionVisitor.cs | 42 +++++++++++-------- .../Language/SymbolReference.cs | 1 + 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs index 4f360e3eb..0c5a012fe 100644 --- a/src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs @@ -14,25 +14,31 @@ namespace Microsoft.PowerShell.EditorServices internal class FindDeclartionVisitor : AstVisitor { private SymbolReference symbolRef; + private string variableName; public SymbolReference FoundDeclartion{ get; private set; } public FindDeclartionVisitor(SymbolReference symbolRef) { this.symbolRef = symbolRef; + if (this.symbolRef.SymbolType == SymbolType.Variable) + { + // converts `$varName` to `varName` or of the form ${varName} to varName + variableName = symbolRef.SymbolName.TrimStart('$').Trim('{', '}'); + } } /// /// Decides if the current function defintion is the right defition - /// for the symbol being searched for. The defintion of the symbol will be a of type + /// for the symbol being searched for. The defintion of the symbol will be a of type /// SymbolType.Function and have the same name as the symbol /// /// A FunctionDefinitionAst in the script's AST - /// A descion to stop searching if the right FunctionDefinitionAst was found, + /// A descion to stop searching if the right FunctionDefinitionAst was found, /// or a decision to continue if it wasn't found public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst) { - // Get the start column number of the function name, + // Get the start column number of the function name, // instead of the the start column of 'function' and create new extent for the functionName int startColumnNumber = functionDefinitionAst.Extent.Text.IndexOf( @@ -62,27 +68,27 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun } /// - /// Decides if the current variable expression is the right defition for - /// the symbol being searched for. The defintion of the symbol will be a of type - /// SymbolType.Variable and have the same name as the symbol + /// Check if the left hand side of an assignmentStatementAst is a VariableExpressionAst + /// with the same name as that of symbolRef. /// - /// A FunctionDefinitionAst in the script's AST - /// A descion to stop searching if the right VariableExpressionAst was found, + /// An AssignmentStatementAst/param> + /// A descion to stop searching if the right VariableExpressionAst was found, /// or a decision to continue if it wasn't found - public override AstVisitAction VisitVariableExpression(VariableExpressionAst variableExpressionAst) + public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst assignmentStatementAst) { - if(symbolRef.SymbolType.Equals(SymbolType.Variable) && - variableExpressionAst.Extent.Text.Equals(symbolRef.SymbolName, StringComparison.CurrentCultureIgnoreCase)) + var variableExprAst = assignmentStatementAst.Left as VariableExpressionAst; + if (variableExprAst == null || + variableName == null || + !variableExprAst.VariablePath.UserPath.Equals( + variableName, + StringComparison.OrdinalIgnoreCase)) { - this.FoundDeclartion = - new SymbolReference( - SymbolType.Variable, - variableExpressionAst.Extent); - - return AstVisitAction.StopVisit; + return AstVisitAction.Continue; } - return AstVisitAction.Continue; + // TODO also find instances of set-variable + FoundDeclartion = new SymbolReference(SymbolType.Variable, variableExprAst.Extent); + return AstVisitAction.StopVisit; } } } diff --git a/src/PowerShellEditorServices/Language/SymbolReference.cs b/src/PowerShellEditorServices/Language/SymbolReference.cs index ed3f6af01..2927fa82a 100644 --- a/src/PowerShellEditorServices/Language/SymbolReference.cs +++ b/src/PowerShellEditorServices/Language/SymbolReference.cs @@ -3,6 +3,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // +using System; using System.Diagnostics; using System.Management.Automation.Language; From f36765016a80d2ba7b146368afe5480f03e74f4d Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 18 May 2017 18:55:31 -0700 Subject: [PATCH 2/4] Add tests for finding variable definitions in other files --- .../LanguageServerTests.cs | 30 +++++++++++++++++++ .../TestFiles/FindReferences.ps1 | 5 +++- .../TestFiles/VariableDefinition.ps1 | 1 + 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 test/PowerShellEditorServices.Test.Host/TestFiles/VariableDefinition.ps1 diff --git a/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs b/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs index 8c85738f4..06dedb141 100644 --- a/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs +++ b/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs @@ -455,6 +455,36 @@ await this.SendRequest( Assert.Equal(7, locations[0].Range.End.Character); } + [Fact] + public async Task FindsDefinitionOfVariableInOtherFile() + { + await this.SendOpenFileEvent("TestFiles\\FindReferences.ps1"); + + Location[] locations = + await this.SendRequest( + DefinitionRequest.Type, + new TextDocumentPositionParams + { + TextDocument = new TextDocumentIdentifier + { + Uri = "TestFiles\\FindReferences.ps1" + }, + Position = new Position + { + Line = 15, + Character = 20, + } + }); + + Assert.NotNull(locations); + Assert.Equal(1, locations.Length); + Assert.EndsWith("VariableDefinition.ps1", locations[0].Uri); + Assert.Equal(0, locations[0].Range.Start.Line); + Assert.Equal(0, locations[0].Range.Start.Character); + Assert.Equal(0, locations[0].Range.End.Line); + Assert.Equal(20, locations[0].Range.End.Character); + } + [Fact] public async Task FindsOccurencesOnFunctionDefinition() { diff --git a/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 b/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 index 4b14bf847..c68c88ad9 100644 --- a/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 +++ b/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 @@ -10,4 +10,7 @@ My-Function $things Write-Output "Hi"; -Write-Output "" \ No newline at end of file +Write-Output "" + +. .\VariableDefinition.ps1 +Write-Output $variableInOtherFile diff --git a/test/PowerShellEditorServices.Test.Host/TestFiles/VariableDefinition.ps1 b/test/PowerShellEditorServices.Test.Host/TestFiles/VariableDefinition.ps1 new file mode 100644 index 000000000..c9f4548ec --- /dev/null +++ b/test/PowerShellEditorServices.Test.Host/TestFiles/VariableDefinition.ps1 @@ -0,0 +1 @@ +$variableInOtherFile = "some value" From 69f48e9cacc238c310d9500b6852476f19a3756c Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 18 May 2017 19:12:03 -0700 Subject: [PATCH 3/4] Add test to find variable defn with special chars in name --- .../LanguageServerTests.cs | 30 +++++++++++++++++++ .../TestFiles/FindReferences.ps1 | 3 ++ 2 files changed, 33 insertions(+) diff --git a/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs b/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs index 06dedb141..eb7ee2dc9 100644 --- a/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs +++ b/test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs @@ -485,6 +485,36 @@ await this.SendRequest( Assert.Equal(20, locations[0].Range.End.Character); } + [Fact] + public async Task FindDefinitionOfVariableWithSpecialChars() + { + await this.SendOpenFileEvent("TestFiles\\FindReferences.ps1"); + + Location[] locations = + await this.SendRequest( + DefinitionRequest.Type, + new TextDocumentPositionParams + { + TextDocument = new TextDocumentIdentifier + { + Uri = "TestFiles\\FindReferences.ps1" + }, + Position = new Position + { + Line = 18, + Character = 24, + } + }); + + Assert.NotNull(locations); + Assert.Equal(1, locations.Length); + Assert.EndsWith("FindReferences.ps1", locations[0].Uri); + Assert.Equal(17, locations[0].Range.Start.Line); + Assert.Equal(0, locations[0].Range.Start.Character); + Assert.Equal(17, locations[0].Range.End.Line); + Assert.Equal(27, locations[0].Range.End.Character); + } + [Fact] public async Task FindsOccurencesOnFunctionDefinition() { diff --git a/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 b/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 index c68c88ad9..ed82df5e4 100644 --- a/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 +++ b/test/PowerShellEditorServices.Test.Host/TestFiles/FindReferences.ps1 @@ -14,3 +14,6 @@ Write-Output "" . .\VariableDefinition.ps1 Write-Output $variableInOtherFile + +${variable-with-weird-name} = "this variable has special characters" +Write-Output ${variable-with-weird-name} From 8ef58a11c322a560bd702fba1c75e0d44247ab60 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 18 May 2017 20:10:10 -0700 Subject: [PATCH 4/4] Fix typo in class name --- .../Language/AstOperations.cs | 34 +++++++++---------- ...onVisitor.cs => FindDeclarationVisitor.cs} | 18 +++++----- 2 files changed, 26 insertions(+), 26 deletions(-) rename src/PowerShellEditorServices/Language/{FindDeclartionVisitor.cs => FindDeclarationVisitor.cs} (83%) diff --git a/src/PowerShellEditorServices/Language/AstOperations.cs b/src/PowerShellEditorServices/Language/AstOperations.cs index 67533903a..da45431bd 100644 --- a/src/PowerShellEditorServices/Language/AstOperations.cs +++ b/src/PowerShellEditorServices/Language/AstOperations.cs @@ -24,7 +24,7 @@ namespace Microsoft.PowerShell.EditorServices internal static class AstOperations { /// - /// Gets completions for the symbol found in the Ast at + /// Gets completions for the symbol found in the Ast at /// the given file offset. /// /// @@ -47,14 +47,14 @@ internal static class AstOperations /// symbol at the given offset. /// static public async Task GetCompletions( - Ast scriptAst, - Token[] currentTokens, + Ast scriptAst, + Token[] currentTokens, int fileOffset, PowerShellContext powerShellContext, CancellationToken cancellationToken) { var type = scriptAst.Extent.StartScriptPosition.GetType(); - var method = + var method = #if CoreCLR type.GetMethod( "CloneWithNewOffset", @@ -67,9 +67,9 @@ static public async Task GetCompletions( new[] { typeof(int) }, null); #endif - IScriptPosition cursorPosition = + IScriptPosition cursorPosition = (IScriptPosition)method.Invoke( - scriptAst.Extent.StartScriptPosition, + scriptAst.Extent.StartScriptPosition, new object[] { fileOffset }); Logger.Write( @@ -138,7 +138,7 @@ static public async Task GetCompletions( } /// - /// Finds the symbol at a given file location + /// Finds the symbol at a given file location /// /// The abstract syntax tree of the given script /// The line number of the cursor for the given script @@ -176,15 +176,15 @@ static public SymbolReference FindCommandAtPosition(Ast scriptAst, int lineNumbe /// Dictionary maping aliases to cmdlets for finding alias references /// static public IEnumerable FindReferencesOfSymbol( - Ast scriptAst, - SymbolReference symbolReference, + Ast scriptAst, + SymbolReference symbolReference, Dictionary> CmdletToAliasDictionary, Dictionary AliasToCmdletDictionary) { // find the symbol evaluators for the node types we are handling - FindReferencesVisitor referencesVisitor = + FindReferencesVisitor referencesVisitor = new FindReferencesVisitor( - symbolReference, + symbolReference, CmdletToAliasDictionary, AliasToCmdletDictionary); scriptAst.Visit(referencesVisitor); @@ -202,8 +202,8 @@ static public IEnumerable FindReferencesOfSymbol( /// A collection of SymbolReference objects that are refrences to the symbolRefrence /// not including aliases static public IEnumerable FindReferencesOfSymbol( - ScriptBlockAst scriptAst, - SymbolReference foundSymbol, + ScriptBlockAst scriptAst, + SymbolReference foundSymbol, bool needsAliases) { FindReferencesVisitor referencesVisitor = @@ -214,7 +214,7 @@ static public IEnumerable FindReferencesOfSymbol( } /// - /// Finds the definition of the symbol + /// Finds the definition of the symbol /// /// The abstract syntax tree of the given script /// The symbol that we are looking for the definition of @@ -223,12 +223,12 @@ static public SymbolReference FindDefinitionOfSymbol( Ast scriptAst, SymbolReference symbolReference) { - FindDeclartionVisitor declarationVisitor = - new FindDeclartionVisitor( + FindDeclarationVisitor declarationVisitor = + new FindDeclarationVisitor( symbolReference); scriptAst.Visit(declarationVisitor); - return declarationVisitor.FoundDeclartion; + return declarationVisitor.FoundDeclaration; } /// diff --git a/src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs similarity index 83% rename from src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs rename to src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs index 0c5a012fe..9aaf1b459 100644 --- a/src/PowerShellEditorServices/Language/FindDeclartionVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs @@ -9,16 +9,16 @@ namespace Microsoft.PowerShell.EditorServices { /// - /// The vistor used to find the defintion of a symbol + /// The visitor used to find the definition of a symbol /// - internal class FindDeclartionVisitor : AstVisitor + internal class FindDeclarationVisitor : AstVisitor { private SymbolReference symbolRef; private string variableName; - public SymbolReference FoundDeclartion{ get; private set; } + public SymbolReference FoundDeclaration{ get; private set; } - public FindDeclartionVisitor(SymbolReference symbolRef) + public FindDeclarationVisitor(SymbolReference symbolRef) { this.symbolRef = symbolRef; if (this.symbolRef.SymbolType == SymbolType.Variable) @@ -29,8 +29,8 @@ public FindDeclartionVisitor(SymbolReference symbolRef) } /// - /// Decides if the current function defintion is the right defition - /// for the symbol being searched for. The defintion of the symbol will be a of type + /// Decides if the current function definition is the right definition + /// for the symbol being searched for. The definition of the symbol will be a of type /// SymbolType.Function and have the same name as the symbol /// /// A FunctionDefinitionAst in the script's AST @@ -56,7 +56,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun if (symbolRef.SymbolType.Equals(SymbolType.Function) && nameExtent.Text.Equals(symbolRef.ScriptRegion.Text, StringComparison.CurrentCultureIgnoreCase)) { - this.FoundDeclartion = + this.FoundDeclaration = new SymbolReference( SymbolType.Function, nameExtent); @@ -72,7 +72,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun /// with the same name as that of symbolRef. /// /// An AssignmentStatementAst/param> - /// A descion to stop searching if the right VariableExpressionAst was found, + /// A decision to stop searching if the right VariableExpressionAst was found, /// or a decision to continue if it wasn't found public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst assignmentStatementAst) { @@ -87,7 +87,7 @@ public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst a } // TODO also find instances of set-variable - FoundDeclartion = new SymbolReference(SymbolType.Variable, variableExprAst.Extent); + FoundDeclaration = new SymbolReference(SymbolType.Variable, variableExprAst.Extent); return AstVisitAction.StopVisit; } }