Skip to content

use nameTable - services layer storage of identifiers in the file #1782

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.

Sign up for GitHub

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

Merged
vladima merged 3 commits into from
Jan 24, 2015
Merged

use nameTable - services layer storage of identifiers in the file #1782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
379d03b
use nameTable - services layer storage of identifiers in the file
vladima Jan 23, 2015
6040e55
add 'find all references' test
vladima Jan 23, 2015
da6070b
fixed typo in test name
vladima Jan 23, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 52 additions & 12 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ module ts {
isOpen: boolean;
version: string;
scriptSnapshot: IScriptSnapshot;

nameTable: Map<string>;
getNamedDeclarations(): Declaration[];
}

Expand Down Expand Up @@ -750,6 +750,7 @@ module ts {
public isOpen: boolean;
public languageVersion: ScriptTarget;
public identifiers: Map<string>;
public nameTable: Map<string>;

private namedDeclarations: Declaration[];

Expand Down Expand Up @@ -1546,6 +1547,8 @@ module ts {
export function createLanguageServiceSourceFile(filename: string, scriptSnapshot: IScriptSnapshot, scriptTarget: ScriptTarget, version: string, isOpen: boolean, setNodeParents: boolean): SourceFile {
var sourceFile = createSourceFile(filename, scriptSnapshot.getText(0, scriptSnapshot.getLength()), scriptTarget, setNodeParents);
setSourceFileFields(sourceFile, scriptSnapshot, version, isOpen);
// after full parsing we can use table with interned strings as name table
sourceFile.nameTable = sourceFile.identifiers;
return sourceFile;
}

Expand Down Expand Up @@ -1577,6 +1580,9 @@ module ts {
if (!disableIncrementalParsing) {
var newSourceFile = sourceFile.update(scriptSnapshot.getText(0, scriptSnapshot.getLength()), textChangeRange);
setSourceFileFields(newSourceFile, scriptSnapshot, version, isOpen);
// after incremental parsing nameTable might not be up-to-date
// drop it so it can be lazily recreated later
newSourceFile.nameTable = undefined;
return newSourceFile;
}
}
Expand Down Expand Up @@ -3235,7 +3241,7 @@ module ts {

if (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.SuperKeyword ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || isNameOfExternalModuleImportOrDeclaration(node)) {
return getReferencesForNode(node, [sourceFile], /*findInStrings:*/ false, /*findInComments:*/ false);
return getReferencesForNode(node, [sourceFile], /*searchOnlyInCurrentFile*/ true, /*findInStrings:*/ false, /*findInComments:*/ false);
}

switch (node.kind) {
Expand Down Expand Up @@ -3788,10 +3794,31 @@ module ts {
}

Debug.assert(node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.NumericLiteral || node.kind === SyntaxKind.StringLiteral);
return getReferencesForNode(node, program.getSourceFiles(), findInStrings, findInComments);
return getReferencesForNode(node, program.getSourceFiles(), /*searchOnlyInCurrentFile*/ false, findInStrings, findInComments);
}

function initializeNameTable(sourceFile: SourceFile): void {
var nameTable: Map<string> = {};

walk(sourceFile);
sourceFile.nameTable = nameTable;

function walk(node: Node) {
switch (node.kind) {
case SyntaxKind.Identifier:
nameTable[(<Identifier>node).text] = (<Identifier>node).text;
break;
case SyntaxKind.StringLiteral:
case SyntaxKind.NumericLiteral:
nameTable[(<LiteralExpression>node).text] = (<LiteralExpression>node).text;
break;
default:
forEachChild(node, walk);
Copy link
Contributor

@JsonFreeman JsonFreeman Jan 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it called visitNodes now? Is there a difference between forEachChild and visitNodes?

Copy link
Contributor Author

@vladima vladima Jan 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, forEachChild is still an entrypoint to a traversal routine. visitNodes and visitEachNode are just helpers that were lifted to a higher scope in order to avoid unnecesary allocations

}
}
}

function getReferencesForNode(node: Node, sourceFiles: SourceFile[], findInStrings: boolean, findInComments: boolean): ReferenceEntry[] {
function getReferencesForNode(node: Node, sourceFiles: SourceFile[], searchOnlyInCurrentFile: boolean, findInStrings: boolean, findInComments: boolean): ReferenceEntry[] {
// Labels
if (isLabelName(node)) {
if (isJumpStatementTarget(node)) {
Expand Down Expand Up @@ -3847,15 +3874,28 @@ module ts {
getReferencesInNode(scope, symbol, declaredName, node, searchMeaning, findInStrings, findInComments, result);
}
else {
var internedName = getInternedName(symbol, declarations)
forEach(sourceFiles, sourceFile => {
cancellationToken.throwIfCancellationRequested();
if (searchOnlyInCurrentFile) {
Debug.assert(sourceFiles.length === 1);
result = [];
getReferencesInNode(sourceFiles[0], symbol, declaredName, node, searchMeaning, findInStrings, findInComments, result);
}
else {
var internedName = getInternedName(symbol, declarations)
forEach(sourceFiles, sourceFile => {
cancellationToken.throwIfCancellationRequested();

if (lookUp(sourceFile.identifiers, internedName)) {
result = result || [];
getReferencesInNode(sourceFile, symbol, declaredName, node, searchMeaning, findInStrings, findInComments, result);
}
});
if (!sourceFile.nameTable) {
Copy link
Contributor

@mhegazy mhegazy Jan 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we turn this into a method getNameTable on SouceFileObject class?

Copy link
Contributor

@JsonFreeman JsonFreeman Jan 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

@vladima vladima Jan 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed offline: this property will be promoted to method later as a part of general effort to revise API

initializeNameTable(sourceFile)
}

Debug.assert(sourceFile.nameTable !== undefined);

if (lookUp(sourceFile.nameTable, internedName)) {
result = result || [];
getReferencesInNode(sourceFile, symbol, declaredName, node, searchMeaning, findInStrings, findInComments, result);
}
});
}
}

return result;
Expand Down
22 changes: 22 additions & 0 deletions tests/cases/fourslash/findReferencesAfterEdit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/// <reference path='fourslash.ts'/>

// @Filename: a.ts
////interface A {
//// foo: string;
////}

// @Filename: b.ts
///////<reference path='a.ts'/>
/////*0*/
////function foo(x: A) {
//// x.f/*1*/oo
////}

goTo.marker("1");
verify.referencesCountIs(2);

goTo.marker("0");
edit.insert("\r\n");

goTo.marker("1");
verify.referencesCountIs(2);
18 changes: 18 additions & 0 deletions tests/cases/fourslash/getOccurrencesAfterEdit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts'/>

/////*0*/
////interface A {
//// foo: string;
////}
////function foo(x: A) {
//// x.f/*1*/oo
////}

goTo.marker("1");
verify.occurrencesAtPositionCount(2);

goTo.marker("0");
edit.insert("\r\n");

goTo.marker("1");
verify.occurrencesAtPositionCount(2);