From 4bea2fee5a2cfa467aa0cc5b6adee58fc2d5bf56 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Tue, 17 Jul 2018 23:05:42 -0700 Subject: [PATCH 1/5] better go to definition --- .../Language/FindDeclarationVisitor.cs | 67 ++++++++++++++++--- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs index e706e1199..74ba7bb96 100644 --- a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs @@ -4,6 +4,8 @@ // using System; +using System.Collections.Generic; +using System.Linq; using System.Management.Automation.Language; namespace Microsoft.PowerShell.EditorServices @@ -76,19 +78,68 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun /// or a decision to continue if it wasn't found public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst assignmentStatementAst) { - var variableExprAst = assignmentStatementAst.Left as VariableExpressionAst; - if (variableExprAst == null || - variableName == null || - !variableExprAst.VariablePath.UserPath.Equals( + if (variableName == null) + { + return AstVisitAction.Continue; + } + + // The AssignmentStatementAst could contain either of the following Ast types: + // VariableExpressionAst, ArrayLiteralAst, ConvertExpressionAst, AttributedExpressionAst + // We might need to recurse down the tree to find the VariableExpressionAst we're looking for + List asts = FindMatchingAsts(assignmentStatementAst.Left, variableName); + if (asts.Count > 0) + { + FoundDeclaration = new SymbolReference(SymbolType.Variable, asts.FirstOrDefault().Extent); + return AstVisitAction.StopVisit; + } + return AstVisitAction.Continue; + } + + /// + /// Takes in an ExpressionAst and recurses until it finds all VariableExpressionAst and returns the list of them. + /// We expect this ExpressionAst to be either: VariableExpressionAst, ArrayLiteralAst, ConvertExpressionAst, AttributedExpressionAst + /// + /// An ExpressionAst + /// The name of the variable we are trying to find + /// A list of ExpressionAsts that match the variable name provided + private static List FindMatchingAsts (ExpressionAst ast, string variableName) { + + // VaraibleExpressionAst case - aka base case. This will return a list with the variableExpressionAst in it or an empty list if it's not the right one. + var variableExprAst = ast as VariableExpressionAst; + if (variableExprAst != null) + { + if (variableExprAst.VariablePath.UserPath.Equals( variableName, StringComparison.OrdinalIgnoreCase)) + { + return new List{ variableExprAst }; + } + return new List(); + } + + // VariableExpressionAsts could be an element of an ArrayLiteralAst. This adds all the elements to the call stack. + var arrayLiteralAst = ast as ArrayLiteralAst; + if (arrayLiteralAst != null) { - return AstVisitAction.Continue; + return (arrayLiteralAst.Elements.SelectMany(e => FindMatchingAsts(e, variableName)).ToList()); } - // TODO also find instances of set-variable - FoundDeclaration = new SymbolReference(SymbolType.Variable, variableExprAst.Extent); - return AstVisitAction.StopVisit; + // The ConvertExpressionAst (static casting for example `[string]$foo = "asdf"`) could contain a VariableExpressionAst so we recurse down + var convertExprAst = ast as ConvertExpressionAst; + if (convertExprAst != null && convertExprAst.Child != null) + { + return FindMatchingAsts(convertExprAst.Child, variableName); + } + + // The AttributedExpressionAst (any attribute for example `[NotNull()]$foo = "asdf"`) could contain a VariableExpressionAst so we recurse down + var attributedExprAst = ast as AttributedExpressionAst; + if (attributedExprAst != null && attributedExprAst.Child != null) + { + return FindMatchingAsts(attributedExprAst.Child, variableName); + }; + + // We shouldn't ever get here, but in case a new Ast type is added to PowerShell, this fails gracefully + return new List(); } } } From dcd687e6f119a631ed61d46f947dd4ed612baf15 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 18 Jul 2018 15:33:39 -0700 Subject: [PATCH 2/5] switched to an astvisitor --- .../Language/FindDeclarationVisitor.cs | 72 +++++++++---------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs index 74ba7bb96..643708c69 100644 --- a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs @@ -86,60 +86,54 @@ public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst a // The AssignmentStatementAst could contain either of the following Ast types: // VariableExpressionAst, ArrayLiteralAst, ConvertExpressionAst, AttributedExpressionAst // We might need to recurse down the tree to find the VariableExpressionAst we're looking for - List asts = FindMatchingAsts(assignmentStatementAst.Left, variableName); - if (asts.Count > 0) + // if (variableExpressionAst.VariablePath.UserPath.Equals(variableName, StringComparison.OrdinalIgnoreCase)) + FindDeclarationVariableExpressionVisitor visitor = new FindDeclarationVariableExpressionVisitor(symbolRef); + assignmentStatementAst.Visit(visitor); + + if (visitor.FoundDeclaration != null) { - FoundDeclaration = new SymbolReference(SymbolType.Variable, asts.FirstOrDefault().Extent); + FoundDeclaration = visitor.FoundDeclaration; return AstVisitAction.StopVisit; } return AstVisitAction.Continue; } /// - /// Takes in an ExpressionAst and recurses until it finds all VariableExpressionAst and returns the list of them. - /// We expect this ExpressionAst to be either: VariableExpressionAst, ArrayLiteralAst, ConvertExpressionAst, AttributedExpressionAst + /// The private visitor used to find the variable expression that matches a symbol /// - /// An ExpressionAst - /// The name of the variable we are trying to find - /// A list of ExpressionAsts that match the variable name provided - private static List FindMatchingAsts (ExpressionAst ast, string variableName) { - - // VaraibleExpressionAst case - aka base case. This will return a list with the variableExpressionAst in it or an empty list if it's not the right one. - var variableExprAst = ast as VariableExpressionAst; - if (variableExprAst != null) + private class FindDeclarationVariableExpressionVisitor : AstVisitor + { + private SymbolReference symbolRef; + private string variableName; + + public SymbolReference FoundDeclaration{ get; private set; } + + public FindDeclarationVariableExpressionVisitor(SymbolReference symbolRef) { - if (variableExprAst.VariablePath.UserPath.Equals( - variableName, - StringComparison.OrdinalIgnoreCase)) + this.symbolRef = symbolRef; + if (this.symbolRef.SymbolType == SymbolType.Variable) { - return new List{ variableExprAst }; + // converts `$varName` to `varName` or of the form ${varName} to varName + variableName = symbolRef.SymbolName.TrimStart('$').Trim('{', '}'); } - return new List(); } - // VariableExpressionAsts could be an element of an ArrayLiteralAst. This adds all the elements to the call stack. - var arrayLiteralAst = ast as ArrayLiteralAst; - if (arrayLiteralAst != null) - { - return (arrayLiteralAst.Elements.SelectMany(e => FindMatchingAsts(e, variableName)).ToList()); - } - - // The ConvertExpressionAst (static casting for example `[string]$foo = "asdf"`) could contain a VariableExpressionAst so we recurse down - var convertExprAst = ast as ConvertExpressionAst; - if (convertExprAst != null && convertExprAst.Child != null) + /// + /// Check if the VariableExpressionAst has the same name as that of symbolRef. + /// + /// A VariableExpressionAst + /// A decision 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) { - return FindMatchingAsts(convertExprAst.Child, variableName); + if(variableExpressionAst.VariablePath.UserPath.Equals(variableName, StringComparison.OrdinalIgnoreCase)) + { + // TODO also find instances of set-variable + FoundDeclaration = new SymbolReference(SymbolType.Variable, variableExpressionAst.Extent); + return AstVisitAction.StopVisit; + } + return AstVisitAction.Continue; } - - // The AttributedExpressionAst (any attribute for example `[NotNull()]$foo = "asdf"`) could contain a VariableExpressionAst so we recurse down - var attributedExprAst = ast as AttributedExpressionAst; - if (attributedExprAst != null && attributedExprAst.Child != null) - { - return FindMatchingAsts(attributedExprAst.Child, variableName); - }; - - // We shouldn't ever get here, but in case a new Ast type is added to PowerShell, this fails gracefully - return new List(); } } } From 241e50a78698e2cac6854f8e88a3e8283dd41a79 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 18 Jul 2018 16:08:55 -0700 Subject: [PATCH 3/5] add a few more overrides --- .../Language/FindDeclarationVisitor.cs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs index 643708c69..47080d401 100644 --- a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs @@ -56,7 +56,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun }; if (symbolRef.SymbolType.Equals(SymbolType.Function) && - nameExtent.Text.Equals(symbolRef.ScriptRegion.Text, StringComparison.CurrentCultureIgnoreCase)) + nameExtent.Text.Equals(symbolRef.ScriptRegion.Text, StringComparison.CurrentCultureIgnoreCase)) { this.FoundDeclaration = new SymbolReference( @@ -83,10 +83,7 @@ public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst a return AstVisitAction.Continue; } - // The AssignmentStatementAst could contain either of the following Ast types: - // VariableExpressionAst, ArrayLiteralAst, ConvertExpressionAst, AttributedExpressionAst - // We might need to recurse down the tree to find the VariableExpressionAst we're looking for - // if (variableExpressionAst.VariablePath.UserPath.Equals(variableName, StringComparison.OrdinalIgnoreCase)) + // We want to check VariableExpressionAsts from within this AssignmentStatementAst so we visit it. FindDeclarationVariableExpressionVisitor visitor = new FindDeclarationVariableExpressionVisitor(symbolRef); assignmentStatementAst.Visit(visitor); @@ -134,6 +131,24 @@ public override AstVisitAction VisitVariableExpression(VariableExpressionAst var } return AstVisitAction.Continue; } + + public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst) + { + // We don't want to discover any variables in nested functions - they get their own scope. + return AstVisitAction.SkipChildren; + } + + public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionAst scriptBlockExpressionAst) + { + // We don't want to discover any variables in script block expressions - they get their own scope. + return AstVisitAction.SkipChildren; + } + + public override AstVisitAction VisitTrap(TrapStatementAst trapStatementAst) + { + // We don't want to discover any variables in traps - they get their own scope. + return AstVisitAction.SkipChildren; + } } } } From 5d07e71e9827c77ab98f9464f62ee5f2b85e3df4 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 19 Jul 2018 15:54:53 -0700 Subject: [PATCH 4/5] only look at LHS --- .../Language/FindDeclarationVisitor.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs index 47080d401..d83a33109 100644 --- a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs @@ -85,7 +85,7 @@ public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst a // We want to check VariableExpressionAsts from within this AssignmentStatementAst so we visit it. FindDeclarationVariableExpressionVisitor visitor = new FindDeclarationVariableExpressionVisitor(symbolRef); - assignmentStatementAst.Visit(visitor); + assignmentStatementAst.Left.Visit(visitor); if (visitor.FoundDeclaration != null) { @@ -123,7 +123,7 @@ public FindDeclarationVariableExpressionVisitor(SymbolReference symbolRef) /// or a decision to continue if it wasn't found public override AstVisitAction VisitVariableExpression(VariableExpressionAst variableExpressionAst) { - if(variableExpressionAst.VariablePath.UserPath.Equals(variableName, StringComparison.OrdinalIgnoreCase)) + if (variableExpressionAst.VariablePath.UserPath.Equals(variableName, StringComparison.OrdinalIgnoreCase)) { // TODO also find instances of set-variable FoundDeclaration = new SymbolReference(SymbolType.Variable, variableExpressionAst.Extent); @@ -132,21 +132,15 @@ public override AstVisitAction VisitVariableExpression(VariableExpressionAst var return AstVisitAction.Continue; } - public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst) + public override AstVisitAction VisitMemberExpression(MemberExpressionAst functionDefinitionAst) { - // We don't want to discover any variables in nested functions - they get their own scope. + // We don't want to discover any variables in member expressisons (`$something.Foo`) return AstVisitAction.SkipChildren; } - public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionAst scriptBlockExpressionAst) + public override AstVisitAction VisitIndexExpression(IndexExpressionAst functionDefinitionAst) { - // We don't want to discover any variables in script block expressions - they get their own scope. - return AstVisitAction.SkipChildren; - } - - public override AstVisitAction VisitTrap(TrapStatementAst trapStatementAst) - { - // We don't want to discover any variables in traps - they get their own scope. + // We don't want to discover any variables in index expressions (`$something[0]`) return AstVisitAction.SkipChildren; } } From 0501314da286d010b26fb06afd4e3a5dc75ff934 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Sun, 22 Jul 2018 09:21:33 -0700 Subject: [PATCH 5/5] fix indent --- .../Language/FindDeclarationVisitor.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs index d83a33109..1f7e8a649 100644 --- a/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs +++ b/src/PowerShellEditorServices/Language/FindDeclarationVisitor.cs @@ -115,12 +115,12 @@ public FindDeclarationVariableExpressionVisitor(SymbolReference symbolRef) } } - /// - /// Check if the VariableExpressionAst has the same name as that of symbolRef. - /// - /// A VariableExpressionAst - /// A decision to stop searching if the right VariableExpressionAst was found, - /// or a decision to continue if it wasn't found + /// + /// Check if the VariableExpressionAst has the same name as that of symbolRef. + /// + /// A VariableExpressionAst + /// A decision 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) { if (variableExpressionAst.VariablePath.UserPath.Equals(variableName, StringComparison.OrdinalIgnoreCase))