diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py index 6f86a62204..182412e545 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py @@ -31,6 +31,8 @@ for key in list(obj.keys()): key in (obj or {}).keys() # SIM118 +(key) in (obj or {}).keys() # SIM118 + from typing import KeysView diff --git a/crates/ruff/src/checkers/ast/analyze/comprehension.rs b/crates/ruff/src/checkers/ast/analyze/comprehension.rs index 8cd73dca0e..c3d368be29 100644 --- a/crates/ruff/src/checkers/ast/analyze/comprehension.rs +++ b/crates/ruff/src/checkers/ast/analyze/comprehension.rs @@ -7,10 +7,6 @@ use crate::rules::flake8_simplify; /// Run lint rules over a [`Comprehension`] syntax nodes. pub(crate) fn comprehension(comprehension: &Comprehension, checker: &mut Checker) { if checker.enabled(Rule::InDictKeys) { - flake8_simplify::rules::key_in_dict_for( - checker, - &comprehension.target, - &comprehension.iter, - ); + flake8_simplify::rules::key_in_dict_comprehension(checker, comprehension); } } diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index 94f10560dc..6c1af752df 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -1178,7 +1178,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { pylint::rules::magic_value_comparison(checker, left, comparators); } if checker.enabled(Rule::InDictKeys) { - flake8_simplify::rules::key_in_dict_compare(checker, expr, left, ops, comparators); + flake8_simplify::rules::key_in_dict_compare(checker, compare); } if checker.enabled(Rule::YodaConditions) { flake8_simplify::rules::yoda_conditions(checker, expr, left, ops, comparators); diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index 0a5c1c22f3..892653835c 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -1139,7 +1139,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pygrep_hooks::rules::non_existent_mock_method(checker, test); } } - Stmt::With(with_ @ ast::StmtWith { items, body, .. }) => { + Stmt::With(with_stmt @ ast::StmtWith { items, body, .. }) => { if checker.enabled(Rule::AssertRaisesException) { flake8_bugbear::rules::assert_raises_exception(checker, items); } @@ -1149,7 +1149,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::MultipleWithStatements) { flake8_simplify::rules::multiple_with_statements( checker, - with_, + with_stmt, checker.semantic.current_statement_parent(), ); } @@ -1168,13 +1168,16 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { perflint::rules::try_except_in_loop(checker, body); } } - Stmt::For(ast::StmtFor { - target, - body, - iter, - orelse, - .. - }) => { + Stmt::For( + for_stmt @ ast::StmtFor { + target, + body, + iter, + orelse, + is_async, + .. + }, + ) => { if checker.any_enabled(&[ Rule::UnusedLoopControlVariable, Rule::IncorrectDictIterator, @@ -1200,17 +1203,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::IterationOverSet) { pylint::rules::iteration_over_set(checker, iter); } - if stmt.is_for_stmt() { - if checker.enabled(Rule::ReimplementedBuiltin) { - flake8_simplify::rules::convert_for_loop_to_any_all(checker, stmt); - } - if checker.enabled(Rule::InDictKeys) { - flake8_simplify::rules::key_in_dict_for(checker, target, iter); - } - if checker.enabled(Rule::TryExceptInLoop) { - perflint::rules::try_except_in_loop(checker, body); - } - } if checker.enabled(Rule::ManualListComprehension) { perflint::rules::manual_list_comprehension(checker, target, body); } @@ -1220,6 +1212,17 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryListCast) { perflint::rules::unnecessary_list_cast(checker, iter); } + if !is_async { + if checker.enabled(Rule::ReimplementedBuiltin) { + flake8_simplify::rules::convert_for_loop_to_any_all(checker, stmt); + } + if checker.enabled(Rule::InDictKeys) { + flake8_simplify::rules::key_in_dict_for(checker, for_stmt); + } + if checker.enabled(Rule::TryExceptInLoop) { + perflint::rules::try_except_in_loop(checker, body); + } + } } Stmt::Try(ast::StmtTry { body, diff --git a/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs b/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs index 8cab146bad..ac25976452 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs @@ -1,16 +1,12 @@ -use anyhow::Result; -use ruff_python_ast::{self as ast, Arguments, CmpOp, Expr, Ranged}; -use ruff_text_size::TextRange; - use ruff_diagnostics::Edit; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_codegen::Stylist; -use ruff_source_file::Locator; +use ruff_python_ast::node::AnyNodeRef; +use ruff_python_ast::parenthesize::parenthesized_range; +use ruff_python_ast::{self as ast, Arguments, CmpOp, Comprehension, Expr, Ranged}; +use ruff_text_size::TextRange; -use crate::autofix::codemods::CodegenStylist; use crate::checkers::ast::Checker; -use crate::cst::matchers::{match_attribute, match_call_mut, match_expression}; use crate::registry::AsRule; /// ## What it does @@ -67,7 +63,7 @@ fn key_in_dict( left: &Expr, right: &Expr, operator: CmpOp, - range: TextRange, + parent: AnyNodeRef, ) { let Expr::Call(ast::ExprCall { func, @@ -100,13 +96,16 @@ fn key_in_dict( return; } - // Slice exact content to preserve formatting. - let left_content = checker.locator().slice(left.range()); - let Ok(value_content) = - value_content_for_key_in_dict(checker.locator(), checker.stylist(), right) - else { - return; - }; + // Extract the exact range of the left and right expressions. + let left_range = parenthesized_range(left.into(), parent, checker.locator().contents()) + .unwrap_or(left.range()); + let right_range = parenthesized_range(right.into(), parent, checker.locator().contents()) + .unwrap_or(right.range()); + let value_range = parenthesized_range(value.into(), parent, checker.locator().contents()) + .unwrap_or(value.range()); + + let left_content = checker.locator().slice(left_range); + let value_content = checker.locator().slice(value_range); let mut diagnostic = Diagnostic::new( InDictKeys { @@ -114,37 +113,42 @@ fn key_in_dict( dict: value_content.to_string(), operator: operator.as_str().to_string(), }, - range, + TextRange::new(left_range.start(), right_range.end()), ); if checker.patch(diagnostic.kind.rule()) { diagnostic.set_fix(Fix::suggested(Edit::range_replacement( - value_content, - right.range(), + value_content.to_string(), + right_range, ))); } checker.diagnostics.push(diagnostic); } -/// SIM118 in a for loop -pub(crate) fn key_in_dict_for(checker: &mut Checker, target: &Expr, iter: &Expr) { +/// SIM118 in a `for` loop. +pub(crate) fn key_in_dict_for(checker: &mut Checker, for_stmt: &ast::StmtFor) { key_in_dict( checker, - target, - iter, + &for_stmt.target, + &for_stmt.iter, CmpOp::In, - TextRange::new(target.start(), iter.end()), + for_stmt.into(), ); } -/// SIM118 in a comparison -pub(crate) fn key_in_dict_compare( - checker: &mut Checker, - expr: &Expr, - left: &Expr, - ops: &[CmpOp], - comparators: &[Expr], -) { - let [op] = ops else { +/// SIM118 in a comprehension. +pub(crate) fn key_in_dict_comprehension(checker: &mut Checker, comprehension: &Comprehension) { + key_in_dict( + checker, + &comprehension.target, + &comprehension.iter, + CmpOp::In, + comprehension.into(), + ); +} + +/// SIM118 in a comparison. +pub(crate) fn key_in_dict_compare(checker: &mut Checker, compare: &ast::ExprCompare) { + let [op] = compare.ops.as_slice() else { return; }; @@ -152,21 +156,9 @@ pub(crate) fn key_in_dict_compare( return; } - let [right] = comparators else { + let [right] = compare.comparators.as_slice() else { return; }; - key_in_dict(checker, left, right, *op, expr.range()); -} - -fn value_content_for_key_in_dict( - locator: &Locator, - stylist: &Stylist, - expr: &Expr, -) -> Result { - let content = locator.slice(expr.range()); - let mut expression = match_expression(content)?; - let call = match_call_mut(&mut expression)?; - let attribute = match_attribute(&mut call.func)?; - Ok(attribute.value.codegen_stylist(stylist)) + key_in_dict(checker, &compare.left, right, *op, compare.into()); } diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap index 05cdf5c898..09dac25801 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap @@ -274,7 +274,7 @@ SIM118.py:32:1: SIM118 [*] Use `key in (obj or {})` instead of `key in (obj or { 32 | key in (obj or {}).keys() # SIM118 | ^^^^^^^^^^^^^^^^^^^^^^^^^ SIM118 33 | -34 | from typing import KeysView +34 | (key) in (obj or {}).keys() # SIM118 | = help: Convert to `key in (obj or {})` @@ -285,7 +285,28 @@ SIM118.py:32:1: SIM118 [*] Use `key in (obj or {})` instead of `key in (obj or { 32 |-key in (obj or {}).keys() # SIM118 32 |+key in (obj or {}) # SIM118 33 33 | -34 34 | from typing import KeysView +34 34 | (key) in (obj or {}).keys() # SIM118 35 35 | +SIM118.py:34:1: SIM118 [*] Use `(key) in (obj or {})` instead of `(key) in (obj or {}).keys()` + | +32 | key in (obj or {}).keys() # SIM118 +33 | +34 | (key) in (obj or {}).keys() # SIM118 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM118 +35 | +36 | from typing import KeysView + | + = help: Convert to `(key) in (obj or {})` + +ℹ Suggested fix +31 31 | +32 32 | key in (obj or {}).keys() # SIM118 +33 33 | +34 |-(key) in (obj or {}).keys() # SIM118 + 34 |+(key) in (obj or {}) # SIM118 +35 35 | +36 36 | from typing import KeysView +37 37 | + diff --git a/crates/ruff_python_formatter/src/statement/suite.rs b/crates/ruff_python_formatter/src/statement/suite.rs index c137dd8aa2..3ea5b83975 100644 --- a/crates/ruff_python_formatter/src/statement/suite.rs +++ b/crates/ruff_python_formatter/src/statement/suite.rs @@ -339,7 +339,9 @@ impl FormatRule> for FormatSuite { pub(crate) fn contains_only_an_ellipsis(body: &[Stmt], comments: &Comments) -> bool { match body { [Stmt::Expr(ast::StmtExpr { value, .. })] => { - let [node] = body else { return false; }; + let [node] = body else { + return false; + }; matches!( value.as_ref(), Expr::Constant(ast::ExprConstant {