Skip to content
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

[pyflakes] Improve error message for UndefinedName when a builtin was added in a newer version than specified in Ruff config (F821) #13293

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

AlexWaygood
Copy link
Member

Summary

Fixes #13287.

This adds a new function to ruff_python_stdlib::builtins that allows us to query if a symbol is a Python builtin that was added in a later version than the one that was specified in a user's Ruff configuration. This function allows us to provide a better error message if a builtin such as aiter, anext etc. is used, but the user either specified a low target-version in their Ruff configuration or didn't specify a target-version at all (which leads us to infer the default, currently py38).

Test Plan

cargo test -p ruff_linter --lib

@AlexWaygood AlexWaygood added rule Implementing or modifying a lint rule linter Related to the linter labels Sep 9, 2024
Comment on lines 39 to 44
"Undefined name `{name}`. \
Added as a builtin in Python 3.{minor_version_builtin_added}; \
consider specifying a newer `target-version` in your Ruff config."
Copy link
Member Author

Choose a reason for hiding this comment

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

This feels quite verbose but I'm not sure how to make it any shorter :/

@AlexWaygood AlexWaygood changed the title [pyflakes] Improve error message for UndefinedName when a builtin was added in a newer version than specified in Ruff config (F821)) [pyflakes] Improve error message for UndefinedName when a builtin was added in a newer version than specified in Ruff config (F821) Sep 9, 2024
Copy link

codspeed-hq bot commented Sep 9, 2024

CodSpeed Performance Report

Merging #13293 will not alter performance

Comparing alex/f821-error-msg (183ecab) with main (b7cef6c)

Summary

✅ 32 untouched benchmarks

@AlexWaygood
Copy link
Member Author

Merging #13293 will degrade performances by 6.48%

Seems spurious to me

Copy link
Contributor

github-actions bot commented Sep 9, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+1 -1 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)

pytest-dev/pytest (+1 -1 violations, +0 -0 fixes)

- testing/_py/test_local.py:22:45: F821 Undefined name `EncodingWarning`
+ testing/_py/test_local.py:22:45: F821 Undefined name `EncodingWarning`. Consider specifying `requires-python = ">= 3.10"` or `tool.ruff.target-version = "py310"` in your `pyproject.toml` file.

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
F821 2 1 1 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1 -1 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)

pytest-dev/pytest (+1 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- testing/_py/test_local.py:22:45: F821 Undefined name `EncodingWarning`
+ testing/_py/test_local.py:22:45: F821 Undefined name `EncodingWarning`. Consider specifying `requires-python = ">= 3.10"` or `tool.ruff.target-version = "py310"` in your `pyproject.toml` file.

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
F821 2 1 1 0 0

… was added in a newer version than specified in Ruff config
Comment on lines 41 to 53
let tip = minor_version_builtin_added.map(|version_added| {
format!(
"Added as a builtin in Python 3.{version_added}; \
consider specifying a newer `target-version` in your Ruff config."
)
});

if let Some(tip) = tip {
format!("Undefined name `{name}`. {tip}")
} else {
format!("Undefined name `{name}`")
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be done as a single if-let, but we get nicer docs if we split it up like this (the generated summary message in the table is very long otherwise):

image

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

I'm probably too jetlag to understand the new function you added but maybe you can explain.

crates/ruff_python_stdlib/src/builtins.rs Show resolved Hide resolved
crates/ruff_python_stdlib/src/builtins.rs Outdated Show resolved Hide resolved
@AlexWaygood AlexWaygood enabled auto-merge (squash) September 10, 2024 18:00
@AlexWaygood AlexWaygood merged commit 1d5bd89 into main Sep 10, 2024
18 checks passed
@AlexWaygood AlexWaygood deleted the alex/f821-error-msg branch September 10, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter Related to the linter rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[F821] Improve error message for anext/aiter
2 participants