From 91188aa723bc4389d54feff8dccf6abcf75c2877 Mon Sep 17 00:00:00 2001 From: John Litborn <11260241+jakkdl@users.noreply.github.com> Date: Wed, 18 Jan 2023 01:35:31 +0100 Subject: [PATCH] Rename B028 to B907, making it optional/opinionated. (#333) --- README.rst | 8 +++- bugbear.py | 19 ++++---- tests/{b028.py => b907.py} | 2 +- tests/test_bugbear.py | 90 +++++++++++++++++++------------------- 4 files changed, 62 insertions(+), 57 deletions(-) rename tests/{b028.py => b907.py} (97%) diff --git a/README.rst b/README.rst index f219932..ffb2af3 100644 --- a/README.rst +++ b/README.rst @@ -169,8 +169,6 @@ 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. - Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ @@ -211,6 +209,8 @@ For more information: https://peps.python.org/pep-0618/ Will only trigger on function names where the part after ``visit_`` is a valid ``ast`` type with a non-empty ``_fields`` attribute. This is meant to be enabled by developers writing visitors using the ``ast`` module, such as flake8 plugin writers. +**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. + **B950**: Line too long. This is a pragmatic equivalent of ``pycodestyle``'s ``E501``: it considers "max-line-length" but only triggers when the value has been exceeded by **more than 10%**. You will no @@ -308,6 +308,10 @@ MIT Change Log ---------- +Future +~~~~~~~~~ +* Rename B028 to B907, making it optional/opinionated. + 23.1.14 ~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 182d08b..e479c08 100644 --- a/bugbear.py +++ b/bugbear.py @@ -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): @@ -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): @@ -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),), @@ -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( @@ -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"] diff --git a/tests/b028.py b/tests/b907.py similarity index 97% rename from tests/b028.py rename to tests/b907.py index c4d2d3f..2a0f834 100644 --- a/tests/b028.py +++ b/tests/b907.py @@ -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}}"' diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 8e0a7b2..0fdd24f 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -39,13 +39,13 @@ B025, B026, B027, - B028, B901, B902, B903, B904, B905, B906, + B907, B950, BugBearChecker, BugBearVisitor, @@ -432,47 +432,47 @@ 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) @@ -480,7 +480,7 @@ def test_b028(self): # 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( @@ -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"