Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the same special cases for B950 as E501 (#176) #213

Merged
merged 2 commits into from Jan 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -23,6 +23,7 @@ var/
*.egg-info/
.installed.cfg
*.egg
venv/

# PyInstaller
# Usually these files are written by a python script from a template
Expand Down
15 changes: 14 additions & 1 deletion README.rst
Expand Up @@ -173,7 +173,15 @@ significantly violate the line length, you will receive a message that
states what the actual limit is. This is inspired by Raymond Hettinger's
`"Beyond PEP 8" talk <https://www.youtube.com/watch?v=wf-BqAjZb8M>`_ and
highway patrol not stopping you if you drive < 5mph too fast. Disable
E501 to avoid duplicate warnings.
E501 to avoid duplicate warnings. Like E501, this error ignores long shebangs
on the first line and urls or paths that are on their own line::

#! long shebang ignored

# https://some-super-long-domain-name.com/with/some/very/long/paths
url = (
"https://some-super-long-domain-name.com/with/some/very/long/paths"
)


How to enable opinionated warnings
Expand Down Expand Up @@ -237,6 +245,11 @@ MIT
Change Log
----------

21.12.0
~~~~~~~~~~

* B950: Add same special cases as E501 (#213)

21.11.29
~~~~~~~~~~

Expand Down
25 changes: 24 additions & 1 deletion bugbear.py
Expand Up @@ -12,7 +12,7 @@
import attr
import pycodestyle

__version__ = "21.11.29"
__version__ = "21.12.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets not do this here and I'll do it on a diff when I release (as it's going to be 22.*)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed


LOG = logging.getLogger("flake8.bugbear")
CONTEXTFUL_NODES = (
Expand Down Expand Up @@ -67,8 +67,31 @@ def gen_line_based_checks(self):
The following simple checks are based on the raw lines, not the AST.
"""
for lineno, line in enumerate(self.lines, start=1):
# Special case: ignore long shebang (following pycodestyle).
if lineno == 1 and line.startswith("#!"):
continue

length = len(line) - 1
if length > 1.1 * self.max_line_length:
# Special case long URLS and paths to follow pycodestyle.
# Would use the `pycodestyle.maximum_line_length` directly but
# need to supply it arguments that are not available so chose
# to replicate instead.
chunks = line.split()

is_line_comment_url_path = len(chunks) == 2 and chunks[0] == "#"

just_long_url_path = len(chunks) == 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I read this right, really this is just a long no white space string right? So it could not be a URL or file path? I guess generally we'll have spaces I guess is the rationale here right?

Is this the same as how pycodestyle does it? if so, (Sorry - being lazy and not checking), then I guess it's proven to be safe enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a long string with no white space, which is mostly just URLs and paths. A path that has a space in it will still fail but would also fail E501. Rational is it is more readable to not split up URLs and paths in comments and code.

It's the same logic as pycodestyle, modified to be a bit more readable to me. Linking here for convenience.


num_leading_whitespaces = len(line) - len(chunks[-1])
too_many_leading_white_spaces = (
num_leading_whitespaces >= self.max_line_length - 7
)

skip = is_line_comment_url_path or just_long_url_path
if skip and not too_many_leading_white_spaces:
continue

yield B950(lineno, length, vars=(length, self.max_line_length))

@classmethod
Expand Down
14 changes: 14 additions & 0 deletions tests/b950.py
@@ -1,7 +1,21 @@
#! Ignore long shebang fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
# Assumes the default allowed line length of 79

"line is fine"
" line is fine "
" line is still fine "
" line is no longer fine by any measures, yup"
"line is fine again"

# Ensure URL/path on it's own line is fine
"https://foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.com"
"NOT OK: https://foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.com"
# https://foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.com
# NOT OK: https://foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.com
#
#: Okay
# This
# almost_empty_line_too_long

# This
# almost_empty_line_too_long
10 changes: 9 additions & 1 deletion tests/test_bugbear.py
Expand Up @@ -318,7 +318,15 @@ def test_b950(self):
filename = Path(__file__).absolute().parent / "b950.py"
bbc = BugBearChecker(filename=str(filename))
errors = list(bbc.run())
self.assertEqual(errors, self.errors(B950(6, 92, vars=(92, 79))))
self.assertEqual(
errors,
self.errors(
B950(7, 92, vars=(92, 79)),
B950(12, 103, vars=(103, 79)),
B950(14, 103, vars=(103, 79)),
B950(21, 97, vars=(97, 79)),
),
)

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