From c45187aba8730dda99db956f148ad2fdaf36e98a Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Tue, 30 May 2023 01:41:45 +0300 Subject: [PATCH] [`flake8-future-annotations`] Implement `FA102` (#4702) --- crates/ruff/src/checkers/ast/mod.rs | 47 ++++++++--- crates/ruff/src/codes.rs | 3 +- crates/ruff/src/registry.rs | 3 +- .../rules/flake8_future_annotations/mod.rs | 22 +++++- .../missing_future_annotations_new_style.rs | 77 +++++++++++++++++++ ...> missing_future_annotations_old_style.rs} | 25 +++--- .../flake8_future_annotations/rules/mod.rs | 10 ++- ...02_no_future_import_uses_lowercase.py.snap | 12 +++ ..._fa102_no_future_import_uses_union.py.snap | 20 +++++ ..._no_future_import_uses_union_inner.py.snap | 36 +++++++++ ...otations__tests__fa102_ok_no_types.py.snap | 4 + ...tions__tests__fa102_ok_uses_future.py.snap | 4 + .../src/analyze/typing.rs | 15 ++++ ruff.schema.json | 1 + 14 files changed, 249 insertions(+), 30 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_new_style.rs rename crates/ruff/src/rules/flake8_future_annotations/rules/{missing_future_annotations.rs => missing_future_annotations_old_style.rs} (73%) create mode 100644 crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap create mode 100644 crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap create mode 100644 crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap create mode 100644 crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_no_types.py.snap create mode 100644 crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_uses_future.py.snap diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 98850c2b1ed2a9..2148dca61edd0e 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2184,19 +2184,19 @@ where Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { // Ex) Optional[...], Union[...] if self.any_enabled(&[ - Rule::MissingFutureAnnotationsImport, + Rule::MissingFutureAnnotationsImportOldStyle, Rule::NonPEP604Annotation, ]) { if let Some(operator) = analyze::typing::to_pep604_operator(value, slice, &self.semantic_model) { - if self.enabled(Rule::MissingFutureAnnotationsImport) { + if self.enabled(Rule::MissingFutureAnnotationsImportOldStyle) { if self.settings.target_version < PythonVersion::Py310 && self.settings.target_version >= PythonVersion::Py37 && !self.semantic_model.future_annotations() && self.semantic_model.in_annotation() { - flake8_future_annotations::rules::missing_future_annotations( + flake8_future_annotations::rules::missing_future_annotations_old_style( self, value, ); } @@ -2215,6 +2215,21 @@ where } } + // Ex) list[...] + if self.enabled(Rule::MissingFutureAnnotationsImportNewStyle) { + if self.settings.target_version < PythonVersion::Py39 + && !self.semantic_model.future_annotations() + && self.semantic_model.in_annotation() + && analyze::typing::is_pep585_generic(value, &self.semantic_model) + { + flake8_future_annotations::rules::missing_future_annotations_new_style( + self, + expr, + flake8_future_annotations::rules::Reason::PEP585, + ); + } + } + if self.semantic_model.match_typing_expr(value, "Literal") { self.semantic_model.flags |= SemanticModelFlags::LITERAL; } @@ -2271,19 +2286,19 @@ where // Ex) List[...] if self.any_enabled(&[ - Rule::MissingFutureAnnotationsImport, + Rule::MissingFutureAnnotationsImportOldStyle, Rule::NonPEP585Annotation, ]) { if let Some(replacement) = analyze::typing::to_pep585_generic(expr, &self.semantic_model) { - if self.enabled(Rule::MissingFutureAnnotationsImport) { + if self.enabled(Rule::MissingFutureAnnotationsImportOldStyle) { if self.settings.target_version < PythonVersion::Py39 && self.settings.target_version >= PythonVersion::Py37 && !self.semantic_model.future_annotations() && self.semantic_model.in_annotation() { - flake8_future_annotations::rules::missing_future_annotations( + flake8_future_annotations::rules::missing_future_annotations_old_style( self, expr, ); } @@ -2349,19 +2364,19 @@ where Expr::Attribute(ast::ExprAttribute { attr, value, .. }) => { // Ex) typing.List[...] if self.any_enabled(&[ - Rule::MissingFutureAnnotationsImport, + Rule::MissingFutureAnnotationsImportOldStyle, Rule::NonPEP585Annotation, ]) { if let Some(replacement) = analyze::typing::to_pep585_generic(expr, &self.semantic_model) { - if self.enabled(Rule::MissingFutureAnnotationsImport) { + if self.enabled(Rule::MissingFutureAnnotationsImportOldStyle) { if self.settings.target_version < PythonVersion::Py39 && self.settings.target_version >= PythonVersion::Py37 && !self.semantic_model.future_annotations() && self.semantic_model.in_annotation() { - flake8_future_annotations::rules::missing_future_annotations( + flake8_future_annotations::rules::missing_future_annotations_old_style( self, expr, ); } @@ -3198,6 +3213,20 @@ where op: Operator::BitOr, .. }) => { + // Ex) `str | None` + if self.enabled(Rule::MissingFutureAnnotationsImportNewStyle) { + if self.settings.target_version < PythonVersion::Py310 + && !self.semantic_model.future_annotations() + && self.semantic_model.in_annotation() + { + flake8_future_annotations::rules::missing_future_annotations_new_style( + self, + expr, + flake8_future_annotations::rules::Reason::PEP604, + ); + } + } + if self.is_stub { if self.enabled(Rule::DuplicateUnionMember) && self.semantic_model.in_type_definition() diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index c6412c142b5926..84679a7419dca5 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -318,7 +318,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Annotations, "401") => (RuleGroup::Unspecified, Rule::AnyType), // flake8-future-annotations - (Flake8FutureAnnotations, "100") => (RuleGroup::Unspecified, Rule::MissingFutureAnnotationsImport), + (Flake8FutureAnnotations, "100") => (RuleGroup::Unspecified, Rule::MissingFutureAnnotationsImportOldStyle), + (Flake8FutureAnnotations, "102") => (RuleGroup::Unspecified, Rule::MissingFutureAnnotationsImportNewStyle), // flake8-2020 (Flake82020, "101") => (RuleGroup::Unspecified, Rule::SysVersionSlice3), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index d4fafa4c35c826..c44fe60b78a1c7 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -267,7 +267,8 @@ ruff_macros::register_rules!( rules::flake8_annotations::rules::MissingReturnTypeClassMethod, rules::flake8_annotations::rules::AnyType, // flake8-future-annotations - rules::flake8_future_annotations::rules::MissingFutureAnnotationsImport, + rules::flake8_future_annotations::rules::MissingFutureAnnotationsImportOldStyle, + rules::flake8_future_annotations::rules::MissingFutureAnnotationsImportNewStyle, // flake8-2020 rules::flake8_2020::rules::SysVersionSlice3, rules::flake8_2020::rules::SysVersion2, diff --git a/crates/ruff/src/rules/flake8_future_annotations/mod.rs b/crates/ruff/src/rules/flake8_future_annotations/mod.rs index fe5cb20ed644c6..d052183105a52f 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/mod.rs +++ b/crates/ruff/src/rules/flake8_future_annotations/mod.rs @@ -25,13 +25,31 @@ mod tests { #[test_case(Path::new("ok_non_simplifiable_types.py"))] #[test_case(Path::new("ok_uses_future.py"))] #[test_case(Path::new("ok_variable_name.py"))] - fn rules(path: &Path) -> Result<()> { + fn fa100(path: &Path) -> Result<()> { let snapshot = path.to_string_lossy().into_owned(); let diagnostics = test_path( Path::new("flake8_future_annotations").join(path).as_path(), &settings::Settings { target_version: PythonVersion::Py37, - ..settings::Settings::for_rule(Rule::MissingFutureAnnotationsImport) + ..settings::Settings::for_rule(Rule::MissingFutureAnnotationsImportOldStyle) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + + #[test_case(Path::new("no_future_import_uses_lowercase.py"))] + #[test_case(Path::new("no_future_import_uses_union.py"))] + #[test_case(Path::new("no_future_import_uses_union_inner.py"))] + #[test_case(Path::new("ok_no_types.py"))] + #[test_case(Path::new("ok_uses_future.py"))] + fn fa102(path: &Path) -> Result<()> { + let snapshot = format!("fa102_{}", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_future_annotations").join(path).as_path(), + &settings::Settings { + target_version: PythonVersion::Py37, + ..settings::Settings::for_rule(Rule::MissingFutureAnnotationsImportNewStyle) }, )?; assert_messages!(snapshot, diagnostics); diff --git a/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_new_style.rs b/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_new_style.rs new file mode 100644 index 00000000000000..20a4806972187a --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_new_style.rs @@ -0,0 +1,77 @@ +use rustpython_parser::ast::{Expr, Ranged}; +use std::fmt; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of PEP 585- and PEP 604-style type annotations in Python +/// modules that lack the required `from __future__ import annotations` import +/// for compatibility with older Python versions. +/// +/// ## Why is this bad? +/// Using PEP 585 and PEP 604 style annotations without a `from __future__ import +/// annotations` import will cause runtime errors on Python versions prior to +/// 3.9 and 3.10, respectively. +/// +/// By adding the `__future__` import, the interpreter will no longer interpret +/// annotations at evaluation time, making the code compatible with both past +/// and future Python versions. +/// +/// ## Example +/// ```python +/// def func(obj: dict[str, int | None]) -> None: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// from __future__ import annotations +/// +/// +/// def func(obj: dict[str, int | None]) -> None: +/// ... +/// ``` +#[violation] +pub struct MissingFutureAnnotationsImportNewStyle { + reason: Reason, +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum Reason { + /// The type annotation is written in PEP 585 style (e.g., `list[int]`). + PEP585, + /// The type annotation is written in PEP 604 style (e.g., `int | None`). + PEP604, +} + +impl fmt::Display for Reason { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + Reason::PEP585 => fmt.write_str("PEP 585 collection"), + Reason::PEP604 => fmt.write_str("PEP 604 union"), + } + } +} + +impl Violation for MissingFutureAnnotationsImportNewStyle { + #[derive_message_formats] + fn message(&self) -> String { + let MissingFutureAnnotationsImportNewStyle { reason } = self; + format!("Missing `from __future__ import annotations`, but uses {reason}") + } +} + +/// FA102 +pub(crate) fn missing_future_annotations_new_style( + checker: &mut Checker, + expr: &Expr, + reason: Reason, +) { + checker.diagnostics.push(Diagnostic::new( + MissingFutureAnnotationsImportNewStyle { reason }, + expr.range(), + )); +} diff --git a/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations.rs b/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_old_style.rs similarity index 73% rename from crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations.rs rename to crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_old_style.rs index 28aebb80743f68..5928fcf872a3bf 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations.rs +++ b/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations_old_style.rs @@ -26,9 +26,8 @@ use crate::checkers::ast::Checker; /// from typing import List, Dict, Optional /// /// -/// def function(a_dict: Dict[str, Optional[int]]) -> None: -/// a_list: List[str] = [] -/// a_list.append("hello") +/// def func(obj: Dict[str, Optional[int]]) -> None: +/// ... /// ``` /// /// Use instead: @@ -38,9 +37,8 @@ use crate::checkers::ast::Checker; /// from typing import List, Dict, Optional /// /// -/// def function(a_dict: Dict[str, Optional[int]]) -> None: -/// a_list: List[str] = [] -/// a_list.append("hello") +/// def func(obj: Dict[str, Optional[int]]) -> None: +/// ... /// ``` /// /// After running the additional pyupgrade rules: @@ -48,25 +46,24 @@ use crate::checkers::ast::Checker; /// from __future__ import annotations /// /// -/// def function(a_dict: dict[str, int | None]) -> None: -/// a_list: list[str] = [] -/// a_list.append("hello") +/// def func(obj: dict[str, int | None]) -> None: +/// ... /// ``` #[violation] -pub struct MissingFutureAnnotationsImport { +pub struct MissingFutureAnnotationsImportOldStyle { name: String, } -impl Violation for MissingFutureAnnotationsImport { +impl Violation for MissingFutureAnnotationsImportOldStyle { #[derive_message_formats] fn message(&self) -> String { - let MissingFutureAnnotationsImport { name } = self; + let MissingFutureAnnotationsImportOldStyle { name } = self; format!("Missing `from __future__ import annotations`, but uses `{name}`") } } /// FA100 -pub(crate) fn missing_future_annotations(checker: &mut Checker, expr: &Expr) { +pub(crate) fn missing_future_annotations_old_style(checker: &mut Checker, expr: &Expr) { let name = checker .semantic_model() .resolve_call_path(expr) @@ -74,7 +71,7 @@ pub(crate) fn missing_future_annotations(checker: &mut Checker, expr: &Expr) { if let Some(name) = name { checker.diagnostics.push(Diagnostic::new( - MissingFutureAnnotationsImport { name }, + MissingFutureAnnotationsImportOldStyle { name }, expr.range(), )); } diff --git a/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs b/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs index 5a845f4b8a4d9c..b578c5e8224261 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs @@ -1,5 +1,9 @@ -pub(crate) use missing_future_annotations::{ - missing_future_annotations, MissingFutureAnnotationsImport, +pub(crate) use missing_future_annotations_new_style::{ + missing_future_annotations_new_style, MissingFutureAnnotationsImportNewStyle, Reason, +}; +pub(crate) use missing_future_annotations_old_style::{ + missing_future_annotations_old_style, MissingFutureAnnotationsImportOldStyle, }; -mod missing_future_annotations; +mod missing_future_annotations_new_style; +mod missing_future_annotations_old_style; diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap new file mode 100644 index 00000000000000..4cf41d1f0cea79 --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap @@ -0,0 +1,12 @@ +--- +source: crates/ruff/src/rules/flake8_future_annotations/mod.rs +--- +no_future_import_uses_lowercase.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +2 | def main() -> None: +3 | a_list: list[str] = [] + | ^^^^^^^^^ FA102 +4 | a_list.append("hello") + | + + diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap new file mode 100644 index 00000000000000..64749b8034251d --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff/src/rules/flake8_future_annotations/mod.rs +--- +no_future_import_uses_union.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union + | +2 | def main() -> None: +3 | a_list: list[str] | None = [] + | ^^^^^^^^^^^^^^^^ FA102 +4 | a_list.append("hello") + | + +no_future_import_uses_union.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +2 | def main() -> None: +3 | a_list: list[str] | None = [] + | ^^^^^^^^^ FA102 +4 | a_list.append("hello") + | + + diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap new file mode 100644 index 00000000000000..3f0d26fc63d551 --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap @@ -0,0 +1,36 @@ +--- +source: crates/ruff/src/rules/flake8_future_annotations/mod.rs +--- +no_future_import_uses_union_inner.py:2:13: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +2 | def main() -> None: +3 | a_list: list[str | None] = [] + | ^^^^^^^^^^^^^^^^ FA102 +4 | a_list.append("hello") + | + +no_future_import_uses_union_inner.py:2:18: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union + | +2 | def main() -> None: +3 | a_list: list[str | None] = [] + | ^^^^^^^^^^ FA102 +4 | a_list.append("hello") + | + +no_future_import_uses_union_inner.py:7:8: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +7 | def hello(y: dict[str | None, int]) -> None: +8 | z: tuple[str, str | None, str] = tuple(y) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FA102 +9 | del z + | + +no_future_import_uses_union_inner.py:7:19: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union + | +7 | def hello(y: dict[str | None, int]) -> None: +8 | z: tuple[str, str | None, str] = tuple(y) + | ^^^^^^^^^^ FA102 +9 | del z + | + + diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_no_types.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_no_types.py.snap new file mode 100644 index 00000000000000..681d4be5e53381 --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_no_types.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_future_annotations/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_uses_future.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_uses_future.py.snap new file mode 100644 index 00000000000000..681d4be5e53381 --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_ok_uses_future.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_future_annotations/mod.rs +--- + diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 8bbde2990b1018..07001551da53c4 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -101,6 +101,21 @@ pub fn to_pep585_generic(expr: &Expr, model: &SemanticModel) -> Option bool { + if let Some(call_path) = model.resolve_call_path(expr) { + let [module, name] = call_path.as_slice() else { + return false; + }; + for (_, (to_module, to_member)) in PEP_585_GENERICS { + if module == to_module && name == to_member { + return true; + } + } + } + false +} + #[derive(Debug, Copy, Clone)] pub enum Pep604Operator { /// The union operator, e.g., `Union[str, int]`, expressible as `str | int` after PEP 604. diff --git a/ruff.schema.json b/ruff.schema.json index b963b9272dc982..31550f9dcb6807 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1883,6 +1883,7 @@ "FA1", "FA10", "FA100", + "FA102", "FBT", "FBT0", "FBT00",