go-to-definition on an import keyword should open the corresponding file

[stephane-archer]

import 'package:flutter/material.dart';

go to the definition on an import statement should open the corresponding file

[bwilkerson]

Navigation currently works for me when I click on the URI. Can you provide more information about what isn't working for you?

[stephane-archer]

Control click on the url works

It would be great if go to definition would work the same. That way I don't need to use the mouse. And it was more intuitive for me (I imagine it would be the same for others)

[bwilkerson]

Ok, thanks. I think I understand now.

It seems a little odd to me to think of a file as being the definition of itself, but I suppose it's not too big a stretch.

@DanTup Is this behavior that would be consistent with go to definition?

[stephane-archer]

I see a path like a function signature and the file content as the function body but I might have a weird mind 🤪

[DanTup]

@DanTup Is this behavior that would be consistent with go to definition?

I think so - and it should already work. I just tested it and it seems to work for me (for relative paths, package: paths, and even dart: paths):

gotodef.mp4

@stephane-archer can you confirm in what cases this doesn't seem to work for you, and what you see (if anything) when clicking Go to Definition?

[stephane-archer]

@DanTup it does not work if your cursor is on the “import” keyword

[DanTup]

Oh, I see. It's less clear to me that it should work for the import keyword, although I can see that it would be convenient.

It's less clear the behaviour would be obvious with conditional imports though:

import 'src/hw_none.dart'
    if (dart.library.io) 'src/hw_io.dart'
    if (dart.library.js_interop) 'src/hw_web.dart';

Presumably you'd want the import keyword to go to hw_none.dart here, but it does feel a little ambiguous.

There is some precedence for Go-to-Definition not strictly being a definition, for example you can use it on keywords like return and break in TypeScript (see microsoft/TypeScript#51222). I'm not sure how I feel about this (it feels semantically wrong, but it's convenient - maybe the issue is really just with the naming...). Although, it appears that they don't support it on the import keyword:

I'm probably on the fence about this and would be interested in others opinions.

[stephane-archer]

That is definitely a convenience.
I didn't expected my cursor position to have an importance on a simple import statement.
In my mind it wasn't working properly.

Why would you prefer the go to definition to return nothing here? The user is clearly expected to get an answer when running the command, not "please move your cursor on the path so I can give you the right answer"

[DanTup]

Why would you prefer the go to definition to return nothing here?

I wouldn't specifically prefer this, but I would prefer that it was obvious what would happen if you do this and I'm not sure if it is in the case of conditional imports (for example might you assume it goes to the import that would resolve for the Dart VM, or whichever Flutter device you have selected in the IDE?).

(We might also need to consider whether changing this would affect IntelliJ, as it uses the navigation data a little differently to VS Code, so I don't know if including the import keyword in the range - or producing an additional range for it - would have any consequences)

[bwilkerson]

I agree, it's completely ambiguous in the case of a conditional import.

We don't currently support "Go to Definition" for any keyword, and I think that's reasonable.

[DanTup]

In that case, I think this is working as intended - shall we close this?