Repository: kythe/kythe
Pull request #5795 (2023-08-19 01:08:24)

fix(typescript_indexer): improve handling of anonymous functions
This PR improves function that returns TS Symbol for Node. Previously it handled only named Nodes such as functions with names. Now we can get Symbols of anonymous functions as well. This fixes certain aspects of anonymous functions such as adds missing `childof` edges.  And additionally it brings close handling of named and anonymous entities. 

Unfortunately it relies on using private Node.symbol property. I considered alternative options such as:

1. Support two different branches: named nodes with Symbol and anonymous nodes without Symbols. That would require re-implementing a [SymbolVNameStore](https://github.com/kythe/kythe/blob/a4e2b7822d4857d26a87341b5b4c3a014b74cc30/kythe/typescript/indexer.ts#L112C5-L112C5) cache to deduplicate vnames for anonymous nodes. Keeping both in sync would be challenging and confusing.

2. For anonymous nodes, where we can't get Symbols using public API, create our own Symbol instance. Symbol is public interface so we can create objects that implement it. Would also need to introduce Node=>FakeSymbol cache. 

Both approaches seem more complicated. So I decided to use private `Node.symbol` API. If future TS versions remove it - we'll have to switch to one of the alternative approaches.


Patch:
diff --git a/kythe/typescript/indexer.ts b/kythe/typescript/indexer.ts
index d66009d5b..a7ad41674 100644
--- a/kythe/typescript/indexer.ts
+++ b/kythe/typescript/indexer.ts
@@ -216,11 +216,32 @@ class StandardIndexerContext implements IndexerHost {
   }
 
   getSymbolAtLocation(node: ts.Node): ts.Symbol|undefined {
-    return this.typeChecker.getSymbolAtLocation(node);
+    // Practically any interesting node has a Symbol: variables, classes, functions.
+    // Both named and anonymous have Symbols. We tie Symbols to Vnames so its
+    // important to get Symbol object for as many nodes as possible. Unfortunately
+    // Typescript doesn't provide good API for extracting Symbol from Nodes.
+    // It is supported well for named nodes, probably logic being that if you can't
+    // refer to a node then no need to have Symbol. But for Kythe we need to handle
+    // anonymous nodes as well. So we do hacks here.
+    // See similar bugs that haven't been resolved properly:
+    // https://github.com/microsoft/TypeScript/issues/26511
+    //
+    // Open FR: https://github.com/microsoft/TypeScript/issues/55433
+    let sym = this.typeChecker.getSymbolAtLocation(node);
+    if (sym) return sym;
+    // Check if it's named node.
+    if ('name' in node) {
+      sym = this.typeChecker.getSymbolAtLocation((node as ts.NamedDeclaration).name!);
+      if (sym) return sym;
+    }
+    // Sad hack. Nodes have symbol property but it's not exposed in the API.
+    // We could create our own Symbol instance to avoid depending on non-public API.
+    // But it's not clear whether it will be less maintainance.
+    return (node as any).symbol;
   }
 
   getSymbolAtLocationFollowingAliases(node: ts.Node): ts.Symbol|undefined {
-    let sym = this.typeChecker.getSymbolAtLocation(node);
+    let sym = this.getSymbolAtLocation(node);
     while (sym && (sym.flags & ts.SymbolFlags.Alias) > 0) {
       // a hack to prevent following aliases in cases like:
       // import * as fooNamespace from './foo';
@@ -846,7 +867,13 @@ class Visitor {
       this.visitDynamicImportCall(node);
       return;
     }
-    const symbol = this.host.getSymbolAtLocationFollowingAliases(node.expression);
+    // Handle case of immediately-invoked functions like:
+    // (() => do stuff... )();
+    let expression: ts.Node = node.expression;
+    while (ts.isParenthesizedExpression(expression)) {
+      expression = expression.expression;
+    }
+    const symbol = this.host.getSymbolAtLocationFollowingAliases(expression);
     if (!symbol) {
       return;
     }
@@ -1106,16 +1133,12 @@ class Visitor {
     } else if (ts.isSetAccessor(node)) {
       context = Context.Setter;
     }
-    if (node.name) {
-      const sym = this.host.getSymbolAtLocationFollowingAliases(node.name);
-      if (!sym) {
-        return {};
-      }
-      const vname = this.host.getSymbolName(sym, TSNamespace.VALUE, context);
-      return {sym, vname};
-    } else {
-      return {vname: this.host.scopedSignature(node)};
+    const sym = this.host.getSymbolAtLocationFollowingAliases(node);
+    if (!sym) {
+      return {};
     }
+    const vname = this.host.getSymbolName(sym, TSNamespace.VALUE, context);
+    return {sym, vname};
   }
 
   /**
@@ -1136,6 +1159,7 @@ class Visitor {
     for (; node.kind !== ts.SyntaxKind.SourceFile; node = node.parent) {
       const kind = node.kind;
       if (kind === ts.SyntaxKind.FunctionDeclaration ||
+          kind === ts.SyntaxKind.FunctionExpression ||
           kind === ts.SyntaxKind.ArrowFunction ||
           kind === ts.SyntaxKind.MethodDeclaration ||
           kind === ts.SyntaxKind.Constructor ||
@@ -1416,7 +1440,7 @@ class Visitor {
   emitModuleAnchor(sf: ts.SourceFile) {
     const kMod =
         this.newVName('module', this.host.moduleName(this.file.fileName));
-    this.emitFact(kMod, FactName.NODE_KIND, 'record');
+    this.emitFact(kMod, FactName.NODE_KIND, NodeKind.RECORD);
     this.emitEdge(this.kFile, EdgeKind.CHILD_OF, kMod);
 
     // Emit the anchor, bound to the beginning of the file.
@@ -2648,7 +2672,7 @@ class Visitor {
 
   /** index is the main entry point, starting the recursive visit. */
   index() {
-    this.emitFact(this.kFile, FactName.NODE_KIND, 'file');
+    this.emitFact(this.kFile, FactName.NODE_KIND, NodeKind.FILE);
     this.emitFact(this.kFile, FactName.TEXT, this.file.text);
 
     this.emitModuleAnchor(this.file);
@@ -2656,7 +2680,7 @@ class Visitor {
     // Emit file-level init function to contain all call anchors that
     // don't have parent functions.
     const fileInitFunc = this.getSyntheticFileInitVName();
-    this.emitFact(fileInitFunc, FactName.NODE_KIND, 'function');
+    this.emitFact(fileInitFunc, FactName.NODE_KIND, NodeKind.FUNCTION);
     this.emitEdge(
         this.newAnchor(this.file, 0, 0), EdgeKind.DEFINES, fileInitFunc);
 


Test patch:
diff --git a/kythe/typescript/testdata/function.ts b/kythe/typescript/testdata/function.ts
index 6db973a63..5941a13dd 100644
--- a/kythe/typescript/testdata/function.ts
+++ b/kythe/typescript/testdata/function.ts
@@ -86,7 +86,7 @@ function bindingTest({aa, bb: {cc, dd}}, [ee, [ff]]: NestedArray) {
   dd = ff;
 }
 
-// Test function expressions introducing an anonymous scope
+// Test function expressions introducing an Anon scope
 //- @ev defines/binding Ev
 //- @an defines/binding An
 let ev, an;
@@ -95,4 +95,18 @@ let ev, an;
 let ev;
 //- @an ref An
 an;
-})()
+})();
+
+//- AnonArrowDef defines _
+//- AnonArrowDef.node/kind anchor
+//- AnonArrowDef.loc/start @^"arg"
+//- AnonArrowDef.loc/end @$"42"
+(arg => 42)();
+
+//- AnonFunctionDef defines _
+//- AnonFunctionDef.node/kind anchor
+//- AnonFunctionDef.loc/start @^"function"
+(function() {
+  return 42;
+//- AnonFunctionDef.loc/end @$"}"
+})();
diff --git a/kythe/typescript/testdata/refcall.ts b/kythe/typescript/testdata/refcall.ts
index 3f7da4ec1..a94d19511 100644
--- a/kythe/typescript/testdata/refcall.ts
+++ b/kythe/typescript/testdata/refcall.ts
@@ -44,3 +44,17 @@ function doNothing() {
 //- DoNothingCall=@"doNothing()" ref/call DoNothing
 //- DoNothingCall childof FileInitFunc
 doNothing();
+
+//- GetAreaCallInAnonArrow=@"square.getArea()" ref/call GetArea
+//- GetAreaCallInAnonArrow childof AnonArrow
+//- AnonArrowCall ref/call AnonArrow
+//- AnonArrowCall childof FileInitFunc
+(() => square.getArea())();
+
+//- AnonFunctionCall ref/call AnonFunction
+//- AnonFunctionCall childof FileInitFunc
+(function() {
+  //- GetAreaCallInAnonFunction=@"square.getArea()" ref/call GetArea
+  //- GetAreaCallInAnonFunction childof AnonFunction
+  square.getArea();
+})();