From 3145695d6929082deb170d492989eae3217fdc4e Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 9 Sep 2024 12:06:53 -0400 Subject: [PATCH 1/3] [`pyflakes`] Improve error message for `UndefinedName` when a builtin was added in a newer version than specified in Ruff config --- .../ast/analyze/unresolved_references.rs | 9 +- crates/ruff_linter/src/rules/pyflakes/mod.rs | 12 + .../rules/pyflakes/rules/undefined_name.rs | 23 +- ...sion_but_old_target_version_specified.snap | 8 + crates/ruff_python_stdlib/src/builtins.rs | 347 ++++++++++-------- 5 files changed, 234 insertions(+), 165 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap diff --git a/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs b/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs index c4d6cc65ab3c2..01ce9ebcfc1b1 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs @@ -1,5 +1,6 @@ use ruff_diagnostics::Diagnostic; use ruff_python_semantic::Exceptions; +use ruff_python_stdlib::builtins::version_builtin_was_added; use crate::checkers::ast::Checker; use crate::codes::Rule; @@ -35,9 +36,15 @@ pub(crate) fn unresolved_references(checker: &mut Checker) { } } + let symbol_name = reference.name(checker.locator); + checker.diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedName { - name: reference.name(checker.locator).to_string(), + name: symbol_name.to_string(), + minor_version_builtin_added: version_builtin_was_added( + symbol_name, + checker.settings.target_version.minor(), + ), }, reference.range(), )); diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index f1d9117d42609..34d03f6805c97 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -208,6 +208,18 @@ mod tests { Ok(()) } + #[test] + fn f821_with_builtin_added_on_new_py_version_but_old_target_version_specified() { + let diagnostics = test_snippet( + "PythonFinalizationError", + &LinterSettings { + target_version: crate::settings::types::PythonVersion::Py312, + ..LinterSettings::for_rule(Rule::UndefinedName) + }, + ); + assert_messages!(diagnostics); + } + #[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))] #[test_case(Rule::UnusedImport, Path::new("__init__.py"))] #[test_case(Rule::UnusedImport, Path::new("F401_24/__init__.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs index 9fae09e6c248c..e1e9b4f315522 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs @@ -19,17 +19,36 @@ use ruff_macros::{derive_message_formats, violation}; /// return n * 2 /// ``` /// +/// ## Options +/// - [`target-version`]: Can be used to configure which symbols Ruff will understand +/// as being available in the `builtins` namespace. +/// /// ## References /// - [Python documentation: Naming and binding](https://docs.python.org/3/reference/executionmodel.html#naming-and-binding) #[violation] pub struct UndefinedName { pub(crate) name: String, + pub(crate) minor_version_builtin_added: Option, } impl Violation for UndefinedName { #[derive_message_formats] fn message(&self) -> String { - let UndefinedName { name } = self; - format!("Undefined name `{name}`") + let UndefinedName { + name, + minor_version_builtin_added, + } = self; + 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}`") + } } } diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap new file mode 100644 index 0000000000000..32f55f03b5564 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap @@ -0,0 +1,8 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +:1:1: F821 Undefined name `PythonFinalizationError`. Added as a builtin in Python 3.13; consider specifying a newer `target-version` in your Ruff config. + | +1 | PythonFinalizationError + | ^^^^^^^^^^^^^^^^^^^^^^^ F821 + | diff --git a/crates/ruff_python_stdlib/src/builtins.rs b/crates/ruff_python_stdlib/src/builtins.rs index 6c65fcafc9cb1..6dac4cb928fbc 100644 --- a/crates/ruff_python_stdlib/src/builtins.rs +++ b/crates/ruff_python_stdlib/src/builtins.rs @@ -23,181 +23,181 @@ pub const MAGIC_GLOBALS: &[&str] = &[ "__file__", ]; +static ALWAYS_AVAILABLE_BUILTINS: &[&str] = &[ + "ArithmeticError", + "AssertionError", + "AttributeError", + "BaseException", + "BlockingIOError", + "BrokenPipeError", + "BufferError", + "BytesWarning", + "ChildProcessError", + "ConnectionAbortedError", + "ConnectionError", + "ConnectionRefusedError", + "ConnectionResetError", + "DeprecationWarning", + "EOFError", + "Ellipsis", + "EnvironmentError", + "Exception", + "False", + "FileExistsError", + "FileNotFoundError", + "FloatingPointError", + "FutureWarning", + "GeneratorExit", + "IOError", + "ImportError", + "ImportWarning", + "IndentationError", + "IndexError", + "InterruptedError", + "IsADirectoryError", + "KeyError", + "KeyboardInterrupt", + "LookupError", + "MemoryError", + "ModuleNotFoundError", + "NameError", + "None", + "NotADirectoryError", + "NotImplemented", + "NotImplementedError", + "OSError", + "OverflowError", + "PendingDeprecationWarning", + "PermissionError", + "ProcessLookupError", + "RecursionError", + "ReferenceError", + "ResourceWarning", + "RuntimeError", + "RuntimeWarning", + "StopAsyncIteration", + "StopIteration", + "SyntaxError", + "SyntaxWarning", + "SystemError", + "SystemExit", + "TabError", + "TimeoutError", + "True", + "TypeError", + "UnboundLocalError", + "UnicodeDecodeError", + "UnicodeEncodeError", + "UnicodeError", + "UnicodeTranslateError", + "UnicodeWarning", + "UserWarning", + "ValueError", + "Warning", + "ZeroDivisionError", + "__build_class__", + "__debug__", + "__doc__", + "__import__", + "__loader__", + "__name__", + "__package__", + "__spec__", + "abs", + "all", + "any", + "ascii", + "bin", + "bool", + "breakpoint", + "bytearray", + "bytes", + "callable", + "chr", + "classmethod", + "compile", + "complex", + "copyright", + "credits", + "delattr", + "dict", + "dir", + "divmod", + "enumerate", + "eval", + "exec", + "exit", + "filter", + "float", + "format", + "frozenset", + "getattr", + "globals", + "hasattr", + "hash", + "help", + "hex", + "id", + "input", + "int", + "isinstance", + "issubclass", + "iter", + "len", + "license", + "list", + "locals", + "map", + "max", + "memoryview", + "min", + "next", + "object", + "oct", + "open", + "ord", + "pow", + "print", + "property", + "quit", + "range", + "repr", + "reversed", + "round", + "set", + "setattr", + "slice", + "sorted", + "staticmethod", + "str", + "sum", + "super", + "tuple", + "type", + "vars", + "zip", +]; +static PY310_PLUS_BUILTINS: &[&str] = &["EncodingWarning", "aiter", "anext"]; +static PY311_PLUS_BUILTINS: &[&str] = &["BaseExceptionGroup", "ExceptionGroup"]; +static PY313_PLUS_BUILTINS: &[&str] = &["PythonFinalizationError"]; + /// Return the list of builtins for the given Python minor version. /// /// Intended to be kept in sync with [`is_python_builtin`]. pub fn python_builtins(minor_version: u8, is_notebook: bool) -> Vec<&'static str> { - let mut builtins = vec![ - "ArithmeticError", - "AssertionError", - "AttributeError", - "BaseException", - "BlockingIOError", - "BrokenPipeError", - "BufferError", - "BytesWarning", - "ChildProcessError", - "ConnectionAbortedError", - "ConnectionError", - "ConnectionRefusedError", - "ConnectionResetError", - "DeprecationWarning", - "EOFError", - "Ellipsis", - "EnvironmentError", - "Exception", - "False", - "FileExistsError", - "FileNotFoundError", - "FloatingPointError", - "FutureWarning", - "GeneratorExit", - "IOError", - "ImportError", - "ImportWarning", - "IndentationError", - "IndexError", - "InterruptedError", - "IsADirectoryError", - "KeyError", - "KeyboardInterrupt", - "LookupError", - "MemoryError", - "ModuleNotFoundError", - "NameError", - "None", - "NotADirectoryError", - "NotImplemented", - "NotImplementedError", - "OSError", - "OverflowError", - "PendingDeprecationWarning", - "PermissionError", - "ProcessLookupError", - "RecursionError", - "ReferenceError", - "ResourceWarning", - "RuntimeError", - "RuntimeWarning", - "StopAsyncIteration", - "StopIteration", - "SyntaxError", - "SyntaxWarning", - "SystemError", - "SystemExit", - "TabError", - "TimeoutError", - "True", - "TypeError", - "UnboundLocalError", - "UnicodeDecodeError", - "UnicodeEncodeError", - "UnicodeError", - "UnicodeTranslateError", - "UnicodeWarning", - "UserWarning", - "ValueError", - "Warning", - "ZeroDivisionError", - "__build_class__", - "__debug__", - "__doc__", - "__import__", - "__loader__", - "__name__", - "__package__", - "__spec__", - "abs", - "all", - "any", - "ascii", - "bin", - "bool", - "breakpoint", - "bytearray", - "bytes", - "callable", - "chr", - "classmethod", - "compile", - "complex", - "copyright", - "credits", - "delattr", - "dict", - "dir", - "divmod", - "enumerate", - "eval", - "exec", - "exit", - "filter", - "float", - "format", - "frozenset", - "getattr", - "globals", - "hasattr", - "hash", - "help", - "hex", - "id", - "input", - "int", - "isinstance", - "issubclass", - "iter", - "len", - "license", - "list", - "locals", - "map", - "max", - "memoryview", - "min", - "next", - "object", - "oct", - "open", - "ord", - "pow", - "print", - "property", - "quit", - "range", - "repr", - "reversed", - "round", - "set", - "setattr", - "slice", - "sorted", - "staticmethod", - "str", - "sum", - "super", - "tuple", - "type", - "vars", - "zip", - ]; - + let mut builtins = ALWAYS_AVAILABLE_BUILTINS.to_vec(); if minor_version >= 10 { - builtins.extend(&["EncodingWarning", "aiter", "anext"]); + builtins.extend(PY310_PLUS_BUILTINS); } - if minor_version >= 11 { - builtins.extend(&["BaseExceptionGroup", "ExceptionGroup"]); + builtins.extend(PY311_PLUS_BUILTINS); } - if minor_version >= 13 { - builtins.push("PythonFinalizationError"); + builtins.extend(PY313_PLUS_BUILTINS); } - if is_notebook { builtins.extend(IPYTHON_BUILTINS); } - builtins } @@ -370,6 +370,29 @@ pub fn is_python_builtin(name: &str, minor_version: u8, is_notebook: bool) -> bo ) } +/// Return `Some(version)`, where `version` corresponds to the Python minor version +/// in which the builtin was added, if `name` corresponds to a Python builtin that +/// was added in a later Python version than the one which the user specified in +/// their Ruff config. Otherwise, return `None`. +/// +/// This is useful in some situations to give a better error message +/// if the user didn't specify a target version at all (in which case we will default +/// to Python 3.8). +pub fn version_builtin_was_added(name: &str, minor_version: u8) -> Option { + if minor_version < 10 { + (PY310_PLUS_BUILTINS.contains(&name) + || PY311_PLUS_BUILTINS.contains(&name) + || PY313_PLUS_BUILTINS.contains(&name)) + .then_some(10) + } else if minor_version < 11 { + (PY311_PLUS_BUILTINS.contains(&name) || PY313_PLUS_BUILTINS.contains(&name)).then_some(11) + } else if minor_version < 13 { + PY313_PLUS_BUILTINS.contains(&name).then_some(13) + } else { + None + } +} + /// Returns `true` if the given name is that of a Python builtin iterator. pub fn is_iterator(name: &str) -> bool { matches!( From 47ae15f5f985635de1b0154089d46708dd458265 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 9 Sep 2024 17:30:20 -0400 Subject: [PATCH 2/3] simplify --- .../ast/analyze/unresolved_references.rs | 5 +--- crates/ruff_python_stdlib/src/builtins.rs | 27 +++++++------------ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs b/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs index 01ce9ebcfc1b1..e0d77052047d1 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs @@ -41,10 +41,7 @@ pub(crate) fn unresolved_references(checker: &mut Checker) { checker.diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedName { name: symbol_name.to_string(), - minor_version_builtin_added: version_builtin_was_added( - symbol_name, - checker.settings.target_version.minor(), - ), + minor_version_builtin_added: version_builtin_was_added(symbol_name), }, reference.range(), )); diff --git a/crates/ruff_python_stdlib/src/builtins.rs b/crates/ruff_python_stdlib/src/builtins.rs index 6dac4cb928fbc..f8ba9a353026b 100644 --- a/crates/ruff_python_stdlib/src/builtins.rs +++ b/crates/ruff_python_stdlib/src/builtins.rs @@ -371,23 +371,16 @@ pub fn is_python_builtin(name: &str, minor_version: u8, is_notebook: bool) -> bo } /// Return `Some(version)`, where `version` corresponds to the Python minor version -/// in which the builtin was added, if `name` corresponds to a Python builtin that -/// was added in a later Python version than the one which the user specified in -/// their Ruff config. Otherwise, return `None`. -/// -/// This is useful in some situations to give a better error message -/// if the user didn't specify a target version at all (in which case we will default -/// to Python 3.8). -pub fn version_builtin_was_added(name: &str, minor_version: u8) -> Option { - if minor_version < 10 { - (PY310_PLUS_BUILTINS.contains(&name) - || PY311_PLUS_BUILTINS.contains(&name) - || PY313_PLUS_BUILTINS.contains(&name)) - .then_some(10) - } else if minor_version < 11 { - (PY311_PLUS_BUILTINS.contains(&name) || PY313_PLUS_BUILTINS.contains(&name)).then_some(11) - } else if minor_version < 13 { - PY313_PLUS_BUILTINS.contains(&name).then_some(13) +/// in which the builtin was added +pub fn version_builtin_was_added(name: &str) -> Option { + if PY310_PLUS_BUILTINS.contains(&name) { + Some(10) + } else if PY311_PLUS_BUILTINS.contains(&name) { + Some(11) + } else if PY313_PLUS_BUILTINS.contains(&name) { + Some(13) + } else if ALWAYS_AVAILABLE_BUILTINS.contains(&name) { + Some(0) } else { None } From 183ecab67b5531bfbba98a207cbec5b688b5ef3e Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 10 Sep 2024 13:59:05 -0400 Subject: [PATCH 3/3] better error message --- crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs | 3 +-- ...ded_on_new_py_version_but_old_target_version_specified.snap | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs index e1e9b4f315522..0c46c14a44a21 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_name.rs @@ -40,8 +40,7 @@ impl Violation for UndefinedName { } = self; 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." + r#"Consider specifying `requires-python = ">= 3.{version_added}"` or `tool.ruff.target-version = "py3{version_added}"` in your `pyproject.toml` file."# ) }); diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap index 32f55f03b5564..1421f07c75e11 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f821_with_builtin_added_on_new_py_version_but_old_target_version_specified.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -:1:1: F821 Undefined name `PythonFinalizationError`. Added as a builtin in Python 3.13; consider specifying a newer `target-version` in your Ruff config. +:1:1: F821 Undefined name `PythonFinalizationError`. Consider specifying `requires-python = ">= 3.13"` or `tool.ruff.target-version = "py313"` in your `pyproject.toml` file. | 1 | PythonFinalizationError | ^^^^^^^^^^^^^^^^^^^^^^^ F821