diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C416.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C416.py index 6ca19ee574671..2e554bdb8170e 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C416.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C416.py @@ -7,6 +7,8 @@ {i for i in x} {k: v for k, v in y} {k: v for k, v in d.items()} +[(k, v) for k, v in d.items()] +{k: (a, b) for k, (a, b) in d.items()} [i for i, in z] [i for i, j in y] diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs index d9095cda499aa..a31ed5fa92d15 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs @@ -1,5 +1,6 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::{self as ast, Comprehension, Expr}; use ruff_text_size::Ranged; @@ -86,28 +87,16 @@ pub(crate) fn unnecessary_dict_comprehension( if !generator.ifs.is_empty() || generator.is_async { return; } - let Some(key) = key.as_name_expr() else { - return; - }; - let Some(value) = value.as_name_expr() else { - return; - }; let Expr::Tuple(ast::ExprTuple { elts, .. }) = &generator.target else { return; }; let [target_key, target_value] = elts.as_slice() else { return; }; - let Some(target_key) = target_key.as_name_expr() else { - return; - }; - let Some(target_value) = target_value.as_name_expr() else { - return; - }; - if target_key.id != key.id { + if ComparableExpr::from(key) != ComparableExpr::from(target_key) { return; } - if target_value.id != value.id { + if ComparableExpr::from(value) != ComparableExpr::from(target_value) { return; } add_diagnostic(checker, expr); @@ -126,13 +115,7 @@ pub(crate) fn unnecessary_list_set_comprehension( if !generator.ifs.is_empty() || generator.is_async { return; } - let Some(elt) = elt.as_name_expr() else { - return; - }; - let Some(target) = generator.target.as_name_expr() else { - return; - }; - if elt.id != target.id { + if ComparableExpr::from(elt) != ComparableExpr::from(&generator.target) { return; } add_diagnostic(checker, expr); diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C416_C416.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C416_C416.py.snap index 7268cef9f7778..96188e987e655 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C416_C416.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C416_C416.py.snap @@ -40,17 +40,18 @@ C416.py:7:1: C416 [*] Unnecessary `set` comprehension (rewrite using `set()`) 7 |+set(x) 8 8 | {k: v for k, v in y} 9 9 | {k: v for k, v in d.items()} -10 10 | +10 10 | [(k, v) for k, v in d.items()] C416.py:8:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) - | -6 | [i for i in x] -7 | {i for i in x} -8 | {k: v for k, v in y} - | ^^^^^^^^^^^^^^^^^^^^ C416 -9 | {k: v for k, v in d.items()} - | - = help: Rewrite using `dict()` + | + 6 | [i for i in x] + 7 | {i for i in x} + 8 | {k: v for k, v in y} + | ^^^^^^^^^^^^^^^^^^^^ C416 + 9 | {k: v for k, v in d.items()} +10 | [(k, v) for k, v in d.items()] + | + = help: Rewrite using `dict()` ℹ Suggested fix 5 5 | @@ -59,8 +60,8 @@ C416.py:8:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) 8 |-{k: v for k, v in y} 8 |+dict(y) 9 9 | {k: v for k, v in d.items()} -10 10 | -11 11 | [i for i, in z] +10 10 | [(k, v) for k, v in d.items()] +11 11 | {k: (a, b) for k, (a, b) in d.items()} C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) | @@ -68,8 +69,8 @@ C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) 8 | {k: v for k, v in y} 9 | {k: v for k, v in d.items()} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416 -10 | -11 | [i for i, in z] +10 | [(k, v) for k, v in d.items()] +11 | {k: (a, b) for k, (a, b) in d.items()} | = help: Rewrite using `dict()` @@ -79,23 +80,64 @@ C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) 8 8 | {k: v for k, v in y} 9 |-{k: v for k, v in d.items()} 9 |+dict(d.items()) -10 10 | -11 11 | [i for i, in z] -12 12 | [i for i, j in y] +10 10 | [(k, v) for k, v in d.items()] +11 11 | {k: (a, b) for k, (a, b) in d.items()} +12 12 | + +C416.py:10:1: C416 [*] Unnecessary `list` comprehension (rewrite using `list()`) + | + 8 | {k: v for k, v in y} + 9 | {k: v for k, v in d.items()} +10 | [(k, v) for k, v in d.items()] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416 +11 | {k: (a, b) for k, (a, b) in d.items()} + | + = help: Rewrite using `list()` + +ℹ Suggested fix +7 7 | {i for i in x} +8 8 | {k: v for k, v in y} +9 9 | {k: v for k, v in d.items()} +10 |-[(k, v) for k, v in d.items()] + 10 |+list(d.items()) +11 11 | {k: (a, b) for k, (a, b) in d.items()} +12 12 | +13 13 | [i for i, in z] + +C416.py:11:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) + | + 9 | {k: v for k, v in d.items()} +10 | [(k, v) for k, v in d.items()] +11 | {k: (a, b) for k, (a, b) in d.items()} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416 +12 | +13 | [i for i, in z] + | + = help: Rewrite using `dict()` + +ℹ Suggested fix +8 8 | {k: v for k, v in y} +9 9 | {k: v for k, v in d.items()} +10 10 | [(k, v) for k, v in d.items()] +11 |-{k: (a, b) for k, (a, b) in d.items()} + 11 |+dict(d.items()) +12 12 | +13 13 | [i for i, in z] +14 14 | [i for i, j in y] -C416.py:22:70: C416 [*] Unnecessary `list` comprehension (rewrite using `list()`) +C416.py:24:70: C416 [*] Unnecessary `list` comprehension (rewrite using `list()`) | -21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196 -22 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType]) +23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196 +24 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType]) | ^^^^^^^^^^^^^^^^^^^^^^^ C416 | = help: Rewrite using `list()` ℹ Suggested fix -19 19 | {k: v if v else None for k, v in y} -20 20 | -21 21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196 -22 |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType]) - 22 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType)) +21 21 | {k: v if v else None for k, v in y} +22 22 | +23 23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196 +24 |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType]) + 24 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType))