From 9a52075d128e2ec20e49469c5f4658b1f27ccde0 Mon Sep 17 00:00:00 2001 From: Steve C Date: Tue, 29 Oct 2024 23:49:17 -0400 Subject: [PATCH 1/5] [`pyupgrade`] - add PEP646 Unpack conversion to `*` (`UP044`) --- .../test/fixtures/pyupgrade/UP044.py | 11 +++ .../src/checkers/ast/analyze/expression.rs | 4 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pyupgrade/mod.rs | 14 ++++ .../src/rules/pyupgrade/rules/mod.rs | 2 + .../pyupgrade/rules/use_pep646_unpack.rs | 80 +++++++++++++++++++ ...yupgrade__tests__unpack_pep_646_py311.snap | 58 ++++++++++++++ ruff.schema.json | 1 + 8 files changed, 171 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py create mode 100644 crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py new file mode 100644 index 0000000000000..1abf6ff17a59d --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py @@ -0,0 +1,11 @@ +from typing import Generic, TypeVarTuple, Unpack + +Shape = TypeVarTuple('Shape') + +class C(Generic[Unpack[Shape]]): + pass + +class D(Generic[Unpack [Shape]]): + pass + +def f(*args: Unpack[tuple[int, ...]]): pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 99deb3b994998..a4c6b2bb56b75 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -150,6 +150,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { ruff::rules::subscript_with_parenthesized_tuple(checker, subscript); } + if checker.enabled(Rule::NonPEP646Unpack) { + pyupgrade::rules::use_pep646_unpack(checker, subscript); + } + pandas_vet::rules::subscript(checker, value, expr); } Expr::Tuple(ast::ExprTuple { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index caa4af3312fcc..38561b88d461e 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -528,6 +528,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pyupgrade, "041") => (RuleGroup::Stable, rules::pyupgrade::rules::TimeoutErrorAlias), (Pyupgrade, "042") => (RuleGroup::Preview, rules::pyupgrade::rules::ReplaceStrEnum), (Pyupgrade, "043") => (RuleGroup::Preview, rules::pyupgrade::rules::UnnecessaryDefaultTypeArgs), + (Pyupgrade, "044") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP646Unpack), // pydocstyle (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule), diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index bb13c8ecbe33d..950e53dee673c 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -235,4 +235,18 @@ mod tests { assert_messages!(diagnostics); Ok(()) } + + #[test] + fn unpack_pep_646_py311() -> Result<()> { + let diagnostics = test_path( + Path::new("pyupgrade/UP044.py"), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + target_version: PythonVersion::Py311, + ..settings::LinterSettings::for_rule(Rule::NonPEP646Unpack) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs index a3dbd706bf516..ac3ea97d30856 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs @@ -35,6 +35,7 @@ pub(crate) use unpacked_list_comprehension::*; pub(crate) use use_pep585_annotation::*; pub(crate) use use_pep604_annotation::*; pub(crate) use use_pep604_isinstance::*; +pub(crate) use use_pep646_unpack::*; pub(crate) use use_pep695_type_alias::*; pub(crate) use useless_metaclass_type::*; pub(crate) use useless_object_inheritance::*; @@ -77,6 +78,7 @@ mod unpacked_list_comprehension; mod use_pep585_annotation; mod use_pep604_annotation; mod use_pep604_isinstance; +mod use_pep646_unpack; mod use_pep695_type_alias; mod useless_metaclass_type; mod useless_object_inheritance; diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs new file mode 100644 index 0000000000000..2b20783d00eef --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs @@ -0,0 +1,80 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::ExprSubscript; + +use crate::{checkers::ast::Checker, settings::types::PythonVersion}; + +/// ## What it does +/// Checks for uses of `Unpack[]` on Python 3.11 and above, and suggests +/// using `*` instead. +/// +/// ## Why is this bad? +/// [PEP 646] introduced a new syntax for unpacking sequences based on the `*` +/// operator. This syntax is more concise and readable than the previous +/// `typing.Unpack` syntax. +/// +/// ## Example +/// ```python +/// from typing import Unpack +/// +/// +/// def foo(*args: Unpack[tuple[int, ...]]) -> None: pass +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(*args: *tuple[int, ...]) -> None: pass +/// ``` +/// +/// ## References +/// - [PEP 646](https://peps.python.org/pep-0646/#unpack-for-backwards-compatibility) +#[violation] +pub struct NonPEP646Unpack; + +impl Violation for NonPEP646Unpack { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Always; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Use `*` for unpacking") + } + + fn fix_title(&self) -> Option { + Some("Convert to `*` for unpacking".to_string()) + } +} + +/// UP044 +pub(crate) fn use_pep646_unpack(checker: &mut Checker, expr: &ExprSubscript) { + if checker.settings.target_version < PythonVersion::Py311 { + return; + } + + if !checker.semantic().seen_typing() { + return; + } + + let ExprSubscript { + range, + value, + slice, + .. + } = expr; + + if !checker.semantic().match_typing_expr(value, "Unpack") { + return; + } + + let mut diagnostic = Diagnostic::new(NonPEP646Unpack, *range); + + let inner = checker.locator().slice(slice.as_ref()); + + if checker.settings.preview.is_enabled() { + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + format!("*{inner}"), + *range, + ))); + } + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap new file mode 100644 index 0000000000000..60b11342b477a --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap @@ -0,0 +1,58 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP044.py:5:17: UP044 [*] Use `*` for unpacking + | +3 | Shape = TypeVarTuple('Shape') +4 | +5 | class C(Generic[Unpack[Shape]]): + | ^^^^^^^^^^^^^ UP044 +6 | pass + | + = help: Convert to `*` for unpacking + +ℹ Safe fix +2 2 | +3 3 | Shape = TypeVarTuple('Shape') +4 4 | +5 |-class C(Generic[Unpack[Shape]]): + 5 |+class C(Generic[*Shape]): +6 6 | pass +7 7 | +8 8 | class D(Generic[Unpack [Shape]]): + +UP044.py:8:17: UP044 [*] Use `*` for unpacking + | +6 | pass +7 | +8 | class D(Generic[Unpack [Shape]]): + | ^^^^^^^^^^^^^^^ UP044 +9 | pass + | + = help: Convert to `*` for unpacking + +ℹ Safe fix +5 5 | class C(Generic[Unpack[Shape]]): +6 6 | pass +7 7 | +8 |-class D(Generic[Unpack [Shape]]): + 8 |+class D(Generic[*Shape]): +9 9 | pass +10 10 | +11 11 | def f(*args: Unpack[tuple[int, ...]]): pass + +UP044.py:11:14: UP044 [*] Use `*` for unpacking + | + 9 | pass +10 | +11 | def f(*args: Unpack[tuple[int, ...]]): pass + | ^^^^^^^^^^^^^^^^^^^^^^^ UP044 + | + = help: Convert to `*` for unpacking + +ℹ Safe fix +8 8 | class D(Generic[Unpack [Shape]]): +9 9 | pass +10 10 | +11 |-def f(*args: Unpack[tuple[int, ...]]): pass + 11 |+def f(*args: *tuple[int, ...]): pass diff --git a/ruff.schema.json b/ruff.schema.json index 7412f47258dda..324f752e05df9 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4066,6 +4066,7 @@ "UP041", "UP042", "UP043", + "UP044", "W", "W1", "W19", From e746582b8950f00acac1ce09eef30d8cf0fb3125 Mon Sep 17 00:00:00 2001 From: Steve C Date: Tue, 29 Oct 2024 23:58:15 -0400 Subject: [PATCH 2/5] fix example --- .../src/rules/pyupgrade/rules/use_pep646_unpack.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs index 2b20783d00eef..41d06a250842e 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs @@ -14,16 +14,20 @@ use crate::{checkers::ast::Checker, settings::types::PythonVersion}; /// `typing.Unpack` syntax. /// /// ## Example +/// /// ```python /// from typing import Unpack /// /// -/// def foo(*args: Unpack[tuple[int, ...]]) -> None: pass +/// def foo(*args: Unpack[tuple[int, ...]]) -> None: +/// pass /// ``` /// /// Use instead: +/// /// ```python -/// def foo(*args: *tuple[int, ...]) -> None: pass +/// def foo(*args: *tuple[int, ...]) -> None: +/// pass /// ``` /// /// ## References From 8a10fd5aed3659451267e71e232aeb9b6df09fd7 Mon Sep 17 00:00:00 2001 From: Steve C Date: Wed, 30 Oct 2024 19:23:27 -0400 Subject: [PATCH 3/5] remove unneeded preview check --- .../src/rules/pyupgrade/rules/use_pep646_unpack.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs index 41d06a250842e..3f0d6aadd77d2 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs @@ -73,12 +73,10 @@ pub(crate) fn use_pep646_unpack(checker: &mut Checker, expr: &ExprSubscript) { let inner = checker.locator().slice(slice.as_ref()); - if checker.settings.preview.is_enabled() { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - format!("*{inner}"), - *range, - ))); - } + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + format!("*{inner}"), + *range, + ))); checker.diagnostics.push(diagnostic); } From 026ee0b3048e5d9455f71eb3e71dbebf36422afc Mon Sep 17 00:00:00 2001 From: Steve C Date: Wed, 30 Oct 2024 19:48:47 -0400 Subject: [PATCH 4/5] add binop early return --- .../ruff_linter/resources/test/fixtures/pyupgrade/UP044.py | 2 ++ .../src/rules/pyupgrade/rules/use_pep646_unpack.rs | 5 +++++ ...inter__rules__pyupgrade__tests__unpack_pep_646_py311.snap | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py index 1abf6ff17a59d..950d1d676b2f7 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py @@ -9,3 +9,5 @@ class D(Generic[Unpack [Shape]]): pass def f(*args: Unpack[tuple[int, ...]]): pass + +def foo(*args: Unpack[int | str]) -> None: pass # not supported diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs index 3f0d6aadd77d2..194f1c001cfe3 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs @@ -69,6 +69,11 @@ pub(crate) fn use_pep646_unpack(checker: &mut Checker, expr: &ExprSubscript) { return; } + if slice.is_bin_op_expr() { + // fixing this would introduce a syntax error + return; + } + let mut diagnostic = Diagnostic::new(NonPEP646Unpack, *range); let inner = checker.locator().slice(slice.as_ref()); diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap index 60b11342b477a..82272d8da724e 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap @@ -47,6 +47,8 @@ UP044.py:11:14: UP044 [*] Use `*` for unpacking 10 | 11 | def f(*args: Unpack[tuple[int, ...]]): pass | ^^^^^^^^^^^^^^^^^^^^^^^ UP044 +12 | +13 | def foo(*args: Unpack[int | str]) -> None: pass # not supported | = help: Convert to `*` for unpacking @@ -56,3 +58,5 @@ UP044.py:11:14: UP044 [*] Use `*` for unpacking 10 10 | 11 |-def f(*args: Unpack[tuple[int, ...]]): pass 11 |+def f(*args: *tuple[int, ...]): pass +12 12 | +13 13 | def foo(*args: Unpack[int | str]) -> None: pass # not supported From 970e2fc418c72fa1b93373984f72569051578164 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 31 Oct 2024 07:52:06 +0100 Subject: [PATCH 5/5] Use allowlist for slice value --- .../test/fixtures/pyupgrade/UP044.py | 8 +++++- .../pyupgrade/rules/use_pep646_unpack.rs | 4 +-- ...yupgrade__tests__unpack_pep_646_py311.snap | 25 +++++++++++++++++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py index 950d1d676b2f7..cd5aebe3780d1 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py @@ -10,4 +10,10 @@ class D(Generic[Unpack [Shape]]): def f(*args: Unpack[tuple[int, ...]]): pass -def foo(*args: Unpack[int | str]) -> None: pass # not supported +def f(*args: Unpack[other.Type]): pass + + +# Not valid unpackings but they are valid syntax +def foo(*args: Unpack[int | str]) -> None: pass +def foo(*args: Unpack[int and str]) -> None: pass +def foo(*args: Unpack[int > str]) -> None: pass diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs index 194f1c001cfe3..454b7b0c8b4e1 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs @@ -69,8 +69,8 @@ pub(crate) fn use_pep646_unpack(checker: &mut Checker, expr: &ExprSubscript) { return; } - if slice.is_bin_op_expr() { - // fixing this would introduce a syntax error + // Skip semantically invalid subscript calls (e.g. `Unpack[str | num]`). + if !(slice.is_name_expr() || slice.is_subscript_expr() || slice.is_attribute_expr()) { return; } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap index 82272d8da724e..6f5aba99dd76a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__unpack_pep_646_py311.snap @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +snapshot_kind: text --- UP044.py:5:17: UP044 [*] Use `*` for unpacking | @@ -48,7 +49,7 @@ UP044.py:11:14: UP044 [*] Use `*` for unpacking 11 | def f(*args: Unpack[tuple[int, ...]]): pass | ^^^^^^^^^^^^^^^^^^^^^^^ UP044 12 | -13 | def foo(*args: Unpack[int | str]) -> None: pass # not supported +13 | def f(*args: Unpack[other.Type]): pass | = help: Convert to `*` for unpacking @@ -59,4 +60,24 @@ UP044.py:11:14: UP044 [*] Use `*` for unpacking 11 |-def f(*args: Unpack[tuple[int, ...]]): pass 11 |+def f(*args: *tuple[int, ...]): pass 12 12 | -13 13 | def foo(*args: Unpack[int | str]) -> None: pass # not supported +13 13 | def f(*args: Unpack[other.Type]): pass +14 14 | + +UP044.py:13:14: UP044 [*] Use `*` for unpacking + | +11 | def f(*args: Unpack[tuple[int, ...]]): pass +12 | +13 | def f(*args: Unpack[other.Type]): pass + | ^^^^^^^^^^^^^^^^^^ UP044 + | + = help: Convert to `*` for unpacking + +ℹ Safe fix +10 10 | +11 11 | def f(*args: Unpack[tuple[int, ...]]): pass +12 12 | +13 |-def f(*args: Unpack[other.Type]): pass + 13 |+def f(*args: *other.Type): pass +14 14 | +15 15 | +16 16 | # Not valid unpackings but they are valid syntax