Skip to content

Conversation

@YuichiNukiyama
Copy link
Contributor

When Both the var and the interface , go to definition isn't work.
To resolve this issue, if variabe declaration was found, only it added to declaration array.

Fixes #1932

When Both the var and the interface , go to definition isn't work.
To resolve this issue, if variabe declaration was found, only it added
to declaration array.
@RyanCavanaugh
Copy link
Member

CI build failing due to linter errors on bad whitespace

if (!tryAddConstructSignature(symbol, node, symbolKind, symbolName, containerName, result) &&
!tryAddCallSignature(symbol, node, symbolKind, symbolName, containerName, result)) {
!tryAddCallSignature(symbol, node, symbolKind, symbolName, containerName, result) &&
!tryAddVariableSignature(declarations, node, symbolKind, symbolName, containerName, result)) {
Copy link
Member

Choose a reason for hiding this comment

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

tryAddDeclarationsWithSignatures

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did it.

forEach(signatureDeclarations, d => {
if ((selectConstructors && d.kind === SyntaxKind.Constructor) ||
(!selectConstructors && (d.kind === SyntaxKind.FunctionDeclaration || d.kind === SyntaxKind.MethodDeclaration || d.kind === SyntaxKind.MethodSignature))) {
(!selectConstructors && (d.kind === SyntaxKind.FunctionDeclaration || d.kind === SyntaxKind.MethodDeclaration || d.kind === SyntaxKind.MethodSignature || d.kind === SyntaxKind.VariableDeclaration))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i do not think this is the correct fix. this function tries to get you on the implementation signature or the last one if no implementation was found. this is special to signatures, this issue is about filtering the declaration location based on the reference location kind. i.e.:

var foo;   // value
type foo = any;  // type
namespace foo {} // namespace

+foo; // should only see the value (i.e var foo;)

var x: foo; // should only see the type (i.e type foo = any;)

foo.blah; // should see both namespace and value (i.e var foo, and namespace foo).

what we need to do is filter the declarations based on the reference node.

we have two functions that do that already, 1. getMeaningFromLocation see https://github.com/Microsoft/TypeScript/blob/master/src/services/services.ts#L6497, this tells you which one of value, type, or namespace the reference node is, and 2. getMeaningFromDeclaration see https://github.com/Microsoft/TypeScript/blob/master/src/services/services.ts#L6368, this tells you what is the meaning of a declaration.
the declaration location we add should be the ones the match:
symbol.declarations.filter(d => getMeaningFromDeclaration(d) & getMeaningFromLocation(node) !== 0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhegazy Thank you for your advise. I try to fix it.

@RyanCavanaugh
Copy link
Member

👍

let definition: Declaration;

forEach(signatureDeclarations, d => {
if ((selectConstructors && d.kind === SyntaxKind.Constructor) ||
Copy link
Contributor

@mhegazy mhegazy Apr 14, 2016

Choose a reason for hiding this comment

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

why not just put this in the main loop in getDefinitionFromSymbol:

            if (!tryAddConstructSignature(symbol, node, symbolKind, symbolName, containerName, result) &&
                 !tryAddCallSignature(symbol, node, symbolKind, symbolName, containerName, result)) {
                 // Just add all the declarations.
                 const searcMeaning = getMeaningFromLocation(node);
                 forEach(declarations, declaration => {
                     if (getMeaningFromDeclaration(declaration) & searcMeaning) {
                         result.push(createDefinitionInfo(declaration, symbolKind, symbolName, containerName));
                     }
                 });
             }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I checked types in the main loop, following tests were failed.
And I think shouldn't change these tests.

@YuichiNukiyama
Copy link
Contributor Author

This PR is too old. I close this one.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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.

Go to definition includes results from other entity spaces

5 participants