diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017_0.py similarity index 95% rename from crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017.py rename to crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017_0.py index 99def7a555..9d706168a9 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017_0.py @@ -1,10 +1,10 @@ """ Should emit: -B017 - on lines 23 and 41 +B017 - on lines 24, 28, 46, 49, 52, and 58 """ import asyncio import unittest -import pytest +import pytest, contextlib CONSTANT = True diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017_1.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017_1.py new file mode 100644 index 0000000000..8a383a92f2 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017_1.py @@ -0,0 +1,28 @@ +""" +Should emit: +B017 - on lines 20, 21, 25, and 26 +""" +import unittest +import pytest + + +def something_else() -> None: + for i in (1, 2, 3): + print(i) + + +class Foo: + pass + + +class Foobar(unittest.TestCase): + def call_form_raises(self) -> None: + self.assertRaises(Exception, something_else) + self.assertRaises(BaseException, something_else) + + +def test_pytest_call_form() -> None: + pytest.raises(Exception, something_else) + pytest.raises(BaseException, something_else) + + pytest.raises(Exception, something_else, match="hello") diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index dfddaab7a3..f00dc4b9fb 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -7,7 +7,9 @@ use ruff_python_semantic::analyze::typing; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -use crate::preview::is_optional_as_none_in_union_enabled; +use crate::preview::{ + is_assert_raises_exception_call_enabled, is_optional_as_none_in_union_enabled, +}; use crate::registry::Rule; use crate::rules::{ airflow, flake8_2020, flake8_async, flake8_bandit, flake8_boolean_trap, flake8_bugbear, @@ -1236,6 +1238,11 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { if checker.is_rule_enabled(Rule::NonOctalPermissions) { ruff::rules::non_octal_permissions(checker, call); } + if checker.is_rule_enabled(Rule::AssertRaisesException) + && is_assert_raises_exception_call_enabled(checker.settings()) + { + flake8_bugbear::rules::assert_raises_exception_call(checker, call); + } } Expr::Dict(dict) => { if checker.any_rule_enabled(&[ diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 26aaea5b53..0957cee29b 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -125,3 +125,8 @@ pub(crate) const fn is_safe_super_call_with_parameters_fix_enabled( ) -> bool { settings.preview.is_enabled() } + +// https://github.com/astral-sh/ruff/pull/19063 +pub(crate) const fn is_assert_raises_exception_call_enabled(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs index 371d1283cf..53f3ce51e3 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs @@ -16,11 +16,14 @@ mod tests { use crate::settings::LinterSettings; use crate::test::test_path; + use crate::settings::types::PreviewMode; + use ruff_python_ast::PythonVersion; #[test_case(Rule::AbstractBaseClassWithoutAbstractMethod, Path::new("B024.py"))] #[test_case(Rule::AssertFalse, Path::new("B011.py"))] - #[test_case(Rule::AssertRaisesException, Path::new("B017.py"))] + #[test_case(Rule::AssertRaisesException, Path::new("B017_0.py"))] + #[test_case(Rule::AssertRaisesException, Path::new("B017_1.py"))] #[test_case(Rule::AssignmentToOsEnviron, Path::new("B003.py"))] #[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))] #[test_case(Rule::ClassAsDataStructure, Path::new("class_as_data_structure.py"))] @@ -174,4 +177,23 @@ mod tests { assert_diagnostics!(snapshot, diagnostics); Ok(()) } + + #[test_case(Rule::AssertRaisesException, Path::new("B017_0.py"))] + #[test_case(Rule::AssertRaisesException, Path::new("B017_1.py"))] + fn rules_preview(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "preview__{}_{}", + rule_code.noqa_code(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("flake8_bugbear").join(path).as_path(), + &LinterSettings { + preview: PreviewMode::Enabled, + ..LinterSettings::for_rule(rule_code) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } } 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 9a74185ad3..73bc6f131e 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 @@ -1,7 +1,7 @@ use std::fmt; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::{self as ast, Expr, WithItem}; +use ruff_python_ast::{self as ast, Arguments, Expr, WithItem}; use ruff_text_size::Ranged; use crate::Violation; @@ -56,6 +56,48 @@ impl fmt::Display for ExceptionKind { } } +fn detect_blind_exception( + semantic: &ruff_python_semantic::SemanticModel<'_>, + func: &Expr, + arguments: &Arguments, +) -> Option { + let is_assert_raises = matches!( + func, + &Expr::Attribute(ast::ExprAttribute { ref attr, .. }) if attr.as_str() == "assertRaises" + ); + + let is_pytest_raises = semantic + .resolve_qualified_name(func) + .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pytest", "raises"])); + + if !(is_assert_raises || is_pytest_raises) { + return None; + } + + if is_pytest_raises { + if arguments.find_keyword("match").is_some() { + return None; + } + + if arguments + .find_positional(1) + .is_some_and(|arg| matches!(arg, Expr::StringLiteral(_) | Expr::BytesLiteral(_))) + { + return None; + } + } + + let first_arg = arguments.args.first()?; + + let builtin_symbol = semantic.resolve_builtin_symbol(first_arg)?; + + match builtin_symbol { + "Exception" => Some(ExceptionKind::Exception), + "BaseException" => Some(ExceptionKind::BaseException), + _ => None, + } +} + /// B017 pub(crate) fn assert_raises_exception(checker: &Checker, items: &[WithItem]) { for item in items { @@ -73,33 +115,31 @@ pub(crate) fn assert_raises_exception(checker: &Checker, items: &[WithItem]) { continue; } - let [arg] = &*arguments.args else { - continue; - }; - - let semantic = checker.semantic(); - - let Some(builtin_symbol) = semantic.resolve_builtin_symbol(arg) else { - continue; - }; - - let exception = match builtin_symbol { - "Exception" => ExceptionKind::Exception, - "BaseException" => ExceptionKind::BaseException, - _ => continue, - }; - - 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()) + if let Some(exception) = + detect_blind_exception(checker.semantic(), func.as_ref(), arguments) { - continue; + checker.report_diagnostic(AssertRaisesException { exception }, item.range()); } - - checker.report_diagnostic(AssertRaisesException { exception }, item.range()); + } +} + +/// B017 (call form) +pub(crate) fn assert_raises_exception_call( + checker: &Checker, + ast::ExprCall { + func, + arguments, + range, + node_index: _, + }: &ast::ExprCall, +) { + let semantic = checker.semantic(); + + if arguments.args.len() < 2 && arguments.find_argument("func", 1).is_none() { + return; + } + + if let Some(exception) = detect_blind_exception(semantic, func.as_ref(), arguments) { + checker.report_diagnostic(AssertRaisesException { exception }, *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_0.py.snap similarity index 77% rename from crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017.py.snap rename to crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017_0.py.snap index bbee0c1d91..3d23080944 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_0.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs --- -B017.py:23:14: B017 Do not assert blind exception: `Exception` +B017_0.py:23:14: B017 Do not assert blind exception: `Exception` | 21 | class Foobar(unittest.TestCase): 22 | def evil_raises(self) -> None: @@ -10,7 +10,7 @@ B017.py:23:14: B017 Do not assert blind exception: `Exception` 24 | raise Exception("Evil I say!") | -B017.py:27:14: B017 Do not assert blind exception: `BaseException` +B017_0.py:27:14: B017 Do not assert blind exception: `BaseException` | 26 | def also_evil_raises(self) -> None: 27 | with self.assertRaises(BaseException): @@ -18,7 +18,7 @@ B017.py:27:14: B017 Do not assert blind exception: `BaseException` 28 | raise Exception("Evil I say!") | -B017.py:45:10: B017 Do not assert blind exception: `Exception` +B017_0.py:45:10: B017 Do not assert blind exception: `Exception` | 44 | def test_pytest_raises(): 45 | with pytest.raises(Exception): @@ -26,7 +26,7 @@ B017.py:45:10: B017 Do not assert blind exception: `Exception` 46 | raise ValueError("Hello") | -B017.py:48:10: B017 Do not assert blind exception: `Exception` +B017_0.py:48:10: B017 Do not assert blind exception: `Exception` | 46 | raise ValueError("Hello") 47 | @@ -35,7 +35,7 @@ B017.py:48:10: B017 Do not assert blind exception: `Exception` 49 | raise ValueError("Hello") | -B017.py:57:36: B017 Do not assert blind exception: `Exception` +B017_0.py:57:36: B017 Do not assert blind exception: `Exception` | 55 | raise ValueError("This is also fine") 56 | diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017_1.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017_1.py.snap new file mode 100644 index 0000000000..967e60a4f9 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017_1.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B017_B017_0.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B017_B017_0.py.snap new file mode 100644 index 0000000000..3d23080944 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B017_B017_0.py.snap @@ -0,0 +1,45 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +--- +B017_0.py:23:14: B017 Do not assert blind exception: `Exception` + | +21 | class Foobar(unittest.TestCase): +22 | def evil_raises(self) -> None: +23 | with self.assertRaises(Exception): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017 +24 | raise Exception("Evil I say!") + | + +B017_0.py:27:14: B017 Do not assert blind exception: `BaseException` + | +26 | def also_evil_raises(self) -> None: +27 | with self.assertRaises(BaseException): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017 +28 | raise Exception("Evil I say!") + | + +B017_0.py:45:10: B017 Do not assert blind exception: `Exception` + | +44 | def test_pytest_raises(): +45 | with pytest.raises(Exception): + | ^^^^^^^^^^^^^^^^^^^^^^^^ B017 +46 | raise ValueError("Hello") + | + +B017_0.py:48:10: B017 Do not assert blind exception: `Exception` + | +46 | raise ValueError("Hello") +47 | +48 | with pytest.raises(Exception), pytest.raises(ValueError): + | ^^^^^^^^^^^^^^^^^^^^^^^^ B017 +49 | raise ValueError("Hello") + | + +B017_0.py:57:36: B017 Do not assert blind exception: `Exception` + | +55 | raise ValueError("This is also fine") +56 | +57 | with contextlib.nullcontext(), pytest.raises(Exception): + | ^^^^^^^^^^^^^^^^^^^^^^^^ B017 +58 | raise ValueError("Multiple context managers") + | diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B017_B017_1.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B017_B017_1.py.snap new file mode 100644 index 0000000000..ee9aadeedd --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B017_B017_1.py.snap @@ -0,0 +1,37 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +--- +B017_1.py:20:9: B017 Do not assert blind exception: `Exception` + | +18 | class Foobar(unittest.TestCase): +19 | def call_form_raises(self) -> None: +20 | self.assertRaises(Exception, something_else) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017 +21 | self.assertRaises(BaseException, something_else) + | + +B017_1.py:21:9: B017 Do not assert blind exception: `BaseException` + | +19 | def call_form_raises(self) -> None: +20 | self.assertRaises(Exception, something_else) +21 | self.assertRaises(BaseException, something_else) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017 + | + +B017_1.py:25:5: B017 Do not assert blind exception: `Exception` + | +24 | def test_pytest_call_form() -> None: +25 | pytest.raises(Exception, something_else) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017 +26 | pytest.raises(BaseException, something_else) + | + +B017_1.py:26:5: B017 Do not assert blind exception: `BaseException` + | +24 | def test_pytest_call_form() -> None: +25 | pytest.raises(Exception, something_else) +26 | pytest.raises(BaseException, something_else) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017 +27 | +28 | pytest.raises(Exception, something_else, match="hello") + |