search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

Go to type definition should go to typedef #57013

Open FMorschel opened 2 weeks ago

FMorschel commented 2 weeks ago

Even after the fix for https://github.com/dart-lang/sdk/issues/56844 I'm still having an issue when working on the SDK. This is the link for the base change I made to see the problem https://github.com/FMorschel/sdk/tree/go-to-type-definition-inside-expanded.

Here is a repro of that part inside pkg/analysis_server_plugin/lib/src/correction/fix_in_file_processor.dart:

return [
  ...?registeredFixGenerators.nonLintProducers[errorCode],
  ...?registeredFixGenerators.nonLintMultiProducers[errorCode]
      ?.expand((multiProducer) {
    multiProducer;
    return [];
  }),
];

In here, with the latest SDK (got mine updated today actually to see if this was solved) I can't seem to activate Go to Type Definition on either multiProducer from the parameter or the one from the useless statement.

I can activate it with no problem on registeredFixGenerators or nonLintMultiProducers.

I'm still looking at this to try and find a better repro but in the meantime, I was instructed by @DanTup to create this issue so that others can take a look as well.

dart-github-bot commented 2 weeks ago

Summary: The "Go to Type Definition" functionality is not working for variables within a lambda function, specifically for multiProducer in the provided code snippet. This issue persists despite the fix for a previous issue related to type definition navigation.

FMorschel commented 2 weeks ago

I was thinking this would give me instances but it actually gives me constructors so because it is a Function there is no actual type here.

FMorschel commented 2 weeks ago

I was thinking of why I got confused there, and then I remembered that I only saw the type like a Function on the variable itself. The generics is tied to a typedef. I think in this case it could move us to the typedef definition (when it is defined by one, not when it matches). I'm not sure if that is possible but anyway, here is my proposal.

image

bwilkerson commented 2 weeks ago

I think what you're proposing is that "Go to type definition", at least for a function type, should navigate to the typedef, when there is one. If so, I think that's reasonable.

For consistency we should probably do that for any type that was defined using a typedef.

I'm not sure whether we have that information available, but I think we do at least some times.

DanTup commented 2 weeks ago

https://github.com/dart-lang/sdk/issues/39332 is a similar issue where showing the typedef is requested instead of the base type. I don't know if the cause or fixes are the same, but they might be.

FMorschel commented 2 weeks ago

I think I'll close this in favour of that one then since it is older and has more discussion. Thanks!

DanTup commented 2 weeks ago

(I'm not certain one fix will fix both, one is about hovers and one is about Type Def... but they're similar enough that the fix could be the same even if it has to be made in two places)

FMorschel commented 2 weeks ago

If needed we can always reopen this again, I'll keep that in mind though

FMorschel commented 1 week ago

I found some problems with argument_type_not_assignable and typedefs as well (https://github.com/dart-lang/sdk/issues/57056). I think I'll reopen this so we have more clarity of whats needs to be done.