[ty] Add version hint for failed stdlib accesses

[Gankra]

This is the ultra-minimal implementation of

• distinguish "doesn't exist" from "doesn't exist on this Python version" ty#296

that was previously discussed as a good starting point. In particular we don't actually bother trying to figure out the exact python versions, but we still mention "hey btw for No Reason At All... you're on python 3.10" when you try to access something that has a definition rooted in the stdlib that we believe exists sometimes.

[github-actions]

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

[github-actions]

mypy_primer results

Changes were detected when running on open source projects

scikit-build-core (https://github.com/scikit-build/scikit-build-core)
+ src/scikit_build_core/build/_wheelfile.py:51:22: error[no-matching-overload] No overload of function `field` matches arguments
- Found 51 diagnostics
+ Found 52 diagnostics

No memory usage changes detected ✅

[sharkdp]

To be honest, I'm not sure if this initial step is a strict improvement. Sure, it seems helpful in the rare case where you are accessing an attribute that is not available on the Python version that you have configured, but is available under the exact name on a higher version.

But in the vast majority of unresolved-attribute cases (e.g. a misspelled attribute name, or an incomplete attribute name, because I'm still typing it), it just adds additional noise?

"test.exe".trimsuffix(".exe")  # no, it's called "replacesuffix"

error[unresolved-attribute]: Type `Literal["test.exe"]` has no attribute `trimsuffix`
 --> /home/shark/playground/test.py:4:1
  |
4 | "test.exe".trimsuffix(".exe")
  | ^^^^^^^^^^^^^^^^^^^^^
  |
info: Python 3.14 was assumed when accessing `trimsuffix` because it was specified on the command line
info: rule `unresolved-attribute` is enabled by default

Note that the diagnostic change here does not show up in the ecosystem diff because info hints are not part of the concise message, but we have thousands of unresolved-attribute diagnostics across the ecosystem, most of which will be more verbose with this change.

[AlexWaygood]

Yeah, I agree with @sharkdp that in its current state this could just add a lot of noise and not be that helpful :-(

But I think it's pretty easy to make a few adjustments to this so that it's less noisy, but still pretty useful. Something like this, where we do actually check that the exact symbol exists in the symbol table somewhere (the definition just isn't visible on this Python version), could work pretty well:

diff --git a/crates/ty_python_semantic/src/semantic_index/place.rs b/crates/ty_python_semantic/src/semantic_index/place.rs
index 38e3d92816..75d3eaac65 100644
--- a/crates/ty_python_semantic/src/semantic_index/place.rs
+++ b/crates/ty_python_semantic/src/semantic_index/place.rs
@@ -181,7 +181,6 @@ impl PlaceTable {
     }
 
     /// Looks up a symbol by its name and returns a reference to it, if it exists.
-    #[cfg(test)]
     pub(crate) fn symbol_by_name(&self, name: &str) -> Option<&Symbol> {
         self.symbols.symbol_id(name).map(|id| self.symbol(id))
     }
diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs
index f07d934dad..cb6bf8c4e7 100644
--- a/crates/ty_python_semantic/src/types/diagnostic.rs
+++ b/crates/ty_python_semantic/src/types/diagnostic.rs
@@ -9,6 +9,7 @@ use crate::lint::{Level, LintRegistryBuilder, LintStatus};
 use crate::module_resolver::file_to_module;
 use crate::semantic_index::definition::{Definition, DefinitionKind};
 use crate::semantic_index::place::{PlaceTable, ScopedPlaceId};
+use crate::semantic_index::{global_scope, place_table};
 use crate::suppression::FileSuppressionId;
 use crate::types::call::CallError;
 use crate::types::class::{DisjointBase, DisjointBaseKind, Field};
@@ -3155,6 +3156,16 @@ pub(super) fn hint_if_stdlib_attribute_exists_on_other_versions(
     value_type: &Type,
     attr: &Identifier,
 ) {
+    let Type::ModuleLiteral(module) = *value_type else {
+        return;
+    };
+    let Some(file) = module.module(db).file(db) else {
+        return;
+    };
+    let symbol_table = place_table(db, global_scope(db, file));
+    if symbol_table.symbol_by_name(attr).is_none() {
+        return;
+    }
     let Some(definition) = value_type.definition(db) else {
         return;
     };

This edit would mean that the suggestions would only fire on unresolved-attribute errors involving module-literal types. But I think probably most "oops, I tried to access this attribute that only exists on a new Python version" errors involve module-literal objects. So I think even this limited hint would help a lot of users and be a great first step.

[Gankra]

Heh, I considered that implementation originally and was like "wait I can do better, and modules are their own definitions so this handles those too!"

Having seen the results, I agree it's a safer minimum.

[Gankra]

Pushed up the new implementation.

[AlexWaygood]

Thank you, this is great!