[ty] followup: add-import action for reveal_type too

[Gankra]

No description provided.

[astral-sh-bot]

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

[astral-sh-bot]

mypy_primer results

No ecosystem changes detected ✅

No memory usage changes detected ✅

[AlexWaygood]

This specific one probably could be added as a Fix, right? There's no extra computation required for us to figure out where we need to import it from, so it wouldn't slow down type-checking for it to be added as a Fix when we emit the diagnostic.

Also, it's possible we shouldn't suggest this autofix when the Python version is <3.11. It was only added to the typing module on Python 3.11 -- on earlier versions, you can import it from typing_extensions, but we don't know whether the user actually has that installed as a dependency or not. (According to typeshed, typing_extensions is part of the stdlib, but don't believe typeshed's lies -- that's just so that typeshed can use new typing features in the stdlib stubs ;)

[Gankra]

Oh that's an interesting wrinkle, we absolutely constantly suggest importing stuff from typing_extensions 😬

[AlexWaygood]

can you use the resolve_module() option you added that ignores stub files to check whether there is actually a version of typing_extensions installed into site-packages before suggesting a typing_extensions import?

[Gankra]

Filed:

• completions suggest importing typing_extensions even if you don't depend on it ty#1658

[MichaReiser]

I'm inclined to exclude typing_extensions from our auto-import machinery for now, here. But I'd also be okay leaving as is, given that these are all still manual actions

ruff/crates/ty_ide/src/completion.rs

Lines 523 to 526 in 054e32e

	if symbol.module.file(db) == Some(file) || symbol.module.is_known(db, KnownModule::Builtins)
	{
	continue;
	}

I think I'd prefer making this a Fix on the reveal_type diagnostic as it then also becomes available on the CLI

[MichaReiser]

Suggested change

	let lint_name = lint_id.name();
	if lint_name == UNRESOLVED_REFERENCE.name() || lint_name == UNDEFINED_REVEAL.name() {
	if lint_id == LintId::of(&UNRESOLVED_REFERENCE) || lint_id == LintId::of(&UNDEFINED_REVEAL){

[Gankra]

Thinking about it more, just because it's Almost Definitely typing.reveal_type doesn't actually mean it is? It feels a little dubious to refuse to check if their project has its own reveal_type

[MichaReiser]

Thinking about it more, just because it's Almost Definitely typing.reveal_type doesn't actually mean it is? It feels a little dubious to refuse to check if their project has its own reveal_type

I think it's fine as an unsafe fix

[Gankra]

I keep getting annoyed that this doesn't work so I'm just going to land this and file followups.