From 1e4e0539129ec7bba979021bdc044b510b4819a9 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Fri, 14 Jun 2024 19:58:33 +0200
Subject: [PATCH] Refactor and fix symbols provider

---
 .../src/services/symbols.ts                   |   8 ++
 .../src/providers/symbolsProvider.ts          | 102 +++++++++++-------
 2 files changed, 74 insertions(+), 36 deletions(-)

diff --git a/server/gx-workflow-ls-native/src/services/symbols.ts b/server/gx-workflow-ls-native/src/services/symbols.ts
index 2dc1f00..445dd25 100644
--- a/server/gx-workflow-ls-native/src/services/symbols.ts
+++ b/server/gx-workflow-ls-native/src/services/symbols.ts
@@ -1,4 +1,5 @@
 import { SymbolsProviderBase } from "@gxwf/server-common/src/providers/symbolsProvider";
+import { PropertyASTNode } from "@gxwf/yaml-language-service/src/parser/astTypes";
 import { injectable } from "inversify";
 
 @injectable()
@@ -15,4 +16,11 @@ export class NativeWorkflowSymbolsProvider extends SymbolsProviderBase {
     ]);
     this.stepContainerNames = new Set(["steps"]);
   }
+
+  protected getSymbolName(property: PropertyASTNode): string {
+    if (this.isStepProperty(property)) {
+      return this.getNodeName(property.valueNode) ?? "unnamed";
+    }
+    return super.getSymbolName(property);
+  }
 }
diff --git a/server/packages/server-common/src/providers/symbolsProvider.ts b/server/packages/server-common/src/providers/symbolsProvider.ts
index 0c21987..d880043 100644
--- a/server/packages/server-common/src/providers/symbolsProvider.ts
+++ b/server/packages/server-common/src/providers/symbolsProvider.ts
@@ -28,46 +28,24 @@ export class SymbolsProviderBase implements SymbolsProvider {
     const collectOutlineEntries = (node: ASTNode, result: DocumentSymbol[]): void => {
       if (node.type === "array") {
         node.items.forEach((node, index) => {
-          if (node) {
-            const name = this.getArrayNodeName(node, index);
-            if (this.symbolNamesToIgnore.has(name)) {
-              return;
-            }
-            const range = documentContext.nodeManager.getNodeRange(node);
-            const selectionRange = range;
-            const symbol = { name, kind: this.getSymbolKind(node.type), range, selectionRange, children: [] };
+          if (!node) {
+            return;
+          }
+          const symbol = this.getArrayItemSymbol(node, index, documentContext);
+          if (symbol) {
             result.push(symbol);
-            toVisit.push({ result: symbol.children, node });
+            toVisit.push({ result: symbol.children!, node });
           }
         });
       } else if (node.type === "object") {
         node.properties.forEach((property: PropertyASTNode) => {
-          const valueNode = property.valueNode;
-          if (valueNode) {
-            let name = undefined;
-            let customSymbol = undefined;
-            if (this.isStepProperty(property)) {
-              name = this.getNodeName(property.valueNode);
-              customSymbol = "step";
-            }
-            name = name || this.getKeyLabel(property);
-            customSymbol = customSymbol || name;
-            if (this.symbolNamesToIgnore.has(name)) {
-              return;
-            }
-            const range = documentContext.nodeManager.getNodeRange(property);
-            const selectionRange = documentContext.nodeManager.getNodeRange(property.keyNode);
-            const children: DocumentSymbol[] = [];
-            const symbol: DocumentSymbol = {
-              name: name,
-              kind: this.getSymbolKind(customSymbol),
-              range,
-              selectionRange,
-              children,
-              detail: this.getDetail(valueNode),
-            };
+          if (!property.valueNode) {
+            return;
+          }
+          const symbol = this.getPropertySymbol(property, documentContext);
+          if (symbol) {
             result.push(symbol);
-            toVisit.push({ result: children, node: valueNode });
+            toVisit.push({ result: symbol.children!, node: property.valueNode });
           }
         });
       }
@@ -80,7 +58,13 @@ export class SymbolsProviderBase implements SymbolsProvider {
     return result;
   }
 
-  protected isStepProperty(property: PropertyASTNode | undefined): boolean {
+  /**
+   * Determines if the given property is a step property.
+   * Inputs and outputs are also considered step properties.
+   * @param property The property to check.
+   * @returns True if the property is a step property, false otherwise.
+   */
+  protected isStepProperty(property: PropertyASTNode): boolean {
     // The direct parent is the object containing this property and we want
     // to check the "steps" property which is the parent of that object
     const grandParent = property?.parent?.parent;
@@ -91,6 +75,51 @@ export class SymbolsProviderBase implements SymbolsProvider {
     return false;
   }
 
+  protected getPropertySymbol(property: PropertyASTNode, documentContext: DocumentContext): DocumentSymbol | undefined {
+    const name = this.getSymbolName(property);
+    if (this.symbolNamesToIgnore.has(name)) {
+      return;
+    }
+    const symbolName = this.isStepProperty(property) ? "step" : name;
+    const range = documentContext.nodeManager.getNodeRange(property);
+    const selectionRange = documentContext.nodeManager.getNodeRange(property.keyNode);
+    const children: DocumentSymbol[] = [];
+    const symbol: DocumentSymbol = {
+      name: name,
+      kind: this.getSymbolKind(symbolName),
+      range,
+      selectionRange,
+      children,
+      detail: this.getDetail(property.valueNode),
+    };
+    return symbol;
+  }
+
+  protected getArrayItemSymbol(
+    node: ASTNode,
+    index: number,
+    documentContext: DocumentContext
+  ): DocumentSymbol | undefined {
+    const name = this.getArrayNodeName(node, index);
+    if (this.symbolNamesToIgnore.has(name)) {
+      return;
+    }
+    const range = documentContext.nodeManager.getNodeRange(node);
+    const selectionRange = range;
+    const symbol: DocumentSymbol = {
+      name,
+      kind: this.getSymbolKind(node.type),
+      range,
+      selectionRange,
+      children: [],
+    };
+    return symbol;
+  }
+
+  protected getSymbolName(property: PropertyASTNode): string {
+    return this.getKeyLabel(property) ?? this.getNodeName(property.valueNode);
+  }
+
   protected getSymbolKind(nodeType: string): SymbolKind {
     switch (nodeType) {
       case "step":
@@ -141,7 +170,8 @@ export class SymbolsProviderBase implements SymbolsProvider {
   }
 
   protected getArrayNodeName(node: ASTNode | undefined, index: number): string {
-    return this.getNodeName(node) ?? String(index);
+    const nodeName = this.getNodeName(node);
+    return nodeName || String(index + 1);
   }
 
   protected getPropertyValueAsString(node: ObjectASTNode, propertyName: string): string | undefined {