[`flake8-bugbear`] Support non-context-manager calls in `B017` #19063

danparizher · 2025-07-01T02:25:44Z

Summary

Fixes #19050

github-actions · 2025-07-01T02:36:30Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

pytest-dev/pytest (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ testing/code/test_code.py:91:15: B017 Do not assert blind exception: `Exception`
+ testing/example_scripts/fixtures/fill_fixtures/test_conftest_funcargs_only_available_in_subdir/sub2/conftest.py:9:5: B017 Do not assert blind exception: `Exception`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
B017	2	2	0	0	0

ntBre

Thanks! This looks good overall, just a couple of minor suggestions. Can you also gate this behind preview since it's an expansion of a stable rule? Here's an example:

https://github.com/astral-sh/ruff/blob/main/crates/ruff_linter/src/preview.rs#L44

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs

danparizher · 2025-07-01T15:46:06Z

Preview flag added. The call-form check only runs when is_assert_raises_exception_call_enabled(settings) is true.

ntBre · 2025-07-01T16:01:13Z

Great, thanks! Could you also add a preview test? You can just make a copy of the rules function and the B017 test_case and set LinterSettings::preview to PreviewMode::Enabled.

ruff/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs

Lines 73 to 81 in 48366a7

    
           fn rules(rule_code: Rule, path: &Path) -> Result<()> { 
        
               let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); 
        
               let diagnostics = test_path( 
        
                   Path::new("flake8_bugbear").join(path).as_path(), 
        
                   &LinterSettings::for_rule(rule_code), 
        
               )?; 
        
               assert_diagnostics!(snapshot, diagnostics); 
        
               Ok(()) 
        
           }

danparizher · 2025-07-01T16:34:34Z

Added!

ntBre

Thanks!

I noticed a few more things here, but it's looking good.

ntBre · 2025-07-01T21:12:58Z

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs

+        return None;
+    }
+
+    if is_pytest_raises && arguments.find_keyword("match").is_some() {


I think this is supposed to be is_none like before right?

Suggested change

if is_pytest_raises && arguments.find_keyword("match").is_some() {

if is_pytest_raises && arguments.find_keyword("match").is_none() {

It's a little hard to tell because the diff shifted around, but I'm pretty sure one of the snapshots is showing a different result.

I think this is_some is actually correct and the existing is_none is a bug unless I'm misunderstanding something, but I'd rather handle that separately, I guess.

Unless I'm missing something, I believe is_some() is the right test here. pytest.raises(..., match="pattern") is a form that already narrows the check to a specific exception message (equivalent to the assertRaisesRegex case for unittest), so when that match= keyword is present we want to skip B017, because it’s no longer a “blind” exception assertion.

Hmm, maybe I got confused by the logic here. The snapshot on main looks correct. Let's regroup the test cases and make sure none of the stable results change, but I think you may be right.

is_some definitely makes sense intuitively, I guess the condition was negated in the original code.

ntBre · 2025-07-01T21:15:23Z

crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B017.py

+    def call_form_raises(self) -> None:
+        self.assertRaises(Exception, something_else)
+        self.assertRaises(BaseException, something_else)
+


I know it would mess up the nice sections here, but it would be easier for reviewing the test diff if these were at the bottom with the other new tests. If you put enough blank lines, the non-preview snapshot should be unchanged, and only the new preview snapshot will be different. Alternatively, you could rename this file to B017_0.py and add B017_1.py with the new tests.

I was fine with this layout before, but I'd like to be sure we're not changing any logic with the is_none vs is_some thing, and this will make that easier.

I split up the test cases, let me know if that's what you were looking for. Feel free to commit directly if there is another format that makes more sense 🙂

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs

ntBre · 2025-07-02T13:10:49Z

...ules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B017_B017_0.py.snap

+B017_0.py:51:10: B017 Do not assert blind exception: `Exception`
+   |
+49 |         raise ValueError("Hello")
+50 |
+51 |     with pytest.raises(Exception, "hello"):
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B017
+52 |         raise ValueError("This is fine")
+   |
+
+B017_0.py:57:36: B017 Do not assert blind exception: `Exception`


I pushed a couple of commits to minimize the diff a bit more. I think something is slightly off with the new logic because it looks like this is getting flagged now when it should be allowed, but I haven't pinned down where it is.

I found the issue—we weren't considering the second positional string/bytes literal argument. I added in that additional check so now it's not registering as a false positive anymore.

ntBre

This looks good, thank you!

Unrelated to your changes, from looking at the code while reviewing the PR, I've noticed some other issues with the rule. For example, the rule has false negatives when the exceptions are provided as keyword arguments:

import pytest
import unittest

with pytest.raises(Exception):
    pass

with pytest.raises(expected_exception=Exception):  # false negative
    pass

class TestClass(unittest.TestCase):
    def test_okay(self):
        with self.assertRaises(Exception):
            pass

    def test_false_negative(self):
        with self.assertRaises(exception=Exception):  # false negative
            pass

We can go ahead and land this, though, and I'll open a follow-up issue for these.

playground

…re_help * 'main' of https://github.com/astral-sh/ruff: (34 commits) [docs] add capital one to who's using ruff (astral-sh#19248) [`pyupgrade`] Keyword arguments in `super` should suppress the `UP008` fix (astral-sh#19131) [`flake8-use-pathlib`] Add autofixes for `PTH100`, `PTH106`, `PTH107`, `PTH108`, `PTH110`, `PTH111`, `PTH112`, `PTH113`, `PTH114`, `PTH115`, `PTH117`, `PTH119`, `PTH120` (astral-sh#19213) [ty] Do not run `mypy_primer.yaml` when all changed files are Markdown files (astral-sh#19244) [`flake8-bandit`] Make example error out-of-the-box (`S412`) (astral-sh#19241) [`pydoclint`] Make example error out-of-the-box (`DOC501`) (astral-sh#19218) [ty] Add "kind" to completion suggestions [ty] Add type information to `all_members` API [ty] Expand API of `all_members` to return a struct [ty] Ecosystem analyzer PR comment workflow (astral-sh#19237) [ty] Merge `ty_macros` into `ruff_macros` (astral-sh#19229) [ty] Fix `ClassLiteral.into_callable` for dataclasses (astral-sh#19192) [ty] `dataclasses.field` support (astral-sh#19140) [ty] Fix panic for attribute expressions with empty value (astral-sh#19069) [ty] Return `CallableType` from `BoundMethodType.into_callable_type` (astral-sh#19193) [`flake8-bugbear`] Support non-context-manager calls in `B017` (astral-sh#19063) [ty] Improved diagnostic for reassignments of `Final` symbols (astral-sh#19214) [ty] Use full range for assignment definitions (astral-sh#19211) [`pylint`] Update `missing-maxsplit-arg` docs and error to suggest proper usage (`PLC0207`) (astral-sh#18949) [ty] Add `set -eu` to mypy-primer script (astral-sh#19212) ... # Conflicts: # crates/ty_python_semantic/src/types/class.rs

danparizher added 2 commits June 30, 2025 22:19

fix-19050

3bfae2f

remove comments from test

f25dc89

ntBre reviewed Jul 1, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs Outdated Show resolved Hide resolved

ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jul 1, 2025

implement feedback

7bbc953

add preview test

d381956

ntBre reviewed Jul 1, 2025

View reviewed changes

danparizher and others added 3 commits July 1, 2025 19:25

split tests

3dc56dc

avoid line number changes

fe26add

whitespace

ba492ef

ntBre reviewed Jul 2, 2025

View reviewed changes

fix false positive

0545a61

ntBre approved these changes Jul 8, 2025

View reviewed changes

ntBre changed the title ~~[flake8-bugbear] support non-with calls in B017~~ [flake8-bugbear] Support non-context-manager calls in B017 Jul 8, 2025

ntBre merged commit 5eb5ec9 into astral-sh:main Jul 8, 2025
35 checks passed

ntBre mentioned this pull request Jul 8, 2025

B017 has false negatives #19215

Closed

danparizher deleted the fix-19050 branch July 8, 2025 19:10

BrewTestBot mentioned this pull request Jul 11, 2025

ruff 0.12.3 Homebrew/homebrew-core#229792

Merged

	if is_pytest_raises && arguments.find_keyword("match").is_some() {
	if is_pytest_raises && arguments.find_keyword("match").is_none() {

[flake8-bugbear] Support non-context-manager calls in B017 #19063

[flake8-bugbear] Support non-context-manager calls in B017 #19063

Uh oh!

Conversation

danparizher commented Jul 1, 2025

Summary

Uh oh!

github-actions bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danparizher commented Jul 1, 2025

Uh oh!

ntBre commented Jul 1, 2025

Uh oh!

danparizher commented Jul 1, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

danparizher Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

danparizher Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ntBre Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

danparizher Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[`flake8-bugbear`] Support non-context-manager calls in `B017` #19063

[`flake8-bugbear`] Support non-context-manager calls in `B017` #19063

github-actions bot commented Jul 1, 2025 •

edited

Loading

`ruff-ecosystem` results