Skip to content

Commit

Permalink
Rename B028 to B907, making it optional/opinionated.
Browse files Browse the repository at this point in the history
  • Loading branch information
jakkdl committed Jan 16, 2023
1 parent 4a0db1a commit e76acd0
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 56 deletions.
6 changes: 5 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ limitations make it difficult.

**B027**: Empty method in abstract base class, but has no abstract decorator. Consider adding @abstractmethod.

**B028**: Consider replacing ``f"'{foo}'"`` with ``f"{foo!r}"`` which is both easier to read and will escape quotes inside ``foo`` if that would appear. The check tries to filter out any format specs that are invalid together with ``!r``. If you're using other conversion flags then e.g. ``f"'{foo!a}'"`` can be replaced with ``f"{ascii(foo)!r}"``. Not currently implemented for python<3.8 or ``str.format()`` calls.
**B907**: Consider replacing ``f"'{foo}'"`` with ``f"{foo!r}"`` which is both easier to read and will escape quotes inside ``foo`` if that would appear. The check tries to filter out any format specs that are invalid together with ``!r``. If you're using other conversion flags then e.g. ``f"'{foo!a}'"`` can be replaced with ``f"{ascii(foo)!r}"``. Not currently implemented for python<3.8 or ``str.format()`` calls.

Opinionated warnings
~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -308,6 +308,10 @@ MIT
Change Log
----------

Future
~~~~~~~~~
* Rename B028 to B907, making it optional/opinionated.

23.1.14
~~~~~~~~~

Expand Down
19 changes: 10 additions & 9 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ def visit_With(self, node):
self.generic_visit(node)

def visit_JoinedStr(self, node):
self.check_for_b028(node)
self.check_for_b907(node)
self.generic_visit(node)

def check_for_b005(self, node):
Expand Down Expand Up @@ -1014,7 +1014,7 @@ def check_for_b906(self, node: ast.FunctionDef):
else:
self.errors.append(B906(node.lineno, node.col_offset))

def check_for_b028(self, node: ast.JoinedStr): # noqa: C901
def check_for_b907(self, node: ast.JoinedStr): # noqa: C901
# AST structure of strings in f-strings in 3.7 is different enough this
# implementation doesn't work
if sys.version_info <= (3, 7):
Expand Down Expand Up @@ -1048,7 +1048,7 @@ def myunparse(node: ast.AST) -> str: # pragma: no cover
and value.value[0] == current_mark
):
self.errors.append(
B028(
B907(
variable.lineno,
variable.col_offset,
vars=(myunparse(variable.value),),
Expand Down Expand Up @@ -1485,12 +1485,6 @@ def visit_Lambda(self, node):
" decorator. Consider adding @abstractmethod."
)
)
B028 = Error(
message=(
"B028 {!r} is manually surrounded by quotes, consider using the `!r` conversion"
" flag."
)
)

# Warnings disabled by default.
B901 = Error(
Expand Down Expand Up @@ -1539,6 +1533,13 @@ def visit_Lambda(self, node):
)
)

B907 = Error(
message=(
"B907 {!r} is manually surrounded by quotes, consider using the `!r` conversion"
" flag."
)
)

B950 = Error(message="B950 line too long ({} > {} characters)")

disabled_by_default = ["B901", "B902", "B903", "B904", "B905", "B906", "B950"]
2 changes: 1 addition & 1 deletion tests/b028.py → tests/b907.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def foo():
# alignment specifier invalid for strings
f'"{var:=}"'

# other types and combinations are tested in test_b028_format_specifier_permutations
# other types and combinations are tested in test_b907_format_specifier_permutations

# don't attempt to parse complex format specs
f'"{var:{var}}"'
Expand Down
90 changes: 45 additions & 45 deletions tests/test_bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@
B025,
B026,
B027,
B028,
B901,
B902,
B903,
B904,
B905,
B906,
B907,
B950,
BugBearChecker,
BugBearVisitor,
Expand Down Expand Up @@ -432,55 +432,55 @@ def test_b027(self):
self.assertEqual(errors, expected)

@unittest.skipIf(sys.version_info < (3, 8), "not implemented for <3.8")
def test_b028(self):
filename = Path(__file__).absolute().parent / "b028.py"
def test_b907(self):
filename = Path(__file__).absolute().parent / "b907.py"
bbc = BugBearChecker(filename=str(filename))
errors = list(bbc.run())
expected = self.errors(
B028(8, 0, vars=("var",)),
B028(9, 0, vars=("var",)),
B028(10, 0, vars=("var",)),
B028(12, 0, vars=("var",)),
B028(13, 0, vars=("var",)),
B028(14, 0, vars=("var",)),
B028(16, 0, vars=("'hello'",)),
B028(17, 0, vars=("foo()",)),
B028(20, 5, vars=("var",)),
B028(25, 5, vars=("var",)),
B028(31, 0, vars=("var",)),
B028(32, 0, vars=("var",)),
B028(33, 0, vars=("var",)),
B028(33, 0, vars=("var2",)),
B028(34, 0, vars=("var",)),
B028(34, 0, vars=("var2",)),
B028(35, 0, vars=("var",)),
B028(35, 0, vars=("var2",)),
B028(38, 0, vars=("var2",)),
B028(41, 0, vars=("var",)),
B028(42, 0, vars=("var.__str__",)),
B028(43, 0, vars=("var.__str__.__repr__",)),
B028(44, 0, vars=("3 + 5" if sys.version_info >= (3, 9) else "BinOp",)),
B028(45, 0, vars=("foo()",)),
B028(46, 0, vars=("None",)),
B028(47, 0, vars=("..." if sys.version_info >= (3, 9) else "Ellipsis",)),
B028(48, 0, vars=("True",)),
B028(51, 0, vars=("var",)),
B028(52, 0, vars=("var",)),
B028(53, 0, vars=("var",)),
B028(54, 0, vars=("var",)),
B028(57, 0, vars=("var",)),
B028(60, 0, vars=("var",)),
B028(64, 0, vars=("var",)),
B028(66, 0, vars=("var",)),
B028(68, 0, vars=("var",)),
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",)),
)
self.assertEqual(errors, expected)

# manual permutations to save overhead when doing >60k permutations
# see format spec at
# https://docs.python.org/3/library/string.html#format-specification-mini-language
@unittest.skipIf(sys.version_info < (3, 8), "not implemented for <3.8")
def test_b028_format_specifier_permutations(self):
def test_b907_format_specifier_permutations(self):
visitor = BugBearVisitor(filename="", lines="")

for fields in itertools.product(
Expand Down Expand Up @@ -509,27 +509,27 @@ def test_b028_format_specifier_permutations(self):
except ValueError:
assert (
visitor.errors == []
), f"b028 raised for {format_spec!r} not valid for string"
), f"b907 raised for {format_spec!r} not valid for string"
continue

new = ("{!r:" + format_spec + "}").format("hello")

# Preceding the width field by 0 in >=3.10 is valid, but does nothing.
# The presence of it means likely numeric variable though.
# A width shorter than the string will look the same, but should not give b028.
# A width shorter than the string will look the same, but should not give b907.
if fields[5] == "0" or fields[6] == "1":
assert (
visitor.errors == []
), f"b028 should not raise on questionable case {format_spec}"
), f"b907 should not raise on questionable case {format_spec}"
elif old == new:
assert visitor.errors, (
f"b028 not raised for {format_spec} that would look identical"
f"b907 not raised for {format_spec} that would look identical"
" with !r"
)
else:
assert (
visitor.errors == []
), f"b028 raised for {format_spec} that would look different with !r"
), f"b907 raised for {format_spec} that would look different with !r"

def test_b901(self):
filename = Path(__file__).absolute().parent / "b901.py"
Expand Down

0 comments on commit e76acd0

Please sign in to comment.