diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B017.py b/crates/ruff/resources/test/fixtures/flake8_bugbear/B017.py index 6332fc487e..43f1b986e9 100644 --- a/crates/ruff/resources/test/fixtures/flake8_bugbear/B017.py +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B017.py @@ -1,9 +1,10 @@ """ Should emit: -B017 - on lines 20 +B017 - on lines 23 and 41 """ import asyncio import unittest +import pytest CONSTANT = True @@ -34,3 +35,14 @@ class Foobar(unittest.TestCase): def raises_with_absolute_reference(self): with self.assertRaises(asyncio.CancelledError): Foo() + + +def test_pytest_raises(): + with pytest.raises(Exception): + raise ValueError("Hello") + + with pytest.raises(Exception, "hello"): + raise ValueError("This is fine") + + with pytest.raises(Exception, match="hello"): + raise ValueError("This is also fine") diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/assert_raises_exception.rs b/crates/ruff/src/rules/flake8_bugbear/rules/assert_raises_exception.rs index 246a248aeb..8d6853f6ca 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/assert_raises_exception.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/assert_raises_exception.rs @@ -6,15 +6,21 @@ use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; +#[derive(Debug, PartialEq, Eq)] +pub enum AssertionKind { + AssertRaises, + PytestRaises, +} + /// ## What it does -/// Checks for `self.assertRaises(Exception)`. +/// Checks for `self.assertRaises(Exception)` or `pytest.raises(Exception)`. /// /// ## Why is this bad? -/// `assertRaises(Exception)` can lead to your test passing even if the -/// code being tested is never executed due to a typo. +/// These forms catch every `Exception`, which can lead to tests passing even +/// if, e.g., the code being tested is never executed due to a typo. /// -/// Either assert for a more specific exception (builtin or custom), use -/// `assertRaisesRegex` or the context manager form of `assertRaises`. +/// Either assert for a more specific exception (builtin or custom), or use +/// `assertRaisesRegex` or `pytest.raises(..., match=)` respectively. /// /// ## Example /// ```python @@ -26,12 +32,21 @@ use crate::checkers::ast::Checker; /// self.assertRaises(SomeSpecificException, foo) /// ``` #[violation] -pub struct AssertRaisesException; +pub struct AssertRaisesException { + kind: AssertionKind, +} impl Violation for AssertRaisesException { #[derive_message_formats] fn message(&self) -> String { - format!("`assertRaises(Exception)` should be considered evil") + match self.kind { + AssertionKind::AssertRaises => { + format!("`assertRaises(Exception)` should be considered evil") + } + AssertionKind::PytestRaises => { + format!("`pytest.raises(Exception)` should be considered evil") + } + } } } @@ -41,7 +56,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With return; }; let item_context = &item.context_expr; - let ExprKind::Call { func, args, .. } = &item_context.node else { + let ExprKind::Call { func, args, keywords } = &item_context.node else { return; }; if args.len() != 1 { @@ -50,9 +65,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With if item.optional_vars.is_some() { return; } - if !matches!(&func.node, ExprKind::Attribute { attr, .. } if attr == "assertRaises") { - return; - } + if !checker .ctx .resolve_call_path(args.first().unwrap()) @@ -61,7 +74,31 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With return; } - checker - .diagnostics - .push(Diagnostic::new(AssertRaisesException, Range::from(stmt))); + let kind = { + if matches!(&func.node, ExprKind::Attribute { attr, .. } if attr == "assertRaises") { + AssertionKind::AssertRaises + } else if checker + .ctx + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["pytest", "raises"] + }) + && !keywords.iter().any(|keyword| { + keyword + .node + .arg + .as_ref() + .map_or(false, |arg| arg == "match") + }) + { + AssertionKind::PytestRaises + } else { + return; + } + }; + + checker.diagnostics.push(Diagnostic::new( + AssertRaisesException { kind }, + Range::from(stmt), + )); } diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B017_B017.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B017_B017.py.snap index a5d580fbaf..3192db89c7 100644 --- a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B017_B017.py.snap +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B017_B017.py.snap @@ -1,16 +1,27 @@ --- source: crates/ruff/src/rules/flake8_bugbear/mod.rs --- -B017.py:22:9: B017 `assertRaises(Exception)` should be considered evil +B017.py:23:9: B017 `assertRaises(Exception)` should be considered evil | -22 | class Foobar(unittest.TestCase): -23 | def evil_raises(self) -> None: -24 | with self.assertRaises(Exception): +23 | class Foobar(unittest.TestCase): +24 | def evil_raises(self) -> None: +25 | with self.assertRaises(Exception): | _________^ -25 | | raise Exception("Evil I say!") +26 | | raise Exception("Evil I say!") | |__________________________________________^ B017 -26 | -27 | def context_manager_raises(self) -> None: +27 | +28 | def context_manager_raises(self) -> None: + | + +B017.py:41:5: B017 `pytest.raises(Exception)` should be considered evil + | +41 | def test_pytest_raises(): +42 | with pytest.raises(Exception): + | _____^ +43 | | raise ValueError("Hello") + | |_________________________________^ B017 +44 | +45 | with pytest.raises(Exception, "hello"): |