Skip to content

Commit

Permalink
Bugfix: Verify the element in item_context.args is of type ast.Name (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
cricalix committed Apr 1, 2021
1 parent d3bf9ab commit 3cab68c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 4 deletions.
4 changes: 3 additions & 1 deletion bugbear.py
Expand Up @@ -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")

Expand Down Expand Up @@ -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
):
Expand Down
12 changes: 10 additions & 2 deletions tests/b017.py
@@ -1,9 +1,9 @@
"""
Should emit:
B017 - on lines 10
B017 - on lines 20
"""
import unittest

import asyncio

CONSTANT = True

Expand All @@ -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):
Expand All @@ -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()
2 changes: 1 addition & 1 deletion tests/test_bugbear.py
Expand Up @@ -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):
Expand Down

0 comments on commit 3cab68c

Please sign in to comment.