Skip to content

Conversation

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 16, 2025

This is the ultra-minimal implementation of

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.

@Gankra Gankra added ty Multi-file analysis & type inference diagnostics Related to reporting of diagnostics. labels Oct 16, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

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 ✅

@Gankra Gankra requested a review from MichaReiser as a code owner October 16, 2025 03:09
@sharkdp
Copy link
Contributor

sharkdp commented Oct 16, 2025

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.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Gankra force-pushed the gankra/old-python-version branch from 8f49ef0 to d08d591 Compare October 16, 2025 13:10
@Gankra
Copy link
Contributor Author

Gankra commented Oct 16, 2025

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 Gankra changed the base branch from gankra/old-python-version to main October 16, 2025 13:23
@Gankra Gankra force-pushed the gankra/older-python branch from 0c41f90 to 1198cb2 Compare October 16, 2025 13:46
@Gankra
Copy link
Contributor Author

Gankra commented Oct 16, 2025

Pushed up the new implementation.

@Gankra Gankra force-pushed the gankra/older-python branch from 1198cb2 to 3079cc3 Compare October 16, 2025 13:48
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is great!

@Gankra Gankra enabled auto-merge (squash) October 16, 2025 14:03
@Gankra Gankra merged commit 7155a62 into main Oct 16, 2025
39 of 40 checks passed
@Gankra Gankra deleted the gankra/older-python branch October 16, 2025 14:07
dcreager added a commit that referenced this pull request Oct 16, 2025
…rable

* origin/main:
  [ty] Support dataclass-transform `field_specifiers` (#20888)
  Bump 0.14.1 (#20925)
  Standardize syntax error construction (#20903)
  [`pydoclint`] Implement `docstring-extraneous-parameter` (`DOC102`) (#20376)
  [ty] Fix panic 'missing root' when handling completion request (#20917)
  [ty] Run file watching tests serial when using nextest (#20918)
  [ty] Add version hint for failed stdlib attribute accesses (#20909)
  More CI improvements (#20920)
  [ty] Check typeshed VERSIONS for parent modules when reporting failed stdlib imports (#20908)
dcreager added a commit that referenced this pull request Oct 17, 2025
* main:
  [ty] Prefer declared type for invariant collection literals (#20927)
  [ty] Don't track inferability via different `Type` variants (#20677)
  [ty] Use declared variable types as bidirectional type context (#20796)
  [ty] Avoid unnecessarily widening generic specializations (#20875)
  [ty] Support dataclass-transform `field_specifiers` (#20888)
  Bump 0.14.1 (#20925)
  Standardize syntax error construction (#20903)
  [`pydoclint`] Implement `docstring-extraneous-parameter` (`DOC102`) (#20376)
  [ty] Fix panic 'missing root' when handling completion request (#20917)
  [ty] Run file watching tests serial when using nextest (#20918)
  [ty] Add version hint for failed stdlib attribute accesses (#20909)
  More CI improvements (#20920)
  [ty] Check typeshed VERSIONS for parent modules when reporting failed stdlib imports (#20908)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diagnostics Related to reporting of diagnostics. ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants