Skip to content

Allow to find all references for constructors #10540

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
12 commits merged into from
Sep 1, 2016
Merged

Allow to find all references for constructors #10540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3f126a5
Allow to find all references for constructors
Aug 24, 2016
c1a291d
Get the type of a constructor as the type of the class
Aug 26, 2016
f90d8dd
Reuse code for tryGetClassExtendingIdentifier
Aug 26, 2016
7e6f18d
Don't use constructor symbol for search -- use class symbol and filte…
Aug 26, 2016
0dc976d
Remove unused parameter
Aug 26, 2016
4514f8f
Make goto-definition go to a signature declaration if possible
Aug 29, 2016
5977473
Support decorators and templates
Aug 30, 2016
efc7e9d
Climb past multiple property accesses if necessary
Aug 30, 2016
e8e7ec6
Remember to check for existence of `target.parent`
Sep 1, 2016
3eadbf6
Rename function
Sep 1, 2016
d6d6a4a
Merge branch 'goto_definition_super', remote-tracking branch 'origin'…
Sep 1, 2016
ab75365
Respond to PR comments
Sep 1, 2016
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
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8758,7 +8758,7 @@ namespace ts {
// The location isn't a reference to the given symbol, meaning we're being asked
// a hypothetical question of what type the symbol would have if there was a reference
// to it at the given location. Since we have no control flow information for the
// hypotherical reference (control flow information is created and attached by the
// hypothetical reference (control flow information is created and attached by the
// binder), we simply return the declared type of the symbol.
return getTypeOfSymbol(symbol);
}
Expand Down
1 change: 0 additions & 1 deletion src/compiler/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1515,5 +1515,4 @@ namespace ts {
? ((fileName) => fileName)
: ((fileName) => fileName.toLowerCase());
}

}
25 changes: 22 additions & 3 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,18 @@ namespace ts {
return undefined;
}

export function isCallLikeExpression(node: Node): node is CallLikeExpression {
switch (node.kind) {
case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
case SyntaxKind.TaggedTemplateExpression:
case SyntaxKind.Decorator:
return true;
default:
return false;
}
}

export function getInvokedExpression(node: CallLikeExpression): Expression {
if (node.kind === SyntaxKind.TaggedTemplateExpression) {
return (<TaggedTemplateExpression>node).tag;
Expand Down Expand Up @@ -2659,10 +2671,17 @@ namespace ts {
return token >= SyntaxKind.FirstAssignment && token <= SyntaxKind.LastAssignment;
}

export function isExpressionWithTypeArgumentsInClassExtendsClause(node: Node): boolean {
return node.kind === SyntaxKind.ExpressionWithTypeArguments &&
/** Get `C` given `N` if `N` is in the position `class C extends N` where `N` is an ExpressionWithTypeArguments. */
export function tryGetClassExtendingExpressionWithTypeArguments(node: Node): ClassLikeDeclaration | undefined {
if (node.kind === SyntaxKind.ExpressionWithTypeArguments &&
(<HeritageClause>node.parent).token === SyntaxKind.ExtendsKeyword &&
isClassLike(node.parent.parent);
isClassLike(node.parent.parent)) {
return node.parent.parent;
}
}

export function isExpressionWithTypeArgumentsInClassExtendsClause(node: Node): boolean {
return tryGetClassExtendingExpressionWithTypeArguments(node) !== undefined;
}

export function isEntityNameExpression(node: Expression): node is EntityNameExpression {
Expand Down
6 changes: 3 additions & 3 deletions src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1564,7 +1564,7 @@ namespace FourSlash {
public goToDefinition(definitionIndex: number) {
const definitions = this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition);
if (!definitions || !definitions.length) {
this.raiseError("goToDefinition failed - expected to at least one definition location but got 0");
this.raiseError("goToDefinition failed - expected to find at least one definition location but got 0");
}

if (definitionIndex >= definitions.length) {
Expand All @@ -1579,7 +1579,7 @@ namespace FourSlash {
public goToTypeDefinition(definitionIndex: number) {
const definitions = this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition);
if (!definitions || !definitions.length) {
this.raiseError("goToTypeDefinition failed - expected to at least one definition location but got 0");
this.raiseError("goToTypeDefinition failed - expected to find at least one definition location but got 0");
}

if (definitionIndex >= definitions.length) {
Expand All @@ -1600,7 +1600,7 @@ namespace FourSlash {
this.raiseError(`goToDefinition - expected to 0 definition locations but got ${definitions.length}`);
}
else if (!foundDefinitions && !negative) {
this.raiseError("goToDefinition - expected to at least one definition location but got 0");
this.raiseError("goToDefinition - expected to find at least one definition location but got 0");
}
}

Expand Down
179 changes: 160 additions & 19 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2792,18 +2792,42 @@ namespace ts {
return node && node.parent && node.parent.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>node.parent).name === node;
}

function climbPastPropertyAccess(node: Node) {
return isRightSideOfPropertyAccess(node) ? node.parent : node;
}

/** Get `C` given `N` if `N` is in the position `class C extends N` or `class C extends foo.N` where `N` is an identifier. */
function tryGetClassByExtendingIdentifier(node: Node): ClassLikeDeclaration | undefined {
return tryGetClassExtendingExpressionWithTypeArguments(climbPastPropertyAccess(node).parent);
}

function isCallExpressionTarget(node: Node): boolean {
if (isRightSideOfPropertyAccess(node)) {
node = node.parent;
}
return node && node.parent && node.parent.kind === SyntaxKind.CallExpression && (<CallExpression>node.parent).expression === node;
return isCallOrNewExpressionTarget(node, SyntaxKind.CallExpression);
}

function isNewExpressionTarget(node: Node): boolean {
if (isRightSideOfPropertyAccess(node)) {
node = node.parent;
}
return node && node.parent && node.parent.kind === SyntaxKind.NewExpression && (<CallExpression>node.parent).expression === node;
return isCallOrNewExpressionTarget(node, SyntaxKind.NewExpression);
}

function isCallOrNewExpressionTarget(node: Node, kind: SyntaxKind) {

@zhengbli zhengbli Sep 1, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kind: SyntaxKind.CallExpression | SyntaxKind.NewExpression

@ghost ghost Sep 1, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think this would require us to do a new LKG to get that feature.

const target = climbPastPropertyAccess(node);
return target && target.parent && target.parent.kind === kind && (<CallExpression>target.parent).expression === target;
}

function climbPastManyPropertyAccesses(node: Node): Node {
return isRightSideOfPropertyAccess(node) ? climbPastManyPropertyAccesses(node.parent) : node;
}

/** Returns a CallLikeExpression where `node` is the target being invoked. */
function getAncestorCallLikeExpression(node: Node): CallLikeExpression | undefined {
const target = climbPastManyPropertyAccesses(node);
const callLike = target.parent;
return callLike && isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike;
}

function tryGetSignatureDeclaration(typeChecker: TypeChecker, node: Node): SignatureDeclaration | undefined {
const callLike = getAncestorCallLikeExpression(node);
return callLike && typeChecker.getResolvedSignature(callLike).declaration;
}

function isNameOfModuleDeclaration(node: Node) {
Expand Down Expand Up @@ -4609,7 +4633,7 @@ namespace ts {
const symbolFlags = symbol.flags;
let symbolKind = getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(symbol, symbolFlags, location);
let hasAddedSymbolInfo: boolean;
const isThisExpression: boolean = location.kind === SyntaxKind.ThisKeyword && isExpression(location);
const isThisExpression = location.kind === SyntaxKind.ThisKeyword && isExpression(location);
let type: Type;

// Class at constructor site need to be shown as constructor apart from property,method, vars
Expand Down Expand Up @@ -5072,14 +5096,25 @@ namespace ts {
};
}

function getSymbolInfo(typeChecker: TypeChecker, symbol: Symbol, node: Node) {
return {
symbolName: typeChecker.symbolToString(symbol), // Do not get scoped name, just the name of the symbol
symbolKind: getSymbolKind(symbol, node),
containerName: symbol.parent ? typeChecker.symbolToString(symbol.parent, node) : ""
};
}

function createDefinitionFromSignatureDeclaration(decl: SignatureDeclaration): DefinitionInfo {
const typeChecker = program.getTypeChecker();
const { symbolName, symbolKind, containerName } = getSymbolInfo(typeChecker, decl.symbol, decl);
return createDefinitionInfo(decl, symbolKind, symbolName, containerName);
}

function getDefinitionFromSymbol(symbol: Symbol, node: Node): DefinitionInfo[] {
const typeChecker = program.getTypeChecker();
const result: DefinitionInfo[] = [];
const declarations = symbol.getDeclarations();
const symbolName = typeChecker.symbolToString(symbol); // Do not get scoped name, just the name of the symbol
const symbolKind = getSymbolKind(symbol, node);
const containerSymbol = symbol.parent;
const containerName = containerSymbol ? typeChecker.symbolToString(containerSymbol, node) : "";
const { symbolName, symbolKind, containerName } = getSymbolInfo(typeChecker, symbol, node);

if (!tryAddConstructSignature(symbol, node, symbolKind, symbolName, containerName, result) &&
!tryAddCallSignature(symbol, node, symbolKind, symbolName, containerName, result)) {
Expand Down Expand Up @@ -5205,6 +5240,12 @@ namespace ts {
}

const typeChecker = program.getTypeChecker();

const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node);
if (calledDeclaration) {
return [createDefinitionFromSignatureDeclaration(calledDeclaration)];
}

let symbol = typeChecker.getSymbolAtLocation(node);

// Could not find a symbol e.g. node is string or number keyword,
Expand Down Expand Up @@ -6012,6 +6053,7 @@ namespace ts {
case SyntaxKind.Identifier:
case SyntaxKind.ThisKeyword:
// case SyntaxKind.SuperKeyword: TODO:GH#9268
case SyntaxKind.ConstructorKeyword:
case SyntaxKind.StringLiteral:
return getReferencedSymbolsForNode(node, program.getSourceFiles(), findInStrings, findInComments);
}
Expand Down Expand Up @@ -6056,6 +6098,8 @@ namespace ts {
return getReferencesForSuperKeyword(node);
}

// `getSymbolAtLocation` normally returns the symbol of the class when given the constructor keyword,
// so we have to specify that we want the constructor symbol.
const symbol = typeChecker.getSymbolAtLocation(node);

if (!symbol && node.kind === SyntaxKind.StringLiteral) {
Expand Down Expand Up @@ -6130,7 +6174,7 @@ namespace ts {
};
}

function getAliasSymbolForPropertyNameSymbol(symbol: Symbol, location: Node): Symbol {
function getAliasSymbolForPropertyNameSymbol(symbol: Symbol, location: Node): Symbol | undefined {
if (symbol.flags & SymbolFlags.Alias) {
// Default import get alias
const defaultImport = getDeclarationOfKind(symbol, SyntaxKind.ImportClause);
Expand All @@ -6156,6 +6200,10 @@ namespace ts {
return undefined;
}

function followAliasIfNecessary(symbol: Symbol, location: Node): Symbol {
return getAliasSymbolForPropertyNameSymbol(symbol, location) || symbol;
}

function getPropertySymbolOfDestructuringAssignment(location: Node) {
return isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent) &&
typeChecker.getPropertySymbolOfDestructuringAssignment(<Identifier>location);
Expand Down Expand Up @@ -6420,7 +6468,8 @@ namespace ts {
if (referenceSymbol) {
const referenceSymbolDeclaration = referenceSymbol.valueDeclaration;
const shorthandValueSymbol = typeChecker.getShorthandAssignmentValueSymbol(referenceSymbolDeclaration);
const relatedSymbol = getRelatedSymbol(searchSymbols, referenceSymbol, referenceLocation);
const relatedSymbol = getRelatedSymbol(searchSymbols, referenceSymbol, referenceLocation,
/*searchLocationIsConstructor*/ searchLocation.kind === SyntaxKind.ConstructorKeyword);

if (relatedSymbol) {
const referencedSymbol = getReferencedSymbol(relatedSymbol);
Expand All @@ -6436,12 +6485,94 @@ namespace ts {
const referencedSymbol = getReferencedSymbol(shorthandValueSymbol);
referencedSymbol.references.push(getReferenceEntryFromNode(referenceSymbolDeclaration.name));
}
else if (searchLocation.kind === SyntaxKind.ConstructorKeyword) {
findAdditionalConstructorReferences(referenceSymbol, referenceLocation);
}
}
});
}

return;

/** Adds references when a constructor is used with `new this()` in its own class and `super()` calls in subclasses. */
function findAdditionalConstructorReferences(referenceSymbol: Symbol, referenceLocation: Node): void {

@mhegazy mhegazy Sep 1, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think a better approach here is to call the main routine getReferencesInNode with the class node, or the decedent class node, and look for the text "this" or "super". this way you do not need to have a different method to search by walking trees.

@mhegazy mhegazy Sep 1, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

come grab me tomorrow if you want to talk about this more.

Debug.assert(isClassLike(searchSymbol.valueDeclaration));

const referenceClass = referenceLocation.parent;
if (referenceSymbol === searchSymbol && isClassLike(referenceClass)) {
Debug.assert(referenceClass.name === referenceLocation);
// This is the class declaration containing the constructor.
addReferences(findOwnConstructorCalls(searchSymbol));
}
else {
// If this class appears in `extends C`, then the extending class' "super" calls are references.
const classExtending = tryGetClassByExtendingIdentifier(referenceLocation);
if (classExtending && isClassLike(classExtending) && followAliasIfNecessary(referenceSymbol, referenceLocation) === searchSymbol) {
addReferences(superConstructorAccesses(classExtending));
}
}
}

function addReferences(references: Node[]): void {
if (references.length) {
const referencedSymbol = getReferencedSymbol(searchSymbol);
addRange(referencedSymbol.references, map(references, getReferenceEntryFromNode));
}
}

/** `classSymbol` is the class where the constructor was defined.
* Reference the constructor and all calls to `new this()`.
*/
function findOwnConstructorCalls(classSymbol: Symbol): Node[] {
const result: Node[] = [];

for (const decl of classSymbol.members["__constructor"].declarations) {
Debug.assert(decl.kind === SyntaxKind.Constructor);
const ctrKeyword = decl.getChildAt(0);
Debug.assert(ctrKeyword.kind === SyntaxKind.ConstructorKeyword);
result.push(ctrKeyword);
}

forEachProperty(classSymbol.exports, member => {
const decl = member.valueDeclaration;
if (decl && decl.kind === SyntaxKind.MethodDeclaration) {
const body = (<MethodDeclaration>decl).body;
if (body) {
forEachDescendantOfKind(body, SyntaxKind.ThisKeyword, thisKeyword => {

@mhegazy mhegazy Sep 1, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you should not decent in nested classes or functions, as this would have a different meaning.

if (isNewExpressionTarget(thisKeyword)) {
result.push(thisKeyword);
}
});
}
}
});

return result;
}

/** Find references to `super` in the constructor of an extending class. */
function superConstructorAccesses(cls: ClassLikeDeclaration): Node[] {
const symbol = cls.symbol;
const ctr = symbol.members["__constructor"];
if (!ctr) {
return [];
}

const result: Node[] = [];
for (const decl of ctr.declarations) {
Debug.assert(decl.kind === SyntaxKind.Constructor);
const body = (<ConstructorDeclaration>decl).body;
if (body) {
forEachDescendantOfKind(body, SyntaxKind.SuperKeyword, node => {
if (isCallExpressionTarget(node)) {
result.push(node);
}
});
}
};
return result;
}

function getReferencedSymbol(symbol: Symbol): ReferencedSymbol {
const symbolId = getSymbolId(symbol);
let index = symbolToIndex[symbolId];
Expand Down Expand Up @@ -6822,16 +6953,17 @@ namespace ts {
}
}

function getRelatedSymbol(searchSymbols: Symbol[], referenceSymbol: Symbol, referenceLocation: Node): Symbol {
if (searchSymbols.indexOf(referenceSymbol) >= 0) {
return referenceSymbol;
function getRelatedSymbol(searchSymbols: Symbol[], referenceSymbol: Symbol, referenceLocation: Node, searchLocationIsConstructor: boolean): Symbol | undefined {
if (contains(searchSymbols, referenceSymbol)) {
// If we are searching for constructor uses, they must be 'new' expressions.
return (!searchLocationIsConstructor || isNewExpressionTarget(referenceLocation)) && referenceSymbol;
}

// If the reference symbol is an alias, check if what it is aliasing is one of the search
// symbols but by looking up for related symbol of this alias so it can handle multiple level of indirectness.
const aliasSymbol = getAliasSymbolForPropertyNameSymbol(referenceSymbol, referenceLocation);
if (aliasSymbol) {
return getRelatedSymbol(searchSymbols, aliasSymbol, referenceLocation);
return getRelatedSymbol(searchSymbols, aliasSymbol, referenceLocation, searchLocationIsConstructor);
}

// If the reference location is in an object literal, try to get the contextual type for the
Expand Down Expand Up @@ -8355,6 +8487,15 @@ namespace ts {
};
}

function forEachDescendantOfKind(node: Node, kind: SyntaxKind, action: (node: Node) => void) {
forEachChild(node, child => {
if (child.kind === kind) {
action(child);
}
forEachDescendantOfKind(child, kind, action);
});
}

/* @internal */
export function getNameTable(sourceFile: SourceFile): Map<number> {
if (!sourceFile.nameTable) {
Expand Down
Loading