Go-to-definition on return, yield, continue, break, and switch keywords

[srawlins]

Go-to-definition on any of these keywords should jump to the start of the function body (for return and yield), loop statement or switch (for continue and break).

This would be helpful in determining which function in a series of nested functions is associated with a return statement (etc.)

Maybe also else can jump to the if that declares it?

Typescript implemented something similar recently: https://devblogs.microsoft.com/typescript/announcing-typescript-4-9/#go-to-def-return-keywords

[jacob314]

Fyi @DanTup. It would also be interesting to think about how we can make jump to definition work better for some typical Flutter patterns as well. One thing I often find is I wish that go to definition for a Flutter widget within my package would jump to the build method of that widget rather than the class definition of that widget particularly for stateful widgets.

[bwilkerson]

I actually prefer a different affordance for this functionality. We have a 'mark occurrences' feature that will highlight all occurrences of a selected element in a file. In the legacy protocol we have that for 'return' that will highlight all of the places in a function that return from the function (and, I think, the return type of the function). I don't know whether it's possible to do this in LSP, but if it is it's much nicer than navigating because it doesn't move the cursor. We could do the same for 'yield', 'continue' and 'break' (ways to exit the loop) and for 'if', 'else if', 'else' sequences.

[srawlins]

Is that a distinct feature? Do we have to choose between go-to-definition and mark occurrences?

[bwilkerson]

Is that a distinct feature?

Yes, at least is it in the legacy protocol. Mark occurrences highlights multiple regions (usually background color) but doesn't move the cursor or scroll the editor.

Do we have to choose between go-to-definition and mark occurrences?

Do you mean, are they mutually exclusive? No, we could do both.

But the user doesn't get nearly as much information from navigation, and it seems a little weird to me to refer to the function as the "declaration" of the return statement, so go-to-declaration seems like an odd place to add this functionality. It might be convenient and might not confuse anyone, but I'm curious to know whether there have been any usability studies indicating that navigation is a good approach for solving this need.

[DanTup]

I actually prefer a different affordance for this functionality. We have a 'mark occurrences' feature that will highlight all occurrences of a selected element in a file. I don't know whether it's possible to do this in LSP, but if it is it's much nicer than navigating because it doesn't move the cursor.

Yep, LSP supports this and we actually already use it for occurrences of a variable. It doesn't seem to be working for return though so I'll take a look if something is missing there.

It also feels a little odd to me for Go-to-Definition to do this, but since it's not being used for anything else on those keywords I'm not sure if there are any drawbacks of doing so. I could imagine that once you learn it does this, it could be quite useful.

@jacob314

Fyi @DanTup. It would also be interesting to think about how we can make jump to definition work better for some typical Flutter patterns as well. One thing I often find is I wish that go to definition for a Flutter widget within my package would jump to the build method of that widget rather than the class definition of that widget particularly for stateful widgets.

If we did this, perhaps the build method should be listed as an additional target, so the user gets the "peek" pane that lets them see both? I think I would be quite surprised if Go-to-Definition behaved entirely differently on some Flutter-specific classes compared to other classes?

[DanTup]

@bwilkerson

In the legacy protocol we have that for 'return' that will highlight all of the places in a function that return from the function (and, I think, the return type of the function)

Do you know where this might be done? I can't find anything that seems to be doing this (_DartUnitOccurrencesComputerVisitor is where I thought I would find it), is it possible the editor was doing it?

[bwilkerson]

What's more likely is that we had this support in DartEditor (a pre-server editor) and that it never made it into the analysis server.

[DanTup]

It looks like highlighting switch with its cases etc. won't fit into the current protocol classes, because all occurrences are expected to have the same length:

https://htmlpreview.github.io/?https://github.com/dart-lang/sdk/blob/master/pkg/analysis_server/doc/api.html#type_Occurrences

So to support this over that protocol there would need to be some changes (and some way to know whether a client can handle those results). It's slightly simpler to support things only for LSP, although currently the computing of occurrences using the protocol classes, so it'd probably make sense to introduce a protocol-agnostic class if making changes there.

As for Go-to-Def on return, etc. - I missed the link up top that TypeScript is now doing this. I think that's perhaps a good reason to do this (even if only via LSP). VS Code tends to be quite consistent across languages, and finding something useful like this working in one language and then not in another (especially one that's fairly similar) is a little annoying (something I usually find in the other direction 😎).