From 1e184e69f305245d7e14083267eb8b43ba6783a7 Mon Sep 17 00:00:00 2001 From: Steve C Date: Thu, 12 Oct 2023 20:56:34 -0400 Subject: [PATCH] Add autofix for `PYI055` (#7886) --- .../test/fixtures/flake8_pyi/PYI055.py | 10 +- .../test/fixtures/flake8_pyi/PYI055.pyi | 3 +- .../rules/unnecessary_type_union.rs | 121 ++++++++++++++--- ...__flake8_pyi__tests__PYI055_PYI055.py.snap | 52 +++++++- ..._flake8_pyi__tests__PYI055_PYI055.pyi.snap | 123 ++++++++++++++++-- 5 files changed, 277 insertions(+), 32 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py index adbc1f737a..6471613f98 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py @@ -28,4 +28,12 @@ item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker def func(): # PYI055 - item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker + x: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker + y: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + + +def func(): + from typing import Union as U + + # PYI055 + x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.pyi index 3cc530f770..530f395dfa 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.pyi @@ -21,4 +21,5 @@ item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker def func(): # PYI055 - item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker + item: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker + item2: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs index 4eb4e8de3a..75ba6ab9bc 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs @@ -1,7 +1,8 @@ -use ruff_diagnostics::{Diagnostic, Violation}; +use ast::{ExprContext, Operator}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::Expr; -use ruff_text_size::Ranged; +use ruff_python_ast::{self as ast, Expr}; +use ruff_text_size::{Ranged, TextRange}; use crate::{checkers::ast::Checker, rules::flake8_pyi::helpers::traverse_union}; @@ -9,8 +10,8 @@ use crate::{checkers::ast::Checker, rules::flake8_pyi::helpers::traverse_union}; /// Checks for the presence of multiple `type`s in a union. /// /// ## Why is this bad? -/// The `type` built-in function accepts unions, and it is -/// clearer to explicitly specify them as a single `type`. +/// The `type` built-in function accepts unions, and it is clearer to +/// explicitly specify them as a single `type`. /// /// ## Example /// ```python @@ -28,6 +29,8 @@ pub struct UnnecessaryTypeUnion { } impl Violation for UnnecessaryTypeUnion { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { let union_str = if self.is_pep604_union { @@ -40,6 +43,23 @@ impl Violation for UnnecessaryTypeUnion { "Multiple `type` members in a union. Combine them into one, e.g., `type[{union_str}]`." ) } + + fn fix_title(&self) -> Option { + Some(format!("Combine multiple `type` members")) + } +} + +fn concatenate_bin_ors(exprs: Vec<&Expr>) -> Expr { + let mut exprs = exprs.into_iter(); + let first = exprs.next().unwrap(); + exprs.fold((*first).clone(), |acc, expr| { + Expr::BinOp(ast::ExprBinOp { + left: Box::new(acc), + op: Operator::BitOr, + right: Box::new((*expr).clone()), + range: TextRange::default(), + }) + }) } /// PYI055 @@ -49,14 +69,17 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) return; } - let mut type_exprs = Vec::new(); - // Check if `union` is a PEP604 union (e.g. `float | int`) or a `typing.Union[float, int]` - let is_pep604_union = !union.as_subscript_expr().is_some_and(|subscript| { - checker + let subscript = union.as_subscript_expr(); + if subscript.is_some_and(|subscript| { + !checker .semantic() .match_typing_expr(&subscript.value, "Union") - }); + }) { + return; + } + + let mut type_exprs = Vec::new(); let mut collect_type_exprs = |expr: &'a Expr, _| { let Some(subscript) = expr.as_subscript_expr() else { @@ -74,15 +97,79 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) traverse_union(&mut collect_type_exprs, checker.semantic(), union, None); if type_exprs.len() > 1 { - checker.diagnostics.push(Diagnostic::new( + let type_members: Vec = type_exprs + .clone() + .into_iter() + .map(|type_expr| checker.locator().slice(type_expr.as_ref()).to_string()) + .collect(); + + let mut diagnostic = Diagnostic::new( UnnecessaryTypeUnion { - members: type_exprs - .into_iter() - .map(|type_expr| checker.locator().slice(type_expr.as_ref()).to_string()) - .collect(), - is_pep604_union, + members: type_members.clone(), + is_pep604_union: subscript.is_none(), }, union.range(), - )); + ); + + if checker.semantic().is_builtin("type") { + let content = if let Some(subscript) = subscript { + checker + .generator() + .expr(&Expr::Subscript(ast::ExprSubscript { + value: Box::new(Expr::Name(ast::ExprName { + id: "type".into(), + ctx: ExprContext::Load, + range: TextRange::default(), + })), + slice: Box::new(Expr::Subscript(ast::ExprSubscript { + value: subscript.value.clone(), + slice: Box::new(Expr::Tuple(ast::ExprTuple { + elts: type_members + .into_iter() + .map(|type_member| { + Expr::Name(ast::ExprName { + id: type_member, + ctx: ExprContext::Load, + range: TextRange::default(), + }) + }) + .collect(), + ctx: ExprContext::Load, + range: TextRange::default(), + })), + ctx: ExprContext::Load, + range: TextRange::default(), + })), + ctx: ExprContext::Load, + range: TextRange::default(), + })) + } else { + checker + .generator() + .expr(&Expr::Subscript(ast::ExprSubscript { + value: Box::new(Expr::Name(ast::ExprName { + id: "type".into(), + ctx: ExprContext::Load, + range: TextRange::default(), + })), + slice: Box::new(concatenate_bin_ors( + type_exprs + .clone() + .into_iter() + .map(std::convert::AsRef::as_ref) + .collect(), + )), + ctx: ExprContext::Load, + range: TextRange::default(), + })) + }; + + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + content, + union.range(), + ))); + } + + checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap index 8addf09918..691a671b4e 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap @@ -1,12 +1,58 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs --- -PYI055.py:31:11: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty]`. +PYI055.py:31:8: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty | str]`. | 29 | def func(): 30 | # PYI055 -31 | item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +31 | x: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +32 | y: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker | + = help: Combine multiple `type` members + +ℹ Fix +28 28 | +29 29 | def func(): +30 30 | # PYI055 +31 |- x: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker + 31 |+ x: type[requests_mock.Mocker | httpretty | str] = requests_mock.Mocker +32 32 | y: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker +33 33 | +34 34 | + +PYI055.py:32:8: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[Union[requests_mock.Mocker, httpretty, str]]`. + | +30 | # PYI055 +31 | x: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker +32 | y: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 + | + = help: Combine multiple `type` members + +ℹ Fix +29 29 | def func(): +30 30 | # PYI055 +31 31 | x: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker +32 |- y: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + 32 |+ y: type[Union[requests_mock.Mocker, httpretty, str]] = requests_mock.Mocker +33 33 | +34 34 | +35 35 | def func(): + +PYI055.py:39:8: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[Union[requests_mock.Mocker, httpretty, str]]`. + | +38 | # PYI055 +39 | x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 + | + = help: Combine multiple `type` members + +ℹ Fix +36 36 | from typing import Union as U +37 37 | +38 38 | # PYI055 +39 |- x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + 39 |+ x: type[Union[requests_mock.Mocker, httpretty, str]] = requests_mock.Mocker diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap index f28020e400..c8fc0694ea 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs --- -PYI055.pyi:4:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. +PYI055.pyi:4:4: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. | 2 | from typing import Union 3 | @@ -10,8 +10,19 @@ PYI055.pyi:4:4: PYI055 Multiple `type` members in a union. Combine them into one 5 | x: type[int] | type[str] | type[float] 6 | y: builtins.type[int] | type[str] | builtins.type[complex] | + = help: Combine multiple `type` members -PYI055.pyi:5:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | float]`. +ℹ Fix +1 1 | import builtins +2 2 | from typing import Union +3 3 | +4 |-w: builtins.type[int] | builtins.type[str] | builtins.type[complex] + 4 |+w: type[int | str | complex] +5 5 | x: type[int] | type[str] | type[float] +6 6 | y: builtins.type[int] | type[str] | builtins.type[complex] +7 7 | z: Union[type[float], type[complex]] + +PYI055.pyi:5:4: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | float]`. | 4 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] 5 | x: type[int] | type[str] | type[float] @@ -19,8 +30,19 @@ PYI055.pyi:5:4: PYI055 Multiple `type` members in a union. Combine them into one 6 | y: builtins.type[int] | type[str] | builtins.type[complex] 7 | z: Union[type[float], type[complex]] | + = help: Combine multiple `type` members -PYI055.pyi:6:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. +ℹ Fix +2 2 | from typing import Union +3 3 | +4 4 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +5 |-x: type[int] | type[str] | type[float] + 5 |+x: type[int | str | float] +6 6 | y: builtins.type[int] | type[str] | builtins.type[complex] +7 7 | z: Union[type[float], type[complex]] +8 8 | z: Union[type[float, int], type[complex]] + +PYI055.pyi:6:4: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. | 4 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] 5 | x: type[int] | type[str] | type[float] @@ -29,8 +51,19 @@ PYI055.pyi:6:4: PYI055 Multiple `type` members in a union. Combine them into one 7 | z: Union[type[float], type[complex]] 8 | z: Union[type[float, int], type[complex]] | + = help: Combine multiple `type` members -PYI055.pyi:7:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, complex]]`. +ℹ Fix +3 3 | +4 4 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +5 5 | x: type[int] | type[str] | type[float] +6 |-y: builtins.type[int] | type[str] | builtins.type[complex] + 6 |+y: type[int | str | complex] +7 7 | z: Union[type[float], type[complex]] +8 8 | z: Union[type[float, int], type[complex]] +9 9 | + +PYI055.pyi:7:4: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, complex]]`. | 5 | x: type[int] | type[str] | type[float] 6 | y: builtins.type[int] | type[str] | builtins.type[complex] @@ -38,8 +71,19 @@ PYI055.pyi:7:4: PYI055 Multiple `type` members in a union. Combine them into one | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 8 | z: Union[type[float, int], type[complex]] | + = help: Combine multiple `type` members -PYI055.pyi:8:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, int, complex]]`. +ℹ Fix +4 4 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +5 5 | x: type[int] | type[str] | type[float] +6 6 | y: builtins.type[int] | type[str] | builtins.type[complex] +7 |-z: Union[type[float], type[complex]] + 7 |+z: type[Union[float, complex]] +8 8 | z: Union[type[float, int], type[complex]] +9 9 | +10 10 | def func(arg: type[int] | str | type[float]) -> None: ... + +PYI055.pyi:8:4: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, int, complex]]`. | 6 | y: builtins.type[int] | type[str] | builtins.type[complex] 7 | z: Union[type[float], type[complex]] @@ -48,8 +92,19 @@ PYI055.pyi:8:4: PYI055 Multiple `type` members in a union. Combine them into one 9 | 10 | def func(arg: type[int] | str | type[float]) -> None: ... | + = help: Combine multiple `type` members -PYI055.pyi:10:15: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | float]`. +ℹ Fix +5 5 | x: type[int] | type[str] | type[float] +6 6 | y: builtins.type[int] | type[str] | builtins.type[complex] +7 7 | z: Union[type[float], type[complex]] +8 |-z: Union[type[float, int], type[complex]] + 8 |+z: type[Union[float, int, complex]] +9 9 | +10 10 | def func(arg: type[int] | str | type[float]) -> None: ... +11 11 | + +PYI055.pyi:10:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[int | float]`. | 8 | z: Union[type[float, int], type[complex]] 9 | @@ -58,8 +113,19 @@ PYI055.pyi:10:15: PYI055 Multiple `type` members in a union. Combine them into o 11 | 12 | # OK | + = help: Combine multiple `type` members -PYI055.pyi:20:7: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty]`. +ℹ Fix +7 7 | z: Union[type[float], type[complex]] +8 8 | z: Union[type[float, int], type[complex]] +9 9 | +10 |-def func(arg: type[int] | str | type[float]) -> None: ... + 10 |+def func(arg: type[int | float]) -> None: ... +11 11 | +12 12 | # OK +13 13 | x: type[int, str, float] + +PYI055.pyi:20:7: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty]`. | 19 | # OK 20 | item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker @@ -67,13 +133,50 @@ PYI055.pyi:20:7: PYI055 Multiple `type` members in a union. Combine them into on 21 | 22 | def func(): | + = help: Combine multiple `type` members -PYI055.pyi:24:11: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty]`. +ℹ Fix +17 17 | def func(arg: type[int, float] | str) -> None: ... +18 18 | +19 19 | # OK +20 |-item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker + 20 |+item: type[requests_mock.Mocker | httpretty] = requests_mock.Mocker +21 21 | +22 22 | def func(): +23 23 | # PYI055 + +PYI055.pyi:24:11: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty | str]`. | 22 | def func(): 23 | # PYI055 -24 | item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +24 | item: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +25 | item2: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker | + = help: Combine multiple `type` members + +ℹ Fix +21 21 | +22 22 | def func(): +23 23 | # PYI055 +24 |- item: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker + 24 |+ item: type[requests_mock.Mocker | httpretty | str] = requests_mock.Mocker +25 25 | item2: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + +PYI055.pyi:25:12: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[Union[requests_mock.Mocker, httpretty, str]]`. + | +23 | # PYI055 +24 | item: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker +25 | item2: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 + | + = help: Combine multiple `type` members + +ℹ Fix +22 22 | def func(): +23 23 | # PYI055 +24 24 | item: type[requests_mock.Mocker] | type[httpretty] | type[str] = requests_mock.Mocker +25 |- item2: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + 25 |+ item2: type[Union[requests_mock.Mocker, httpretty, str]] = requests_mock.Mocker