diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF063.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF063.py new file mode 100644 index 0000000000000..7681b5eea127b --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF063.py @@ -0,0 +1,18 @@ +# RUF063 +# Cases that should trigger the violation + +foo.__dict__.get("__annotations__") # RUF063 +foo.__dict__.get("__annotations__", None) # RUF063 +foo.__dict__.get("__annotations__", {}) # RUF063 +foo.__dict__["__annotations__"] # RUF063 + +# Cases that should NOT trigger the violation + +foo.__dict__.get("not__annotations__") +foo.__dict__.get("not__annotations__", None) +foo.__dict__.get("not__annotations__", {}) +foo.__dict__["not__annotations__"] +foo.__annotations__ +foo.get("__annotations__") +foo.get("__annotations__", None) +foo.get("__annotations__", {}) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 09f1f4f4e9fb8..79b5be716e93e 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -179,6 +179,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { if checker.enabled(Rule::MissingMaxsplitArg) { pylint::rules::missing_maxsplit_arg(checker, value, slice, expr); } + if checker.enabled(Rule::AccessAnnotationsFromClassDict) { + ruff::rules::access_annotations_from_class_dict_by_key(checker, subscript); + } pandas_vet::rules::subscript(checker, value, expr); } Expr::Tuple(ast::ExprTuple { @@ -1196,6 +1199,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { if checker.enabled(Rule::StarmapZip) { ruff::rules::starmap_zip(checker, call); } + if checker.enabled(Rule::AccessAnnotationsFromClassDict) { + ruff::rules::access_annotations_from_class_dict_with_get(checker, call); + } if checker.enabled(Rule::LogExceptionOutsideExceptHandler) { flake8_logging::rules::log_exception_outside_except_handler(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 144006b07db24..16b3baaa06f10 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1028,6 +1028,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "059") => (RuleGroup::Preview, rules::ruff::rules::UnusedUnpackedVariable), (Ruff, "060") => (RuleGroup::Preview, rules::ruff::rules::InEmptyCollection), (Ruff, "061") => (RuleGroup::Preview, rules::ruff::rules::LegacyFormPytestRaises), + (Ruff, "063") => (RuleGroup::Preview, rules::ruff::rules::AccessAnnotationsFromClassDict), (Ruff, "064") => (RuleGroup::Preview, rules::ruff::rules::NonOctalPermissions), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index a41dccdaecf82..57aa4c7912964 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -171,6 +171,60 @@ mod tests { Ok(()) } + #[test] + fn access_annotations_from_class_dict_py39_no_typing_extensions() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF063.py"), + &LinterSettings { + typing_extensions: false, + unresolved_target_version: PythonVersion::PY39.into(), + ..LinterSettings::for_rule(Rule::AccessAnnotationsFromClassDict) + }, + )?; + assert_diagnostics!(diagnostics); + Ok(()) + } + + #[test] + fn access_annotations_from_class_dict_py39_with_typing_extensions() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF063.py"), + &LinterSettings { + typing_extensions: true, + unresolved_target_version: PythonVersion::PY39.into(), + ..LinterSettings::for_rule(Rule::AccessAnnotationsFromClassDict) + }, + )?; + assert_diagnostics!(diagnostics); + Ok(()) + } + + #[test] + fn access_annotations_from_class_dict_py310() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF063.py"), + &LinterSettings { + unresolved_target_version: PythonVersion::PY310.into(), + ..LinterSettings::for_rule(Rule::AccessAnnotationsFromClassDict) + }, + )?; + assert_diagnostics!(diagnostics); + Ok(()) + } + + #[test] + fn access_annotations_from_class_dict_py314() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF063.py"), + &LinterSettings { + unresolved_target_version: PythonVersion::PY314.into(), + ..LinterSettings::for_rule(Rule::AccessAnnotationsFromClassDict) + }, + )?; + assert_diagnostics!(diagnostics); + Ok(()) + } + #[test] fn confusables() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/ruff/rules/access_annotations_from_class_dict.rs b/crates/ruff_linter/src/rules/ruff/rules/access_annotations_from_class_dict.rs new file mode 100644 index 0000000000000..f8961f82a4aa8 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/access_annotations_from_class_dict.rs @@ -0,0 +1,185 @@ +use crate::checkers::ast::Checker; +use crate::{FixAvailability, Violation}; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::{Expr, ExprCall, ExprSubscript, PythonVersion}; +use ruff_text_size::Ranged; + +/// ## What it does +/// Checks for uses of `foo.__dict__.get("__annotations__")` or +/// `foo.__dict__["__annotations__"]` on Python 3.10+ and Python < 3.10 when +/// [typing-extensions](https://docs.astral.sh/ruff/settings/#lint_typing-extensions) +/// is enabled. +/// +/// ## Why is this bad? +/// Starting with Python 3.14, directly accessing `__annotations__` via +/// `foo.__dict__.get("__annotations__")` or `foo.__dict__["__annotations__"]` +/// will only return annotations if the class is defined under +/// `from __future__ import annotations`. +/// +/// Therefore, it is better to use dedicated library functions like +/// `annotationlib.get_annotations` (Python 3.14+), `inspect.get_annotations` +/// (Python 3.10+), or `typing_extensions.get_annotations` (for Python < 3.10 if +/// [typing-extensions](https://pypi.org/project/typing-extensions/) is +/// available). +/// +/// The benefits of using these functions include: +/// 1. **Avoiding Undocumented Internals:** They provide a stable, public API, +/// unlike direct `__dict__` access which relies on implementation details. +/// 2. **Forward-Compatibility:** They are designed to handle changes in +/// Python's annotation system across versions, ensuring your code remains +/// robust (e.g., correctly handling the Python 3.14 behavior mentioned +/// above). +/// +/// See [Python Annotations Best Practices](https://docs.python.org/3.14/howto/annotations.html) +/// for alternatives. +/// +/// ## Example +/// +/// ```python +/// foo.__dict__.get("__annotations__", {}) +/// # or +/// foo.__dict__["__annotations__"] +/// ``` +/// +/// On Python 3.14+, use instead: +/// ```python +/// import annotationlib +/// +/// annotationlib.get_annotations(foo) +/// ``` +/// +/// On Python 3.10+, use instead: +/// ```python +/// import inspect +/// +/// inspect.get_annotations(foo) +/// ``` +/// +/// On Python < 3.10 with [typing-extensions](https://pypi.org/project/typing-extensions/) +/// installed, use instead: +/// ```python +/// import typing_extensions +/// +/// typing_extensions.get_annotations(foo) +/// ``` +/// +/// ## Fix safety +/// +/// No autofix is currently provided for this rule. +/// +/// ## Fix availability +/// +/// No autofix is currently provided for this rule. +/// +/// ## References +/// - [Python Annotations Best Practices](https://docs.python.org/3.14/howto/annotations.html) +#[derive(ViolationMetadata)] +pub(crate) struct AccessAnnotationsFromClassDict { + python_version: PythonVersion, +} + +impl Violation for AccessAnnotationsFromClassDict { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + let suggestion = if self.python_version >= PythonVersion::PY314 { + "annotationlib.get_annotations" + } else if self.python_version >= PythonVersion::PY310 { + "inspect.get_annotations" + } else { + "typing_extensions.get_annotations" + }; + format!("Use `{suggestion}` instead of `__dict__` access") + } +} + +/// RUF063 +pub(crate) fn access_annotations_from_class_dict_with_get(checker: &Checker, call: &ExprCall) { + let python_version = checker.target_version(); + let typing_extensions = checker.settings.typing_extensions; + + // Only apply this rule for Python 3.10 and newer unless `typing-extensions` is enabled. + if python_version < PythonVersion::PY310 && !typing_extensions { + return; + } + + // Expected pattern: foo.__dict__.get("__annotations__" [, ]) + // Here, `call` is the `.get(...)` part. + + // 1. Check that the `call.func` is `get` + let get_attribute = match call.func.as_ref() { + Expr::Attribute(attr) if attr.attr.as_str() == "get" => attr, + _ => return, + }; + + // 2. Check that the `get_attribute.value` is `__dict__` + match get_attribute.value.as_ref() { + Expr::Attribute(attr) if attr.attr.as_str() == "__dict__" => {} + _ => return, + } + + // At this point, we have `foo.__dict__.get`. + + // 3. Check arguments to `.get()`: + // - No keyword arguments. + // - One or two positional arguments. + // - First positional argument must be the string literal "__annotations__". + // - The value of the second positional argument (if present) does not affect the match. + if !call.arguments.keywords.is_empty() || call.arguments.len() > 2 { + return; + } + + let Some(first_arg) = &call.arguments.find_positional(0) else { + return; + }; + + let is_first_arg_correct = first_arg + .as_string_literal_expr() + .is_some_and(|s| s.value.to_str() == "__annotations__"); + + if is_first_arg_correct { + checker.report_diagnostic( + AccessAnnotationsFromClassDict { python_version }, + call.range(), + ); + } +} + +/// RUF063 +pub(crate) fn access_annotations_from_class_dict_by_key( + checker: &Checker, + subscript: &ExprSubscript, +) { + let python_version = checker.target_version(); + let typing_extensions = checker.settings.typing_extensions; + + // Only apply this rule for Python 3.10 and newer unless `typing-extensions` is enabled. + if python_version < PythonVersion::PY310 && !typing_extensions { + return; + } + + // Expected pattern: foo.__dict__["__annotations__"] + + // 1. Check that the slice is a string literal "__annotations__" + if subscript + .slice + .as_string_literal_expr() + .is_none_or(|s| s.value.to_str() != "__annotations__") + { + return; + } + + // 2. Check that the `subscript.value` is `__dict__` + let is_value_correct = subscript + .value + .as_attribute_expr() + .is_some_and(|attr| attr.attr.as_str() == "__dict__"); + + if is_value_correct { + checker.report_diagnostic( + AccessAnnotationsFromClassDict { python_version }, + subscript.range(), + ); + } +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 39df796762818..375034fadc255 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -1,3 +1,4 @@ +pub(crate) use access_annotations_from_class_dict::*; pub(crate) use ambiguous_unicode_character::*; pub(crate) use assert_with_print_message::*; pub(crate) use assignment_in_assert::*; @@ -59,6 +60,7 @@ pub(crate) use used_dummy_variable::*; pub(crate) use useless_if_else::*; pub(crate) use zip_instead_of_pairwise::*; +mod access_annotations_from_class_dict; mod ambiguous_unicode_character; mod assert_with_print_message; mod assignment_in_assert; diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py310.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py310.snap new file mode 100644 index 0000000000000..06274f1af201b --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py310.snap @@ -0,0 +1,40 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF063.py:4:1: RUF063 Use `inspect.get_annotations` instead of `__dict__` access + | +2 | # Cases that should trigger the violation +3 | +4 | foo.__dict__.get("__annotations__") # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 + | + +RUF063.py:5:1: RUF063 Use `inspect.get_annotations` instead of `__dict__` access + | +4 | foo.__dict__.get("__annotations__") # RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | + +RUF063.py:6:1: RUF063 Use `inspect.get_annotations` instead of `__dict__` access + | +4 | foo.__dict__.get("__annotations__") # RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | + +RUF063.py:7:1: RUF063 Use `inspect.get_annotations` instead of `__dict__` access + | +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +8 | +9 | # Cases that should NOT trigger the violation + | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py314.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py314.snap new file mode 100644 index 0000000000000..0f07c9a8e954d --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py314.snap @@ -0,0 +1,40 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF063.py:4:1: RUF063 Use `annotationlib.get_annotations` instead of `__dict__` access + | +2 | # Cases that should trigger the violation +3 | +4 | foo.__dict__.get("__annotations__") # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 + | + +RUF063.py:5:1: RUF063 Use `annotationlib.get_annotations` instead of `__dict__` access + | +4 | foo.__dict__.get("__annotations__") # RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | + +RUF063.py:6:1: RUF063 Use `annotationlib.get_annotations` instead of `__dict__` access + | +4 | foo.__dict__.get("__annotations__") # RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | + +RUF063.py:7:1: RUF063 Use `annotationlib.get_annotations` instead of `__dict__` access + | +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +8 | +9 | # Cases that should NOT trigger the violation + | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py39_no_typing_extensions.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py39_no_typing_extensions.snap new file mode 100644 index 0000000000000..7f58cfd7246a3 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py39_no_typing_extensions.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py39_with_typing_extensions.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py39_with_typing_extensions.snap new file mode 100644 index 0000000000000..c5ef0419bad28 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__access_annotations_from_class_dict_py39_with_typing_extensions.snap @@ -0,0 +1,40 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF063.py:4:1: RUF063 Use `typing_extensions.get_annotations` instead of `__dict__` access + | +2 | # Cases that should trigger the violation +3 | +4 | foo.__dict__.get("__annotations__") # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 + | + +RUF063.py:5:1: RUF063 Use `typing_extensions.get_annotations` instead of `__dict__` access + | +4 | foo.__dict__.get("__annotations__") # RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | + +RUF063.py:6:1: RUF063 Use `typing_extensions.get_annotations` instead of `__dict__` access + | +4 | foo.__dict__.get("__annotations__") # RUF063 +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | + +RUF063.py:7:1: RUF063 Use `typing_extensions.get_annotations` instead of `__dict__` access + | +5 | foo.__dict__.get("__annotations__", None) # RUF063 +6 | foo.__dict__.get("__annotations__", {}) # RUF063 +7 | foo.__dict__["__annotations__"] # RUF063 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF063 +8 | +9 | # Cases that should NOT trigger the violation + | diff --git a/ruff.schema.json b/ruff.schema.json index ae2a8d4b1e86e..b35fcdc8daaba 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4040,6 +4040,7 @@ "RUF06", "RUF060", "RUF061", + "RUF063", "RUF064", "RUF1", "RUF10",