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

feat(typescript_indexer): implement ref inlining for imported symbols #5527

Merged
merged 9 commits into from
Mar 7, 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ MANIFEST-*
.vscode
.clangd
.cache
.idea

# Binary files
*.class
Expand Down
82 changes: 59 additions & 23 deletions kythe/typescript/indexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ export interface IndexingOptions {
* is used.
*/
readFile?: (path: string) => Buffer;

/**
* Enables post processing that "inlines" refs to imported symbols to point instead
* to the original definition.
*/
enableImportsEdgeReassignment?: boolean;
}

/**
Expand Down Expand Up @@ -245,6 +251,14 @@ function todo(sourceRoot: string, node: ts.Node, message: string) {
console.warn(`TODO: ${file}:${line}:${character}: ${message}`);
}

/**
* Convert VName to a string that can be used as key in Maps.
*/
function vnameToString(vname: VName): string {
return `(${vname.corpus},${vname.language},${vname.path},${vname.root},${
vname.signature})`;
}

type NamespaceAndContext = string&{__brand: 'nsctx'};
/**
* A SymbolVNameStore stores a mapping of symbols to the (many) VNames it may
Expand Down Expand Up @@ -793,6 +807,18 @@ class Visitor {
/** Cached anchor nodes. Signature is used as key. */
private readonly anchors = new Map<string, VName>();

/**
* Mapping from imported values to their remote definitions. For cases like
*
* import {Foo} from './dep';
* new Foo();
*
* We don't produce a defition to Foo symbol. Instead all refs ot it will
* point to the remote Foo from dep.ts. This map contains mapping from
* VName of local Foo to the VName of the remote Foo.
*/
private readonly localSymbolToRemoteReassignMap = new Map<string, VName>();

constructor(
private readonly host: IndexerHost,
private file: ts.SourceFile,
Expand Down Expand Up @@ -860,6 +886,7 @@ class Visitor {

/** emitEdge emits a new edge entry, relating two VNames. */
emitEdge(source: VName, kind: EdgeKind|OrdinalEdge, target: VName) {
target = this.localSymbolToRemoteReassignMap.get(vnameToString(target)) ?? target;
this.host.options.emit({
source,
edge_kind: kind,
Expand Down Expand Up @@ -1288,11 +1315,13 @@ class Visitor {
*
* @param name TypeScript import declaration node
* @param bindingAnchor anchor that "defines/binding" the local import
* definition
* definition. If the bindingAnchor is null then the local definition
* won't be created and refs to the local definition will be reassigned
* to the remote definition.
* @param refAnchor anchor that "ref" the import's remote declaration
*/
visitImport(
name: ts.Node, bindingAnchor: Readonly<VName>,
name: ts.Node, bindingAnchor: Readonly<VName>|null,
refAnchor: Readonly<VName>) {
// An import both aliases a symbol from another module
// (call it the remote symbol) and it defines a local symbol.
Expand Down Expand Up @@ -1325,31 +1354,35 @@ class Visitor {
const kLocalValue = this.host.getSymbolName(localSym, TSNamespace.VALUE);
if (!kRemoteValue || !kLocalValue) return;

// The local import value is a "variable" with an "import" subkind, and
// aliases its remote definition.
this.emitNode(kLocalValue, NodeKind.VARIABLE);
this.emitFact(kLocalValue, FactName.SUBKIND, Subkind.IMPORT);
this.emitEdge(kLocalValue, EdgeKind.ALIASES, kRemoteValue);

// Emit edges from the binding and referencing anchors to the import's
// local and remote definition, respectively.
this.emitEdge(bindingAnchor, EdgeKind.DEFINES_BINDING, kLocalValue);
if (bindingAnchor == null) {
this.localSymbolToRemoteReassignMap.set(vnameToString(kLocalValue), kRemoteValue);
} else {
// The local import value is a "variable" with an "import" subkind, and
// aliases its remote definition.
this.emitNode(kLocalValue, NodeKind.VARIABLE);
this.emitFact(kLocalValue, FactName.SUBKIND, Subkind.IMPORT);
this.emitEdge(kLocalValue, EdgeKind.ALIASES, kRemoteValue);
this.emitEdge(bindingAnchor, EdgeKind.DEFINES_BINDING, kLocalValue);
}
// Emit edges from the referencing anchor to the import's remote definition.
this.emitEdge(refAnchor, EdgeKind.REF_IMPORTS, kRemoteValue);
}
if (remoteSym.flags & ts.SymbolFlags.Type) {
const kRemoteType = this.host.getSymbolName(remoteSym, TSNamespace.TYPE);
const kLocalType = this.host.getSymbolName(localSym, TSNamespace.TYPE);
if (!kRemoteType || !kLocalType) return;

// The local import value is a "talias" (type alias) with an "import"
// subkind, and aliases its remote definition.
this.emitNode(kLocalType, NodeKind.TALIAS);
this.emitFact(kLocalType, FactName.SUBKIND, Subkind.IMPORT);
this.emitEdge(kLocalType, EdgeKind.ALIASES, kRemoteType);

// Emit edges from the binding and referencing anchors to the import's
// local and remote definition, respectively.
this.emitEdge(bindingAnchor, EdgeKind.DEFINES_BINDING, kLocalType);
if (bindingAnchor == null) {
this.localSymbolToRemoteReassignMap.set(vnameToString(kLocalType), kRemoteType);
} else {
// The local import value is a "talias" (type alias) with an "import"
// subkind, and aliases its remote definition.
this.emitNode(kLocalType, NodeKind.TALIAS);
this.emitFact(kLocalType, FactName.SUBKIND, Subkind.IMPORT);
this.emitEdge(kLocalType, EdgeKind.ALIASES, kRemoteType);
this.emitEdge(bindingAnchor, EdgeKind.DEFINES_BINDING, kLocalType);
}
// Emit edges from the referencing anchor to the import's remote definition.
this.emitEdge(refAnchor, EdgeKind.REF_IMPORTS, kRemoteType);
}
}
Expand Down Expand Up @@ -1438,13 +1471,15 @@ class Visitor {
const importTextAnchor =
this.newAnchor(decl, importTextSpan.start, importTextSpan.end);

const enableReassignment = this.host.options.enableImportsEdgeReassignment;
if (ts.isImportEqualsDeclaration(decl)) {
// This is an equals import, e.g.:
// import foo = require('./bar');
//
// TODO(#4021): Bind the local definition and reference the remote
// definition on the import name.
this.visitImport(decl.name, importTextAnchor, this.newAnchor(decl.name));
const refAnchor = this.newAnchor(decl.name)
this.visitImport(decl.name, enableReassignment ? null : importTextAnchor, refAnchor);
return;
}

Expand All @@ -1461,8 +1496,9 @@ class Visitor {
//
// TODO(#4021): Bind the local definition and reference the remote
// definition on the import name.
const refAnchor = this.newAnchor(clause.name);
this.visitImport(
clause.name, importTextAnchor, this.newAnchor(clause.name));
clause.name, enableReassignment ? null : importTextAnchor, refAnchor);
return;
}

Expand Down Expand Up @@ -1507,8 +1543,8 @@ class Visitor {
bindingAnchor = this.newAnchor(imp.name);
refAnchor = this.newAnchor(imp.propertyName);
} else {
bindingAnchor = importTextAnchor;
refAnchor = this.newAnchor(imp.name);
bindingAnchor = enableReassignment ? null : importTextAnchor;
}
this.visitImport(imp.name, bindingAnchor, refAnchor);
}
Expand Down
2 changes: 2 additions & 0 deletions kythe/typescript/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ function verify(
for (const file of testFiles) {
fileVNames.set(file, {...rootVName, path: file});
}
const enableEdgeReassignment = testCase.name.includes('enable_edge_reassignment');

try {
const compilationUnit: indexer.CompilationUnit = {
Expand All @@ -124,6 +125,7 @@ function verify(
verifier.stdin.write(JSON.stringify(obj) + '\n');
},
plugins,
enableImportsEdgeReassignment: enableEdgeReassignment,
});
} finally {
// Ensure we close stdin on the verifier even on crashes, or otherwise
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Syntax 1: exporting a value from another module.
//- @"'./module'" ref/imports Mod
export {value} from './module';

// Syntax 2: a bare "export" statement.
//- @local defines/binding Local
const local = 3;
//- @local ref Local
export {local};
//- @local ref Local
//- @aliasedLocal ref Local
export {local as aliasedLocal};
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// This test imports the same value in a variety of names and ensures
// they all resolve to the same VName.

//- @"'./module'" ref/imports Mod
import './module';

//- @mod_imp defines/binding ModLocal
//- @"'./module'" ref/imports Mod
import * as mod_imp from './module';

// Importing from a module gets a VName that refers into the other module.
//- @value ref/imports Val
//- @"'./module'" ref/imports Mod
import {value} from './module';

// Importing from a module gets a VName that refers into the other module,.
//- @value ref/imports Val
//- @renamedValue defines/binding RenamedValue
//- RenamedValue.node/kind variable
//- RenamedValue.subkind import
//- RenamedValue aliases Val
import {value as renamedValue} from './module';

// Ensure the various names of the imported value link together.

//- @value ref Val
value;

//- @mod_imp ref ModLocal
//- @value ref Val
mod_imp.value;

//- @renamedValue ref RenamedValue
renamedValue;

//- @value ref/imports Val
import {value as exportedValue} from './export';
//- @local ref/imports Local
import {local} from './export';
//- @aliasedLocal ref/imports Local
import {aliasedLocal} from './export';

// Importing a type from another module.
//- @MyType ref/imports MyType
import {MyType} from './module';
//- @MyType ref MyType
let x: MyType;
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// All modules define a 'module' record representing the file as a module.
// The signature is 'module' and the path is the module path (the filename
// without an extension). See the discussion of "Module name" in README.md.

//- Mod=vname("module", _, _, "testdata/imports_enable_edge_reassignment_group/module", _).node/kind record

// The first letter in the module is tagged as defining the module.
// See discussion in emitModuleAnchor().
//- ModAnchor.node/kind anchor
//- ModAnchor./kythe/loc/start 0
//- ModAnchor./kythe/loc/end 1
//- ModAnchor defines/binding Mod

//- @value defines/binding Val
export let value = 3;

//- @MyType defines/binding MyType
export type MyType = string;