Skip to content

Commit

Permalink
B907: fix crash and test failures on py312 (#399)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexWaygood
AlexWaygood authored Jul 1, 2023
1 parent f820ba8 commit c722166
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 76 deletions.
6 changes: 6 additions & 0 deletions README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,12 @@ MIT
Change Log
----------

Unreleased
~~~~~~~~~~

* Fix a crash and several test failures on Python 3.12, all relating to the B907
check.

23.6.5
~~~~~~

Expand Down
54 changes: 26 additions & 28 deletions bugbear.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1245,36 +1245,34 @@ def myunparse(node: ast.AST) -> str: # pragma: no cover
current_mark = None
variable = None
for value in node.values:
# check for quote mark after pre-marked variable
if (
current_mark is not None
and variable is not None
and isinstance(value, ast.Constant)
and isinstance(value.value, str)
and value.value[0] == current_mark
):
self.errors.append(
B907(
variable.lineno,
variable.col_offset,
vars=(myunparse(variable.value),),
)
)
current_mark = variable = None
# don't continue with length>1, so we can detect a new pre-mark
# in the same string as a post-mark, e.g. `"{foo}" "{bar}"`
if len(value.value) == 1:
if isinstance(value, ast.Constant) and isinstance(value.value, str):
if not value.value:
continue

# detect pre-mark
if (
isinstance(value, ast.Constant)
and isinstance(value.value, str)
and value.value[-1] in quote_marks
):
current_mark = value.value[-1]
variable = None
continue
# check for quote mark after pre-marked variable
if (
current_mark is not None
and variable is not None
and value.value[0] == current_mark
):
self.errors.append(
B907(
variable.lineno,
variable.col_offset,
vars=(myunparse(variable.value),),
)
)
current_mark = variable = None
# don't continue with length>1, so we can detect a new pre-mark
# in the same string as a post-mark, e.g. `"{foo}" "{bar}"`
if len(value.value) == 1:
continue

# detect pre-mark
if value.value[-1] in quote_marks:
current_mark = value.value[-1]
variable = None
continue

# detect variable, if there's a pre-mark
if (
Expand Down
4 changes: 2 additions & 2 deletions tests/b907.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ def foo():
f'a "{foo()}" b'

# fmt: off
k = (f'"' # error emitted on this line since all values are assigned the same lineno
k = (f'"' # Error emitted here on <py312 (all values assigned the same lineno)
f'{var}'
f'"'
f'"')

k = (f'"' # error emitted on this line
k = (f'"' # error emitted on this line on <py312
f'{var}'
'"'
f'"')
Expand Down
102 changes: 56 additions & 46 deletions tests/test_bugbear.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
from argparse import Namespace
from pathlib import Path

from hypothesis import HealthCheck, given, settings
from hypothesmith import from_grammar

from bugbear import (
B001,
B002,
Expand Down Expand Up @@ -524,43 +521,51 @@ def test_b907(self):
filename = Path(__file__).absolute().parent / "b907.py"
bbc = BugBearChecker(filename=str(filename))
errors = list(bbc.run())
py39 = sys.version_info >= (3, 9)
py312 = sys.version_info >= (3, 12)

def on_py312(number):
"""F-string nodes have column numbers set to 0 on <py312"""
return number if py312 else 0

expected = self.errors(
B907(8, 0, vars=("var",)),
B907(9, 0, vars=("var",)),
B907(10, 0, vars=("var",)),
B907(12, 0, vars=("var",)),
B907(13, 0, vars=("var",)),
B907(14, 0, vars=("var",)),
B907(16, 0, vars=("'hello'",)),
B907(17, 0, vars=("foo()",)),
B907(20, 5, vars=("var",)),
B907(25, 5, vars=("var",)),
B907(31, 0, vars=("var",)),
B907(32, 0, vars=("var",)),
B907(33, 0, vars=("var",)),
B907(33, 0, vars=("var2",)),
B907(34, 0, vars=("var",)),
B907(34, 0, vars=("var2",)),
B907(35, 0, vars=("var",)),
B907(35, 0, vars=("var2",)),
B907(38, 0, vars=("var2",)),
B907(41, 0, vars=("var",)),
B907(42, 0, vars=("var.__str__",)),
B907(43, 0, vars=("var.__str__.__repr__",)),
B907(44, 0, vars=("3 + 5" if sys.version_info >= (3, 9) else "BinOp",)),
B907(45, 0, vars=("foo()",)),
B907(46, 0, vars=("None",)),
B907(47, 0, vars=("..." if sys.version_info >= (3, 9) else "Ellipsis",)),
B907(48, 0, vars=("True",)),
B907(51, 0, vars=("var",)),
B907(52, 0, vars=("var",)),
B907(53, 0, vars=("var",)),
B907(54, 0, vars=("var",)),
B907(57, 0, vars=("var",)),
B907(60, 0, vars=("var",)),
B907(64, 0, vars=("var",)),
B907(66, 0, vars=("var",)),
B907(68, 0, vars=("var",)),
B907(8, on_py312(9), vars=("var",)),
B907(9, on_py312(3), vars=("var",)),
B907(10, on_py312(9), vars=("var",)),
B907(12, on_py312(9), vars=("var",)),
B907(13, on_py312(3), vars=("var",)),
B907(14, on_py312(9), vars=("var",)),
B907(16, on_py312(5), vars=("'hello'",)),
B907(17, on_py312(5), vars=("foo()",)),
# Multiline f-strings have lineno changes as well as colno changes on py312+
B907(21 if py312 else 20, 7 if py312 else 5, vars=("var",)),
B907(26 if py312 else 25, 7 if py312 else 5, vars=("var",)),
B907(31, on_py312(12), vars=("var",)),
B907(32, on_py312(3), vars=("var",)),
B907(33, on_py312(3), vars=("var",)),
B907(33, on_py312(29), vars=("var2",)),
B907(34, on_py312(3), vars=("var",)),
B907(34, on_py312(15), vars=("var2",)),
B907(35, on_py312(3), vars=("var",)),
B907(35, on_py312(10), vars=("var2",)),
B907(38, on_py312(13), vars=("var2",)),
B907(41, on_py312(3), vars=("var",)),
B907(42, on_py312(3), vars=("var.__str__",)),
B907(43, on_py312(3), vars=("var.__str__.__repr__",)),
B907(44, on_py312(3), vars=("3 + 5" if py39 else "BinOp",)),
B907(45, on_py312(3), vars=("foo()",)),
B907(46, on_py312(3), vars=("None",)),
B907(47, on_py312(3), vars=("..." if py39 else "Ellipsis",)),
B907(48, on_py312(3), vars=("True",)),
B907(51, on_py312(3), vars=("var",)),
B907(52, on_py312(3), vars=("var",)),
B907(53, on_py312(3), vars=("var",)),
B907(54, on_py312(3), vars=("var",)),
B907(57, on_py312(3), vars=("var",)),
B907(60, on_py312(3), vars=("var",)),
B907(64, on_py312(5), vars=("var",)),
B907(66, on_py312(3), vars=("var",)),
B907(68, on_py312(3), vars=("var",)),
)
self.assertEqual(errors, expected)

Expand Down Expand Up @@ -795,13 +800,18 @@ def test_selfclean_test_bugbear(self):


class TestFuzz(unittest.TestCase):
@settings(suppress_health_check=[HealthCheck.too_slow])
@given(from_grammar().map(ast.parse))
def test_does_not_crash_on_any_valid_code(self, syntax_tree):
# Given any syntatically-valid source code, flake8-bugbear should
# not crash. This tests doesn't check that we do the *right* thing,
# just that we don't crash on valid-if-poorly-styled code!
BugBearVisitor(filename="<string>", lines=[]).visit(syntax_tree)
# TODO: enable this test on py312 once hypothesmith supports py312
if sys.version_info < (3, 12):
from hypothesis import HealthCheck, given, settings
from hypothesmith import from_grammar

@settings(suppress_health_check=[HealthCheck.too_slow])
@given(from_grammar().map(ast.parse))
def test_does_not_crash_on_any_valid_code(self, syntax_tree):
# Given any syntatically-valid source code, flake8-bugbear should
# not crash. This tests doesn't check that we do the *right* thing,
# just that we don't crash on valid-if-poorly-styled code!
BugBearVisitor(filename="<string>", lines=[]).visit(syntax_tree)

def test_does_not_crash_on_site_code(self):
# Because the generator isn't perfect, we'll also test on all the code
Expand Down

0 comments on commit c722166

Please sign in to comment.