mirror of https://github.com/astral-sh/ruff
[`flake8-bugbear`] Support non-context-manager calls in `B017` (#19063)
## Summary Fixes #19050 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
1a099886ab
commit
5eb5ec987d
|
|
@ -1,10 +1,10 @@
|
||||||
"""
|
"""
|
||||||
Should emit:
|
Should emit:
|
||||||
B017 - on lines 23 and 41
|
B017 - on lines 24, 28, 46, 49, 52, and 58
|
||||||
"""
|
"""
|
||||||
import asyncio
|
import asyncio
|
||||||
import unittest
|
import unittest
|
||||||
import pytest
|
import pytest, contextlib
|
||||||
|
|
||||||
CONSTANT = True
|
CONSTANT = True
|
||||||
|
|
||||||
|
|
@ -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")
|
||||||
|
|
@ -7,7 +7,9 @@ use ruff_python_semantic::analyze::typing;
|
||||||
use ruff_text_size::Ranged;
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
use crate::checkers::ast::Checker;
|
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::registry::Rule;
|
||||||
use crate::rules::{
|
use crate::rules::{
|
||||||
airflow, flake8_2020, flake8_async, flake8_bandit, flake8_boolean_trap, flake8_bugbear,
|
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) {
|
if checker.is_rule_enabled(Rule::NonOctalPermissions) {
|
||||||
ruff::rules::non_octal_permissions(checker, call);
|
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) => {
|
Expr::Dict(dict) => {
|
||||||
if checker.any_rule_enabled(&[
|
if checker.any_rule_enabled(&[
|
||||||
|
|
|
||||||
|
|
@ -125,3 +125,8 @@ pub(crate) const fn is_safe_super_call_with_parameters_fix_enabled(
|
||||||
) -> bool {
|
) -> bool {
|
||||||
settings.preview.is_enabled()
|
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()
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -16,11 +16,14 @@ mod tests {
|
||||||
use crate::settings::LinterSettings;
|
use crate::settings::LinterSettings;
|
||||||
use crate::test::test_path;
|
use crate::test::test_path;
|
||||||
|
|
||||||
|
use crate::settings::types::PreviewMode;
|
||||||
|
|
||||||
use ruff_python_ast::PythonVersion;
|
use ruff_python_ast::PythonVersion;
|
||||||
|
|
||||||
#[test_case(Rule::AbstractBaseClassWithoutAbstractMethod, Path::new("B024.py"))]
|
#[test_case(Rule::AbstractBaseClassWithoutAbstractMethod, Path::new("B024.py"))]
|
||||||
#[test_case(Rule::AssertFalse, Path::new("B011.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::AssignmentToOsEnviron, Path::new("B003.py"))]
|
||||||
#[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))]
|
#[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))]
|
||||||
#[test_case(Rule::ClassAsDataStructure, Path::new("class_as_data_structure.py"))]
|
#[test_case(Rule::ClassAsDataStructure, Path::new("class_as_data_structure.py"))]
|
||||||
|
|
@ -174,4 +177,23 @@ mod tests {
|
||||||
assert_diagnostics!(snapshot, diagnostics);
|
assert_diagnostics!(snapshot, diagnostics);
|
||||||
Ok(())
|
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(())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
use std::fmt;
|
use std::fmt;
|
||||||
|
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
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 ruff_text_size::Ranged;
|
||||||
|
|
||||||
use crate::Violation;
|
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<ExceptionKind> {
|
||||||
|
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
|
/// B017
|
||||||
pub(crate) fn assert_raises_exception(checker: &Checker, items: &[WithItem]) {
|
pub(crate) fn assert_raises_exception(checker: &Checker, items: &[WithItem]) {
|
||||||
for item in items {
|
for item in items {
|
||||||
|
|
@ -73,33 +115,31 @@ pub(crate) fn assert_raises_exception(checker: &Checker, items: &[WithItem]) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
let [arg] = &*arguments.args else {
|
if let Some(exception) =
|
||||||
continue;
|
detect_blind_exception(checker.semantic(), func.as_ref(), arguments)
|
||||||
};
|
|
||||||
|
|
||||||
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())
|
|
||||||
{
|
{
|
||||||
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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
---
|
---
|
||||||
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
|
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):
|
21 | class Foobar(unittest.TestCase):
|
||||||
22 | def evil_raises(self) -> None:
|
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!")
|
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:
|
26 | def also_evil_raises(self) -> None:
|
||||||
27 | with self.assertRaises(BaseException):
|
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!")
|
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():
|
44 | def test_pytest_raises():
|
||||||
45 | with pytest.raises(Exception):
|
45 | with pytest.raises(Exception):
|
||||||
|
|
@ -26,7 +26,7 @@ B017.py:45:10: B017 Do not assert blind exception: `Exception`
|
||||||
46 | raise ValueError("Hello")
|
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")
|
46 | raise ValueError("Hello")
|
||||||
47 |
|
47 |
|
||||||
|
|
@ -35,7 +35,7 @@ B017.py:48:10: B017 Do not assert blind exception: `Exception`
|
||||||
49 | raise ValueError("Hello")
|
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")
|
55 | raise ValueError("This is also fine")
|
||||||
56 |
|
56 |
|
||||||
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
|
||||||
|
---
|
||||||
|
|
||||||
|
|
@ -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")
|
||||||
|
|
|
||||||
|
|
@ -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")
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue