diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B015.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B015.py index 9453e3cddfb24..2794f1512b54c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B015.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B015.py @@ -17,6 +17,11 @@ def test(): 1 in (1, 2) +def test2(): + 1 in (1, 2) + return + + data = [x for x in [1, 2, 3] if x in (1, 2)] diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_comparison.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_comparison.rs index d9d217799f748..83e295a3381ae 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_comparison.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_comparison.rs @@ -1,6 +1,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::Expr; +use ruff_python_ast::{Expr, Stmt}; +use ruff_python_semantic::ScopeKind; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -33,24 +34,34 @@ use super::super::helpers::at_last_top_level_expression_in_cell; /// ## References /// - [Python documentation: `assert` statement](https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement) #[violation] -pub struct UselessComparison; +pub struct UselessComparison { + at: ComparisonLocationAt, +} impl Violation for UselessComparison { #[derive_message_formats] fn message(&self) -> String { - format!( - "Pointless comparison. Did you mean to assign a value? \ - Otherwise, prepend `assert` or remove it." - ) + match self.at { + ComparisonLocationAt::MiddleBody => format!( + "Pointless comparison. Did you mean to assign a value? \ + Otherwise, prepend `assert` or remove it." + ), + ComparisonLocationAt::EndOfFunction => format!( + "Pointless comparison at end of function scope. Did you mean \ + to return the expression result?" + ), + } } } /// B015 pub(crate) fn useless_comparison(checker: &mut Checker, expr: &Expr) { if expr.is_compare_expr() { + let semantic = checker.semantic(); + if checker.source_type.is_ipynb() && at_last_top_level_expression_in_cell( - checker.semantic(), + semantic, checker.locator(), checker.cell_offsets(), ) @@ -58,8 +69,34 @@ pub(crate) fn useless_comparison(checker: &mut Checker, expr: &Expr) { return; } - checker - .diagnostics - .push(Diagnostic::new(UselessComparison, expr.range())); + if let ScopeKind::Function(func_def) = semantic.current_scope().kind { + if func_def + .body + .last() + .and_then(Stmt::as_expr_stmt) + .is_some_and(|last_stmt| &*last_stmt.value == expr) + { + checker.diagnostics.push(Diagnostic::new( + UselessComparison { + at: ComparisonLocationAt::EndOfFunction, + }, + expr.range(), + )); + return; + } + } + + checker.diagnostics.push(Diagnostic::new( + UselessComparison { + at: ComparisonLocationAt::MiddleBody, + }, + expr.range(), + )); } } + +#[derive(Debug, PartialEq, Eq, Copy, Clone)] +enum ComparisonLocationAt { + MiddleBody, + EndOfFunction, +} diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B015_B015.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B015_B015.py.snap index a59d23cb43b92..73529071e6797 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B015_B015.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B015_B015.py.snap @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +assertion_line: 74 --- B015.py:3:1: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it. | @@ -19,7 +20,7 @@ B015.py:7:1: B015 Pointless comparison. Did you mean to assign a value? Otherwis | ^^^^^^^^^^^ B015 | -B015.py:17:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it. +B015.py:17:5: B015 Pointless comparison at end of function scope. Did you mean to return the expression result? | 15 | assert 1 in (1, 2) 16 | @@ -27,11 +28,17 @@ B015.py:17:5: B015 Pointless comparison. Did you mean to assign a value? Otherwi | ^^^^^^^^^^^ B015 | -B015.py:24:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it. +B015.py:21:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it. | -23 | class TestClass: -24 | 1 == 1 - | ^^^^^^ B015 +20 | def test2(): +21 | 1 in (1, 2) + | ^^^^^^^^^^^ B015 +22 | return | - +B015.py:29:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it. + | +28 | class TestClass: +29 | 1 == 1 + | ^^^^^^ B015 + |