Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typescript_indexer): improve handling of anonymous functions #5795

Merged
merged 7 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 39 additions & 15 deletions kythe/typescript/indexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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};
}

/**
Expand All @@ -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 ||
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2648,15 +2672,15 @@ 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);

// 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);

Expand Down
18 changes: 16 additions & 2 deletions kythe/typescript/testdata/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 @$"}"
})();
14 changes: 14 additions & 0 deletions kythe/typescript/testdata/refcall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
})();