From 9d1c1600a17ad4050902e654295407af416c8509 Mon Sep 17 00:00:00 2001 From: azinneck0485 <123660683+azinneck0485@users.noreply.github.com> Date: Mon, 20 Nov 2023 21:42:37 -0500 Subject: [PATCH 1/6] handle ast.ImportFrom nodes in b005 check to add this to the list of imports --- bugbear.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/bugbear.py b/bugbear.py index 5fbfa19..06ec7f2 100644 --- a/bugbear.py +++ b/bugbear.py @@ -547,6 +547,9 @@ def visit_Import(self, node): self.check_for_b005(node) self.generic_visit(node) + def visit_ImportFrom(self,node): + self.visit_Import(node) + def visit_Set(self, node): self.check_for_b033(node) self.generic_visit(node) @@ -555,6 +558,10 @@ def check_for_b005(self, node): if isinstance(node, ast.Import): for name in node.names: self._b005_imports.add(name.asname or name.name) + elif isinstance(node, ast.ImportFrom): + print(ast.dump(ast.parse(node), indent=4)) + for name in node.names: + self._b005_imports.add(f'{node.module}.{name.name or name.asname}') elif isinstance(node, ast.Call): if node.func.attr not in B005.methods: return # method name doesn't match @@ -650,6 +657,15 @@ def check_for_b017(self, node): item = node.items[0] item_context = item.context_expr + #print(ast.dump(ast.parse(item), indent=4)) + + # notes: this prints out ast.Name for raises and ast.Attribute for pytest.raises + # need to modify the logic below to recognize this somehow...avoiding any + # false positives (e.g. for a 'from Dummy import raises' import) + if (isinstance(item_context.func, ast.Name)): + #print(item_context.func.ctx) # this is an ast.Load object, but doesn't "link" to the import statement + print(self._b005_imports) # this is only called for Import, not FromImport...what happens if that changes? + if ( hasattr(item_context, "func") and isinstance(item_context.func, ast.Attribute) From 4e8f6d96b64f0dbba4758c3f10e4f89f070a3516 Mon Sep 17 00:00:00 2001 From: azinneck0485 <123660683+azinneck0485@users.noreply.github.com> Date: Mon, 20 Nov 2023 22:32:28 -0500 Subject: [PATCH 2/6] work out logic to determine if ast.Name node "raises" correspondes to the pytest.raises package --- bugbear.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/bugbear.py b/bugbear.py index 06ec7f2..36b8a78 100644 --- a/bugbear.py +++ b/bugbear.py @@ -559,7 +559,6 @@ def check_for_b005(self, node): for name in node.names: self._b005_imports.add(name.asname or name.name) elif isinstance(node, ast.ImportFrom): - print(ast.dump(ast.parse(node), indent=4)) for name in node.names: self._b005_imports.add(f'{node.module}.{name.name or name.asname}') elif isinstance(node, ast.Call): @@ -657,14 +656,12 @@ def check_for_b017(self, node): item = node.items[0] item_context = item.context_expr - #print(ast.dump(ast.parse(item), indent=4)) - - # notes: this prints out ast.Name for raises and ast.Attribute for pytest.raises - # need to modify the logic below to recognize this somehow...avoiding any - # false positives (e.g. for a 'from Dummy import raises' import) - if (isinstance(item_context.func, ast.Name)): - #print(item_context.func.ctx) # this is an ast.Load object, but doesn't "link" to the import statement - print(self._b005_imports) # this is only called for Import, not FromImport...what happens if that changes? + if (isinstance(item_context.func, ast.Name) + and item_context.func.id == "raises"#: # ast.Name element with id = raises implies a call to a "raises" method + and isinstance(item_context.func.ctx, ast.Load)#): # the "type" of the ast.Name object is a Load() object + and "pytest.raises" in self._b005_imports): + print('pytest.raises has been used') + if ( hasattr(item_context, "func") From fd5f89193b8d8890f47094391d04ec7fdffea25c Mon Sep 17 00:00:00 2001 From: azinneck0485 <123660683+azinneck0485@users.noreply.github.com> Date: Mon, 20 Nov 2023 22:44:01 -0500 Subject: [PATCH 3/6] integrate new logic for B017 to the check itself --- bugbear.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/bugbear.py b/bugbear.py index 36b8a78..ce62d43 100644 --- a/bugbear.py +++ b/bugbear.py @@ -654,25 +654,28 @@ def check_for_b017(self, node): lookup. """ item = node.items[0] - item_context = item.context_expr - - if (isinstance(item_context.func, ast.Name) - and item_context.func.id == "raises"#: # ast.Name element with id = raises implies a call to a "raises" method - and isinstance(item_context.func.ctx, ast.Load)#): # the "type" of the ast.Name object is a Load() object - and "pytest.raises" in self._b005_imports): - print('pytest.raises has been used') - + item_context = item.context_expr if ( hasattr(item_context, "func") - and isinstance(item_context.func, ast.Attribute) and ( - item_context.func.attr == "assertRaises" + ( + isinstance(item_context.func, ast.Attribute) + and ( + item_context.func.attr == "assertRaises" + or ( + item_context.func.attr == "raises" + and isinstance(item_context.func.value, ast.Name) + and item_context.func.value.id == "pytest" + and "match" not in [kwd.arg for kwd in item_context.keywords] + ) + ) + ) or ( - item_context.func.attr == "raises" - and isinstance(item_context.func.value, ast.Name) - and item_context.func.value.id == "pytest" - and "match" not in [kwd.arg for kwd in item_context.keywords] + isinstance(item_context.func, ast.Name) + and item_context.func.id == "raises" + and isinstance(item_context.func.ctx, ast.Load) + and "pytest.raises" in self._b005_imports ) ) and len(item_context.args) == 1 From 011896497ea1428cb04d0bd81e139ba515361250 Mon Sep 17 00:00:00 2001 From: azinneck0485 <123660683+azinneck0485@users.noreply.github.com> Date: Mon, 20 Nov 2023 23:50:13 -0500 Subject: [PATCH 4/6] add check for matches keyword to prevent B017; whitespace corrections to pass tests --- bugbear.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bugbear.py b/bugbear.py index ce62d43..65610f8 100644 --- a/bugbear.py +++ b/bugbear.py @@ -547,7 +547,7 @@ def visit_Import(self, node): self.check_for_b005(node) self.generic_visit(node) - def visit_ImportFrom(self,node): + def visit_ImportFrom(self, node): self.visit_Import(node) def visit_Set(self, node): @@ -654,7 +654,7 @@ def check_for_b017(self, node): lookup. """ item = node.items[0] - item_context = item.context_expr + item_context = item.context_expr if ( hasattr(item_context, "func") @@ -676,6 +676,7 @@ def check_for_b017(self, node): and item_context.func.id == "raises" and isinstance(item_context.func.ctx, ast.Load) and "pytest.raises" in self._b005_imports + and "match" not in [kwd.arg for kwd in item_context.keywords] ) ) and len(item_context.args) == 1 From cffdbc5d0df8d4b258b77ec8458a3759b78597db Mon Sep 17 00:00:00 2001 From: azinneck0485 <123660683+azinneck0485@users.noreply.github.com> Date: Mon, 20 Nov 2023 23:50:54 -0500 Subject: [PATCH 5/6] update tests to capture changes to B017 logic --- tests/b017.py | 12 ++++++++++++ tests/test_bugbear.py | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/b017.py b/tests/b017.py index 2365cd1..af52f6d 100644 --- a/tests/b017.py +++ b/tests/b017.py @@ -7,6 +7,7 @@ import unittest import pytest +from pytest import raises CONSTANT = True @@ -28,31 +29,42 @@ def evil_raises(self) -> None: raise Exception("Evil I say!") with pytest.raises(Exception): raise Exception("Evil I say!") + with raises(Exception): + raise Exception("Evil I say!") # These are evil as well but we are only testing inside a with statement self.assertRaises(Exception, lambda x, y: x / y, 1, y=0) pytest.raises(Exception, lambda x, y: x / y, 1, y=0) + raises(Exception, lambda x, y: x / y, 1, y=0) def context_manager_raises(self) -> None: with self.assertRaises(Exception) as ex: raise Exception("Context manager is good") with pytest.raises(Exception) as pyt_ex: raise Exception("Context manager is good") + with raises(Exception) as r_ex: + raise Exception("Context manager is good") self.assertEqual("Context manager is good", str(ex.exception)) self.assertEqual("Context manager is good", str(pyt_ex.value)) + self.assertEqual("Context manager is good", str(r_ex.value)) def regex_raises(self) -> None: with self.assertRaisesRegex(Exception, "Regex is good"): raise Exception("Regex is good") with pytest.raises(Exception, match="Regex is good"): raise Exception("Regex is good") + with raises(Exception, match="Regex is good"): + raise Exception("Regex is good") def non_context_manager_raises(self) -> None: self.assertRaises(ZeroDivisionError, lambda x, y: x / y, 1, y=0) pytest.raises(ZeroDivisionError, lambda x, y: x / y, 1, y=0) + raises(ZeroDivisionError, lambda x, y: x / y, 1, y=0) def raises_with_absolute_reference(self): with self.assertRaises(asyncio.CancelledError): Foo() with pytest.raises(asyncio.CancelledError): Foo() + with raises(asyncio.CancelledError): + Foo() diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 49377b4..f65df0d 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -258,7 +258,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(25, 8), B017(27, 8), B017(29, 8)) + expected = self.errors(B017(26, 8), B017(28, 8), B017(30, 8), B017(32, 8)) self.assertEqual(errors, expected) def test_b018_functions(self): @@ -530,9 +530,11 @@ def test_b908(self): B908(15, 8), B908(21, 8), B908(27, 8), + B017(37, 0), B908(37, 0), B908(41, 0), B908(45, 0), + B017(56, 0) ) self.assertEqual(errors, expected) From e8ddd376c6e64718e2f820b88bcacfa52365abd0 Mon Sep 17 00:00:00 2001 From: azinneck0485 <123660683+azinneck0485@users.noreply.github.com> Date: Tue, 21 Nov 2023 00:12:27 -0500 Subject: [PATCH 6/6] files modified by pre-commit --- bugbear.py | 5 +++-- tests/test_bugbear.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bugbear.py b/bugbear.py index 65610f8..ee2afd4 100644 --- a/bugbear.py +++ b/bugbear.py @@ -560,7 +560,7 @@ def check_for_b005(self, node): self._b005_imports.add(name.asname or name.name) elif isinstance(node, ast.ImportFrom): for name in node.names: - self._b005_imports.add(f'{node.module}.{name.name or name.asname}') + self._b005_imports.add(f"{node.module}.{name.name or name.asname}") elif isinstance(node, ast.Call): if node.func.attr not in B005.methods: return # method name doesn't match @@ -667,7 +667,8 @@ def check_for_b017(self, node): item_context.func.attr == "raises" and isinstance(item_context.func.value, ast.Name) and item_context.func.value.id == "pytest" - and "match" not in [kwd.arg for kwd in item_context.keywords] + and "match" + not in [kwd.arg for kwd in item_context.keywords] ) ) ) diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index f65df0d..22ccd6a 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -534,7 +534,7 @@ def test_b908(self): B908(37, 0), B908(41, 0), B908(45, 0), - B017(56, 0) + B017(56, 0), ) self.assertEqual(errors, expected)