Skip to content

Make goto-definition go to a signature declaration if possible#10593

Merged
5 commits merged into
masterfrom
goto_definition_super
Sep 1, 2016
Merged

Make goto-definition go to a signature declaration if possible#10593
5 commits merged into
masterfrom
goto_definition_super

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Aug 29, 2016

Fixes #9269 and #10074

@ghost ghost force-pushed the goto_definition_super branch from 6f2567e to 4514f8f Compare August 29, 2016 18:41
Comment thread src/services/services.ts Outdated
return node && node.parent && node.parent.kind === SyntaxKind.NewExpression && (<CallExpression>node.parent).expression === node;
}

function getCallOrNewExpressionWorker(node: Node, kind: SyntaxKind): Node | undefined {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change the type of kind to SyntaxKind.CallExpression | SyntaxKind.NewExpression

Copy link
Copy Markdown
Author

@ghost ghost Aug 29, 2016

Choose a reason for hiding this comment

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

We'd need to do a new LKG before using enums-as-unions.

@DanielRosenwasser
Copy link
Copy Markdown
Member

For the record, calls and news are not the only things that undergo overload resolution. Tagged templates and decorators do as well.

If it's not too hard, it'd be ideal to keep the behavior consistent between them.

@DanielRosenwasser
Copy link
Copy Markdown
Member

Can you also add a test for going to definition on a call where overload resolution failed?

declare function doStuff(n: number): number;
declare function doStuff(s: string): string;

/**/doStuff(true);

Ideally, the language service should still jump to one of the overloads.

@ghost ghost assigned zhengbli Aug 30, 2016
Comment thread src/services/services.ts Outdated
function getAncestorCallLikeExpression(node: Node): CallLikeExpression | undefined {
const target = climbPastManyPropertyAccesses(node);
const callLike = target.parent;
return isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike;
Copy link
Copy Markdown

@zhengbli zhengbli Aug 30, 2016

Choose a reason for hiding this comment

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

Put callLike first to bail early

Comment thread src/services/services.ts
function getAncestorCallLikeExpression(node: Node): CallLikeExpression | undefined {
const target = climbPastManyPropertyAccesses(node);
const callLike = target.parent;
return callLike && isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems you are checking callLike twice

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.

First one is checking whether it exists. Last one is returning it from the function.

@zhengbli
Copy link
Copy Markdown

zhengbli commented Sep 1, 2016

Other than the comments, 👍

@ghost ghost merged commit 6ddcdcd into master Sep 1, 2016
@ghost ghost deleted the goto_definition_super branch September 1, 2016 19:57
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants