-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[ty] Add a note to the diagnostic if a new builtin is used on an old Python version #18068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
dcb1a5c
to
a06f9ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it!
a06f9ed
to
2ab9783
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the line numbers here changed because I added an import to the top of diagnostic.rs
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how dare you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
// TODO: can we tell the user *why* we're inferring this target version? | ||
// CLI flag? pyproject.toml? Python environment? | ||
diagnostic.info(format_args!( | ||
"The inferred target version of your project is Python {}", | ||
Program::get(context.db()).python_version(context.db()) | ||
)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be too hard. We have the information if it comes from the CLI or configuration but we throw it away when converting it to a ProgramSettings
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome!
// TODO: can we tell the user *why* we're inferring this target version? | ||
// CLI flag? pyproject.toml? Python environment? | ||
diagnostic.info(format_args!( | ||
"The inferred target version of your project is Python {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a bit odd to call it the "inferred" Python version if it came from eg a CLI flag. But I guess this is part of the TODO above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and yes
…eep-dish * origin/main: [ty] __file__ is always a string inside a Python module (#18071) [ty] Recognize submodules in self-referential imports (#18005) [ty] Add a note to the diagnostic if a new builtin is used on an old Python version (#18068) [ty] Add tests for `else` branches of `hasattr()` narrowing (#18067) [ty] Improve diagnostics for `assert_type` and `assert_never` (#18050) [ty] contribution guide (#18061) [ty] Implement `DataClassInstance` protocol for dataclasses. (#18018) [ruff_python_ast] Fix redundant visitation of test expressions in elif clause statements (#18064)
…Python version (astral-sh#18068) ## Summary If the user tries to use a new builtin on an old Python version, tell them what Python version the builtin was added on, what our inferred Python version is for their project, and what configuration settings they can tweak to fix the error. ## Test Plan Snapshots and screenshots: 
Summary
If the user tries to use a new builtin on an old Python version, tell them what Python version the builtin was added on, what our inferred Python version is for their project, and what configuration settings they can tweak to fix the error.
Test Plan
Snapshots and screenshots: