Inlay hint insertion should consider imports/use qualified paths

[ckuethe]

Describe the Bug

When letting pyrefly suggest type annotations on code that imports re and calls functions like re.compile, the suggested annotation do not include the full module name, at least not in VSCode.

I would have expected that if I import re, and use re.compile(), the inferred type would be re.Pattern, instead of Pattern. What if I'm also using another package that has a different something also called Pattern? I see that same pattern with argparse. If I import argparse and say ap = argparse.ArgumentParser(), the inferred type is ArgumentParser, not argparse.ArgumentParser, and the type of ap.parse_args() is inferred as NameSpace, not argparse.NameSpace.

Also, what do you think about automatically importing Any with from typing import Any if I haven't already done so and pyrefly is suggesting list[Any]? If I accept that suggestion without having imported Any, VSCode tells me it "Could not find name AnyPyrefly(unknown-name)"

Sandbox Link

https://pyrefly.org/sandbox/?code=JYWwDg9gTgLgBFApgKFJWcCGUDmZsDOicAxHAO4AWEcAJhIgXDJcAQPyrjTy2YyIYoYmQA2ggORMCmAJ5wAkhJB0IwAHY4so0XADWG2kwgAzOAGNM49XyhwCMAK4mTAOmTJ88ALwJEr8whwYHEACgkTQxBECQBKUntqR1FadQl4FjY4ACNiAAMkVwAFfgEodQBtBygAXTyAGjh1CHgAK0cHODySmDL1PM4kAmSfOC9XSJsrUXDs5xNEOxwGJkpFmPiyOfgqdbgsgEF1eVpEScRaTg9MMDhfbDxCfwPcR2j1GBKoIihQ+OQbq5MLRaAB9B5vRAfcIAWgAbhJGhIYfDFtkIEREXQzpgRt4AGJWIiNTDmIQQdTeKQwaCIUEwKCODYA3BMe5gVz4b50h4EP4JYBmG77Jh5B5coiuF44SEfL4-BqKCS0OCIAAeYEQZOYrCYNJy+XFT1cADlMNEAMr4cyIPIeZrkO50fiCYSuPgCITRVwO-lbRzwPIe13RPJYJhoHgXEVYOAgCC0ZL+VyuIA

(Only applicable for extension issues) IDE Information

Pyrefly 0.16.1, Code 1.100.2-1747260578, Python 3.12.3, on Ubuntu 24LTS in case that matters.

[yangdanny97]

In inlay hints we don't display the fully-qualified name of the type (and we probably shouldn't, for conciseness).

However, when we actually persist them to the file by double clicking or via CLI (coming soon) we should try to add them in a way that's consistent with the imports. cc @kinto0

[ckuethe]

That would be great.

[kinto0]

I have lots of issues with our inlay hints:

• we can't cmd+click on them
• we don't display anything additional on hover (we could get the docstring + maybe fully-qualified name)
• adding them isn't consistent with imports

I'll see how much I can get to related to this this morning

[ndmitchell]

I don't think you will ever be able to cmd+click or hover them. I just don't think that's how VS Code works.

When you click on them, we should definitely insert the necessary imports. For modules that are already imported qualified, we should probably respect that, e.g.

• If we don't have foo imported, and infer something as x: foo.Bar we should probably display Bar and insert from foo import bar on click.
• If we do have foo imported, as import foo as t or import foo then we should probably display t.Bar or foo.Bar in the inlay hint, since the user already told us how they'd like this imported, and we should respect that.

[stroxler]

Agreed on most points. I also think cmd-click may actually be possible.

When I work on Pyrefly using (open-source, on my macbook) VS Code, the rust inlay hints are indeed command-clickable (and hoverable). I actually use this pretty frequently to jump to a type definition.

If it is in fact possible, then it's a pretty big ux feature precisely for the reason I use it in rust: it makes it much easier to jump to the definition of a type that has only been inferred, not imported

None of the vanilla language server methods seem like they would support this so we might have to dig a bit to figure out how it works, I'm guessing there's a more complex cursor position involved(?)

[kinto0]

It's possible to cmd+click on them in VSCode and I think it's an important part of the hints. The spec cuts it up by InlayHintLabelPart which lets you provide a definition location or more information for each section of it (sections being "Union", "[", "ClassName", "]" etc where Union and Classname are theoretically clickable). since we know the module_info of each class in a type, I think we can find the definition without needing to import it.

In terms of importing when we double click, I might want to leave that for when we implement auto-import quick fixes.

[kinto0]

Attempted all morning. I wanted something similar to TypeDisplayContext.fmt for types but I don't want to duplicate the logic of adding the string literal characters (the stuff we shouldn't be able to click - "[", "class", etc).

The intuition: what if we had shared logic between inlay hints, type formatting for error messages, and type formatting for hover (which could be different)? It seemed like there was a lot of format("mandatory stuff before", actual type, "mandatory stuff after").

I was trying to write an abstract Type visitor which factors out all of the write! calls used by fmt. This will let me make an InlayHintLabelPart out of the contents of write in the inlay hints implementation and keep string formatting in the fmt method.

This works in simple cases but fails with complicated types. It seems like it might be worse in every way (readability, maintainability) than the simpler approach of duplicating the entire method for inlay hints.

I will think about this and revisit soon

[github-actions[bot]]

This issue has been automatically marked as stale because it is assigned, but has not had recent activity for more than 2 weeks.

If you are still working on this issue, please add a comment to keep it active. Otherwise, please unassign yourself and allow someone else to take over.

Thank you for your contributions!

[ckuethe]

it's still a thing, @github-actions

[ndmitchell]

The idea is that the stale bot gives @kinto0 a push to not assign squat. I'll tweak the language to make that clear. The issue itself is definitely not stale.

[github-actions[bot]]

This issue has someone assigned, but has not had recent activity for more than 2 weeks.

If you are still working on this issue, please add a comment so everyone knows. Otherwise, please unassign yourself and allow someone else to take over.

Thank you for your contributions!

[kinto0]

@jvansch1 this is pretty similar to go-to type definition. maybe it's a good continuation?

[asukaminato0721]

This is a bit strange... (yeah, I tried to implement it)

Branch is https://github.com/asukaminato0721/pyrefly/tree/317 .

[jvansch1]

@asukaminato0721 Thanks for working on this! I am going to take a look at your branch soon. I see this commit is in your own fork of Pyrefly. Could you open up a PR with your changes? That will make it easier for us to review on our end. Thanks!

[asukaminato0721]

@asukaminato0721 Thanks for working on this! I am going to take a look at your branch soon. I see this commit is in your own fork of Pyrefly. Could you open up a PR with your changes? That will make it easier for us to review on our end. Thanks!

#1576

[Davidyz]

Just leaving my 2 cents as a user here: basedpyright includes the import statements (for example, from re import Pattern) when providing inlayhints that contains textEdits. As a result, users won't have to worry about writing imports themselves, and it avoids confusions when there are classes of the same name. I haven't read the rest of the source code, but I think it might be reusing the code for auto import (on completion and code actions).