From 3cab68c8860005e396c0ea2948bd1e2cd098d0c7 Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 1 Apr 2021 22:16:40 +0100 Subject: [PATCH] Bugfix: Verify the element in item_context.args is of type ast.Name (#166) * Add B017 - detection for a bad form of assertRaises ```with assertRaises(Exception):``` is basically a "catch 'em all" assert that casts far too broad of a net when it comes to detecting failures in code being tested. Assertions should be testing specific failure cases, not "did Python throw /any/ type of error?", and so the context manager form, or the assertRaisesRegex form are far better to use. * Amend documentation, revert version change * Bugfix: Add more conditionals to B017 checks * Bugfix: Add more conditionals to B017 checks Attempts to ensure we're only looking at Name type objects in the item_context.args, since apparently this code is still matching other objects that don't have an id attr. * More updates to fix the B017 breakage * Black things * And change the line number again due to black --- bugbear.py | 4 +++- tests/b017.py | 12 ++++++++++-- tests/test_bugbear.py | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/bugbear.py b/bugbear.py index 429ebd6..b881b8b 100644 --- a/bugbear.py +++ b/bugbear.py @@ -10,9 +10,10 @@ from keyword import iskeyword import attr + import pycodestyle -__version__ = "21.4.2" +__version__ = "21.4.3" LOG = logging.getLogger("flake8.bugbear") @@ -443,6 +444,7 @@ def check_for_b017(self, node): and hasattr(item_context.func, "attr") # noqa W503 and item_context.func.attr == "assertRaises" # noqa W503 and len(item_context.args) == 1 # noqa W503 + and isinstance(item_context.args[0], ast.Name) # noqa W503 and item_context.args[0].id == "Exception" # noqa W503 and not item.optional_vars # noqa W503 ): diff --git a/tests/b017.py b/tests/b017.py index e9b7430..21836a6 100644 --- a/tests/b017.py +++ b/tests/b017.py @@ -1,9 +1,9 @@ """ Should emit: -B017 - on lines 10 +B017 - on lines 20 """ import unittest - +import asyncio CONSTANT = True @@ -13,6 +13,10 @@ def something_else() -> None: print(i) +class Foo: + pass + + class Foobar(unittest.TestCase): def evil_raises(self) -> None: with self.assertRaises(Exception): @@ -26,3 +30,7 @@ def context_manager_raises(self) -> None: def regex_raises(self) -> None: with self.assertRaisesRegex(Exception, "Regex is good"): raise Exception("Regex is good") + + def raises_with_absolute_reference(self): + with self.assertRaises(asyncio.CancelledError): + Foo() diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 8bd279f..c6e31bb 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -210,7 +210,7 @@ def test_b017(self): filename = Path(__file__).absolute().parent / "b017.py" bbc = BugBearChecker(filename=str(filename)) errors = list(bbc.run()) - expected = self.errors(B017(18, 8)) + expected = self.errors(B017(22, 8)) self.assertEqual(errors, expected) def test_b301_b302_b305(self):