From 23ad319b55937a7c83170385d76d22e176fd253d Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Fri, 10 Jan 2025 07:48:18 +0000 Subject: [PATCH] [`flake8-bugbear`] Improve assert-raises-exception (B017) message (#15389) --- .../rules/assert_raises_exception.rs | 44 +++++-------------- ...__flake8_bugbear__tests__B017_B017.py.snap | 10 ++--- 2 files changed, 15 insertions(+), 39 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs index cf11734a8e8dc..98f741a00c707 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs @@ -30,33 +30,14 @@ use crate::checkers::ast::Checker; /// ``` #[derive(ViolationMetadata)] pub(crate) struct AssertRaisesException { - assertion: AssertionKind, exception: ExceptionKind, } impl Violation for AssertRaisesException { #[derive_message_formats] fn message(&self) -> String { - let AssertRaisesException { - assertion, - exception, - } = self; - format!("`{assertion}({exception})` should be considered evil") - } -} - -#[derive(Debug, PartialEq, Eq)] -enum AssertionKind { - AssertRaises, - PytestRaises, -} - -impl fmt::Display for AssertionKind { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self { - AssertionKind::AssertRaises => fmt.write_str("assertRaises"), - AssertionKind::PytestRaises => fmt.write_str("pytest.raises"), - } + let AssertRaisesException { exception } = self; + format!("Do not assert blind exception: `{exception}`") } } @@ -107,24 +88,19 @@ pub(crate) fn assert_raises_exception(checker: &mut Checker, items: &[WithItem]) _ => continue, }; - let assertion = if matches!(func.as_ref(), Expr::Attribute(ast::ExprAttribute { attr, .. }) if attr == "assertRaises") - { - AssertionKind::AssertRaises - } else if semantic - .resolve_qualified_name(func) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pytest", "raises"])) - && arguments.find_keyword("match").is_none() + if !(matches!(func.as_ref(), Expr::Attribute(ast::ExprAttribute { attr, .. }) if attr == "assertRaises") + || semantic + .resolve_qualified_name(func) + .is_some_and(|qualified_name| { + matches!(qualified_name.segments(), ["pytest", "raises"]) + }) + && arguments.find_keyword("match").is_none()) { - AssertionKind::PytestRaises - } else { continue; }; checker.diagnostics.push(Diagnostic::new( - AssertRaisesException { - assertion, - exception, - }, + AssertRaisesException { exception }, item.range(), )); } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017.py.snap index ef0af2bc0a602..5b578e1f50669 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017.py.snap @@ -2,7 +2,7 @@ source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs snapshot_kind: text --- -B017.py:23:14: B017 `assertRaises(Exception)` should be considered evil +B017.py:23:14: B017 Do not assert blind exception: `Exception` | 21 | class Foobar(unittest.TestCase): 22 | def evil_raises(self) -> None: @@ -11,7 +11,7 @@ B017.py:23:14: B017 `assertRaises(Exception)` should be considered evil 24 | raise Exception("Evil I say!") | -B017.py:27:14: B017 `assertRaises(BaseException)` should be considered evil +B017.py:27:14: B017 Do not assert blind exception: `BaseException` | 26 | def also_evil_raises(self) -> None: 27 | with self.assertRaises(BaseException): @@ -19,7 +19,7 @@ B017.py:27:14: B017 `assertRaises(BaseException)` should be considered evil 28 | raise Exception("Evil I say!") | -B017.py:45:10: B017 `pytest.raises(Exception)` should be considered evil +B017.py:45:10: B017 Do not assert blind exception: `Exception` | 44 | def test_pytest_raises(): 45 | with pytest.raises(Exception): @@ -27,7 +27,7 @@ B017.py:45:10: B017 `pytest.raises(Exception)` should be considered evil 46 | raise ValueError("Hello") | -B017.py:48:10: B017 `pytest.raises(Exception)` should be considered evil +B017.py:48:10: B017 Do not assert blind exception: `Exception` | 46 | raise ValueError("Hello") 47 | @@ -36,7 +36,7 @@ B017.py:48:10: B017 `pytest.raises(Exception)` should be considered evil 49 | raise ValueError("Hello") | -B017.py:57:36: B017 `pytest.raises(Exception)` should be considered evil +B017.py:57:36: B017 Do not assert blind exception: `Exception` | 55 | raise ValueError("This is also fine") 56 |