microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.5k stars 12.33k forks source link

Go-to-definition should only show relevant definitions when value & type have same name #57959

Open ulugbekna opened 4 months ago

ulugbekna commented 4 months ago

πŸ”Ž Search Terms

"go to definition", "same identifier", "value", "type"

πŸ•— Version & Regression Information

Always was the case, AFAIK

⏯ Playground Link

https://www.typescriptlang.org/play?#code/C4TwDgpgBMULxQN4F8oFgBQmDGB7AdgM6ywIrpYYBmArvtsAJYFRUAUAhgE4DmAXDACUSKKgD0YqD1yN8PGLigATCFVmMmLFgANg2qAAsIXCIGgCKYwBuEQstXrNRKLipQARrmAGoAWhjhoDnwlKEsOABsaaF1tABooCGt8GANcGh5vMMjovShsIPxPd2gaQggQ2RTGWzBwjmwIAWBCbF8Eri5cLigAFQBlACYAdgAWAE4BAHJgSagTKmNbYEUOUIio+LcaWGri2XlS8qgOW1XQSENjCAA6KAARRhCQNKgAWwggqGmAlxhJgH5MJggA

πŸ’» Code

type t = {} 

const t = {} 

function f(arg: t) { } // going to definition on `t` here​ gives definitions of both - type and value `t`, even though value `t` cannot be used in this place: tsc - error TS2749: 't' refers to a value, but is being used as a type here. Did you mean 'typeof t'?

πŸ™ Actual behavior

"Go to definition" on a param type t shows 2 definitions - value & type which have same name - even though the value's definition is completely irrelevant.

πŸ™‚ Expected behavior

"Go to definition" shows only relevant definition of type t

Additional information about the issue

No response

ulugbekna commented 4 months ago

Hi @RyanCavanaugh

Thanks for fast triaging! :-)

Wouldn't it count as a bug given value t is not the definition of type t ?

RyanCavanaugh commented 4 months ago

People complain a lot if the LS doesn't show a reference they think "should" count, but it's generally understood that a human is reading this list and can decide if the found reference is one they care about or not.

fatcerberus commented 4 months ago

To be specific: the value t "could" count in the case where the code is attempting to emulate a class structure manually (i.e. type t is the class instance type, const t is the static part of the class).

ulugbekna commented 3 months ago

Another example when going to definition should not show the type but it does:

image

https://www.typescriptlang.org/play?ts=5.5.0-dev.20240409&ssl=12&ssc=1&pln=1&pc=1#code/C4TwDgpgBMULxQN4FgBQUNQGYHscC4oA7AVwFsAjCAJwG40BfKNNAYxyIGdZYEV1MWIoQAUASngA+JA0bNUaLCSKtgASw7ZxSNJhgA6IePqpZC1EA

ulugbekna commented 3 months ago

People complain a lot if the LS doesn't show a reference they think "should" count, but it's generally understood that a human is reading this list and can decide if the found reference is one they care about or not.

I see, maybe having a setting or running an experiment could help with this? It seems unfortunate that ts-server has to provide incorrect results because of some people's preferences

snarbies commented 3 months ago

This sounds like a "cure worse than the disease" type situation.

To be specific: the value t "could" count in the case where the code is attempting to emulate a class structure manually (i.e. type t is the class instance type, const t is the static part of the class).

I couldn't even tell you the number of times I've exported both a constant and a type with the same name because they have a relationship along these lines. Factory function and instance type, interface paired with Math-like utility object, etc.

How frequent and how problematic is the issue of pulling up a type whose name is the same as an unrelated value. I'd expect it to be both rare and inconsequential.