Integrate FindSymbolsVisitor into ReferenceTable

We were missing the distinction between properties and enum members,
as well as DSC configuration.

Author: andyleejordan

src/PowerShellEditorServices/Services/Symbols/ReferenceTable.cs

@@ -111,6 +111,10 @@ public override AstVisitAction VisitCommand(CommandAst commandAst)
 
         public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst)
         {
+            SymbolType symbolType = functionDefinitionAst.IsWorkflow
+                ? SymbolType.Workflow
+                : SymbolType.Function;
+
             // Extent for constructors and method trigger both this and VisitFunctionMember(). Covered in the latter.
             // This will not exclude nested functions as they have ScriptBlockAst as parent
             if (functionDefinitionAst.Parent is FunctionMemberAst)
@@ -119,9 +123,12 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
             }
 
             // We only want the function name as the extent for highlighting (and so forth).
+            //
+            // TODO: After we replace the deprecated SymbolInformation usage with DocumentSymbol,
+            // we'll want *both* the name extent and the full extent.
             IScriptExtent nameExtent = VisitorUtils.GetNameExtent(functionDefinitionAst);
             _references.AddReference(
-                SymbolType.Function,
+                symbolType,
                 functionDefinitionAst.Name,
                 nameExtent,
                 isDeclaration: true);
@@ -154,7 +161,9 @@ public override AstVisitAction VisitVariableExpression(VariableExpressionAst var
 
         public override AstVisitAction VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst)
         {
-            SymbolType symbolType = typeDefinitionAst.IsEnum ? SymbolType.Enum : SymbolType.Class;
+            SymbolType symbolType = typeDefinitionAst.IsEnum
+                ? SymbolType.Enum
+                : SymbolType.Class;
 
             IScriptExtent nameExtent = VisitorUtils.GetNameExtent(typeDefinitionAst);
             _references.AddReference(symbolType, typeDefinitionAst.Name, nameExtent);
@@ -196,13 +205,28 @@ public override AstVisitAction VisitFunctionMember(FunctionMemberAst functionMem
 
         public override AstVisitAction VisitPropertyMember(PropertyMemberAst propertyMemberAst)
         {
+            SymbolType symbolType =
+                propertyMemberAst.Parent is TypeDefinitionAst typeAst && typeAst.IsEnum
+                    ? SymbolType.EnumMember : SymbolType.Property;
+
             IScriptExtent nameExtent = VisitorUtils.GetNameExtent(propertyMemberAst, false);
             _references.AddReference(
-                SymbolType.Property,
+                symbolType,
                 VisitorUtils.GetMemberOverloadName(propertyMemberAst, false),
                 nameExtent);
 
             return AstVisitAction.Continue;
         }
+
+        public override AstVisitAction VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst)
+        {
+            IScriptExtent nameExtent = VisitorUtils.GetNameExtent(configurationDefinitionAst);
+            _references.AddReference(
+                SymbolType.Configuration,
+                nameExtent.Text,
+                nameExtent);
+
+            return AstVisitAction.Continue;
+        }
     }
 }

src/PowerShellEditorServices/Services/Symbols/Vistors/FindSymbolsVisitor.cs

@@ -3,184 +3,11 @@
 
 #nullable enable
 
-using Microsoft.PowerShell.EditorServices.Utility;
 using System.Collections.Generic;
 using System.Management.Automation.Language;
 
 namespace Microsoft.PowerShell.EditorServices.Services.Symbols
 {
-    /// <summary>
-    /// The visitor used to find all the symbols (variables, functions and class defs etc) in the AST.
-    /// </summary>
-    internal class FindSymbolsVisitor : AstVisitor2
-    {
-        public List<SymbolReference> SymbolReferences { get; }
-
-        public FindSymbolsVisitor() => SymbolReferences = new List<SymbolReference>();
-
-        /// <summary>
-        /// Adds each function definition to symbol reference list
-        /// </summary>
-        /// <param name="functionDefinitionAst">A FunctionDefinitionAst in the script's AST</param>
-        /// <returns>A visit action that continues the search for references</returns>
-        public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst)
-        {
-            // Extent for constructors and method trigger both this and VisitFunctionMember(). Covered in the latter.
-            // This will not exclude nested functions as they have ScriptBlockAst as parent
-            if (functionDefinitionAst.Parent is FunctionMemberAst)
-            {
-                return AstVisitAction.Continue;
-            }
-
-            (int startColumn, int startLine) = VisitorUtils.GetNameStartColumnAndLineFromAst(functionDefinitionAst);
-            IScriptExtent nameExtent = GetNewExtent(functionDefinitionAst, functionDefinitionAst.Name, startLine, startColumn);
-
-            SymbolType symbolType =
-                functionDefinitionAst.IsWorkflow ?
-                    SymbolType.Workflow : SymbolType.Function;
-
-            SymbolReferences.Add(
-                new SymbolReference(
-                    symbolType,
-                    nameExtent));
-
-            return AstVisitAction.Continue;
-        }
-
-        /// <summary>
-        /// Adds each script scoped variable assignment to symbol reference list
-        /// </summary>
-        /// <param name="variableExpressionAst">A VariableExpressionAst in the script's AST</param>
-        /// <returns>A visit action that continues the search for references</returns>
-        public override AstVisitAction VisitVariableExpression(VariableExpressionAst variableExpressionAst)
-        {
-            if (!IsAssignedAtScriptScope(variableExpressionAst))
-            {
-                return AstVisitAction.Continue;
-            }
-
-            SymbolReferences.Add(
-                new SymbolReference(
-                    SymbolType.Variable,
-                    variableExpressionAst.Extent));
-
-            return AstVisitAction.Continue;
-        }
-
-        private static bool IsAssignedAtScriptScope(VariableExpressionAst variableExpressionAst)
-        {
-            Ast parent = variableExpressionAst.Parent;
-            if (parent is not AssignmentStatementAst)
-            {
-                return false;
-            }
-
-            parent = parent.Parent;
-            return parent is null || parent.Parent is null || parent.Parent.Parent is null;
-        }
-
-        /// <summary>
-        /// Adds class and enum AST to symbol reference list
-        /// </summary>
-        /// <param name="typeDefinitionAst">A TypeDefinitionAst in the script's AST</param>
-        /// <returns>A visit action that continues the search for references</returns>
-        public override AstVisitAction VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst)
-        {
-            (int startColumn, int startLine) = VisitorUtils.GetNameStartColumnAndLineFromAst(typeDefinitionAst);
-            IScriptExtent nameExtent = GetNewExtent(typeDefinitionAst, typeDefinitionAst.Name, startLine, startColumn);
-
-            SymbolType symbolType =
-                typeDefinitionAst.IsEnum ?
-                    SymbolType.Enum : SymbolType.Class;
-
-            SymbolReferences.Add(
-                new SymbolReference(
-                    symbolType,
-                    nameExtent));
-
-            return AstVisitAction.Continue;
-        }
-
-        /// <summary>
-        /// Adds class method and constructor AST to symbol reference list
-        /// </summary>
-        /// <param name="functionMemberAst">A FunctionMemberAst in the script's AST</param>
-        /// <returns>A visit action that continues the search for references</returns>
-        public override AstVisitAction VisitFunctionMember(FunctionMemberAst functionMemberAst)
-        {
-            (int startColumn, int startLine) = VisitorUtils.GetNameStartColumnAndLineFromAst(functionMemberAst);
-            IScriptExtent nameExtent = GetNewExtent(functionMemberAst, VisitorUtils.GetMemberOverloadName(functionMemberAst, false, false), startLine, startColumn);
-
-            SymbolType symbolType =
-                functionMemberAst.IsConstructor ?
-                    SymbolType.Constructor : SymbolType.Method;
-
-            SymbolReferences.Add(
-                new SymbolReference(
-                    symbolType,
-                    nameExtent));
-
-            return AstVisitAction.Continue;
-        }
-
-        /// <summary>
-        /// Adds class property AST to symbol reference list
-        /// </summary>
-        /// <param name="propertyMemberAst">A PropertyMemberAst in the script's AST</param>
-        /// <returns>A visit action that continues the search for references</returns>
-        public override AstVisitAction VisitPropertyMember(PropertyMemberAst propertyMemberAst)
-        {
-            SymbolType symbolType =
-                propertyMemberAst.Parent is TypeDefinitionAst typeAst && typeAst.IsEnum ?
-                    SymbolType.EnumMember : SymbolType.Property;
-
-            bool isEnumMember = symbolType.Equals(SymbolType.EnumMember);
-            (int startColumn, int startLine) = VisitorUtils.GetNameStartColumnAndLineFromAst(propertyMemberAst, isEnumMember);
-            IScriptExtent nameExtent = GetNewExtent(propertyMemberAst, propertyMemberAst.Name, startLine, startColumn);
-
-            SymbolReferences.Add(
-                new SymbolReference(
-                    symbolType,
-                    nameExtent));
-
-            return AstVisitAction.Continue;
-        }
-
-        /// <summary>
-        /// Adds DSC configuration AST to symbol reference list
-        /// </summary>
-        /// <param name="configurationDefinitionAst">A ConfigurationDefinitionAst in the script's AST</param>
-        /// <returns>A visit action that continues the search for references</returns>
-        public override AstVisitAction VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst)
-        {
-            (int startColumn, int startLine) = VisitorUtils.GetNameStartColumnAndLineFromAst(configurationDefinitionAst);
-            IScriptExtent nameExtent = GetNewExtent(configurationDefinitionAst, configurationDefinitionAst.InstanceName.Extent.Text, startLine, startColumn);
-
-            SymbolReferences.Add(
-                new SymbolReference(
-                    SymbolType.Configuration,
-                    nameExtent));
-
-            return AstVisitAction.Continue;
-        }
-
-        /// <summary>
-        /// Gets a new ScriptExtent for a given Ast with same range but modified Text
-        /// </summary>
-        private static ScriptExtent GetNewExtent(Ast ast, string text, int startLine, int startColumn)
-        {
-            return new ScriptExtent()
-            {
-                Text = text,
-                StartLineNumber = startLine,
-                EndLineNumber = ast.Extent.EndLineNumber,
-                StartColumnNumber = startColumn,
-                EndColumnNumber = ast.Extent.EndColumnNumber,
-                File = ast.Extent.File
-            };
-        }
-    }
-
     /// <summary>
     /// Visitor to find all the keys in Hashtable AST
     /// </summary>

test/PowerShellEditorServices.Test/Language/SymbolsServiceTests.cs

@@ -559,9 +559,7 @@ public async Task FindsDetailsWithSignatureForMethod()
         [Fact]
         public void FindsSymbolsInFile()
         {
-            List<SymbolReference> symbolsResult =
-                FindSymbolsInFile(
-                    FindSymbolsInMultiSymbolFile.SourceDetails);
+            List<SymbolReference> symbolsResult = FindSymbolsInFile(FindSymbolsInMultiSymbolFile.SourceDetails);
 
             Assert.Equal(4, symbolsResult.Count(symbolReference => symbolReference.SymbolType == SymbolType.Function));
             Assert.Equal(3, symbolsResult.Count(symbolReference => symbolReference.SymbolType == SymbolType.Variable));
@@ -622,50 +620,47 @@ public void FindsSymbolsInFile()
         [Fact]
         public void FindsSymbolsWithNewLineInFile()
         {
-            List<SymbolReference> symbolsResult =
-                FindSymbolsInFile(
-                    FindSymbolsInNewLineSymbolFile.SourceDetails);
+            List<SymbolReference> symbols = FindSymbolsInFile(FindSymbolsInNewLineSymbolFile.SourceDetails);
 
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.Function));
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.Class));
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.Constructor));
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.Property));
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.Method));
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.Enum));
-            Assert.Single(symbolsResult.Where(symbolReference => symbolReference.SymbolType == SymbolType.EnumMember));
-
-            SymbolReference firstFunctionSymbol = symbolsResult.First(r => r.SymbolType == SymbolType.Function);
+            SymbolReference firstFunctionSymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.Function));
             Assert.Equal("returnTrue", firstFunctionSymbol.SymbolName);
             Assert.Equal(2, firstFunctionSymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(1, firstFunctionSymbol.ScriptRegion.StartColumnNumber);
 
-            SymbolReference firstClassSymbol = symbolsResult.First(r => r.SymbolType == SymbolType.Class);
+            SymbolReference firstClassSymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.Class));
             Assert.Equal("NewLineClass", firstClassSymbol.SymbolName);
             Assert.Equal(7, firstClassSymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(1, firstClassSymbol.ScriptRegion.StartColumnNumber);
 
-            SymbolReference firstConstructorSymbol = symbolsResult.First(r => r.SymbolType == SymbolType.Constructor);
-            Assert.Equal("NewLineClass()", firstConstructorSymbol.SymbolName);
+            SymbolReference firstConstructorSymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.Constructor));
+            Assert.Equal("NewLineClass.NewLineClass()", firstConstructorSymbol.SymbolName);
             Assert.Equal(8, firstConstructorSymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(5, firstConstructorSymbol.ScriptRegion.StartColumnNumber);
 
-            SymbolReference firstPropertySymbol = symbolsResult.First(r => r.SymbolType == SymbolType.Property);
-            Assert.Equal("SomePropWithDefault", firstPropertySymbol.SymbolName);
+            SymbolReference firstPropertySymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.Property));
+            Assert.Equal("NewLineClass.SomePropWithDefault", firstPropertySymbol.SymbolName);
             Assert.Equal(15, firstPropertySymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(5, firstPropertySymbol.ScriptRegion.StartColumnNumber);
 
-            SymbolReference firstMethodSymbol = symbolsResult.First(r => r.SymbolType == SymbolType.Method);
-            Assert.Equal("MyClassMethod([MyNewLineEnum]$param1)", firstMethodSymbol.SymbolName);
+            SymbolReference firstMethodSymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.Method));
+            Assert.Equal("NewLineClass.MyClassMethod([MyNewLineEnum]$param1)", firstMethodSymbol.SymbolName);
             Assert.Equal(20, firstMethodSymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(5, firstMethodSymbol.ScriptRegion.StartColumnNumber);
 
-            SymbolReference firstEnumSymbol = symbolsResult.First(r => r.SymbolType == SymbolType.Enum);
+            SymbolReference firstEnumSymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.Enum));
             Assert.Equal("MyNewLineEnum", firstEnumSymbol.SymbolName);
             Assert.Equal(26, firstEnumSymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(1, firstEnumSymbol.ScriptRegion.StartColumnNumber);
 
-            SymbolReference firstEnumMemberSymbol = symbolsResult.First(r => r.SymbolType == SymbolType.EnumMember);
-            Assert.Equal("First", firstEnumMemberSymbol.SymbolName);
+            SymbolReference firstEnumMemberSymbol =
+                Assert.Single(symbols.Where(symbolReference => symbolReference.SymbolType == SymbolType.EnumMember));
+            Assert.Equal("MyNewLineEnum.First", firstEnumMemberSymbol.SymbolName);
             Assert.Equal(27, firstEnumMemberSymbol.ScriptRegion.StartLineNumber);
             Assert.Equal(5, firstEnumMemberSymbol.ScriptRegion.StartColumnNumber);
         }