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.

Sign up for GitHub

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

Jump to bottom

PyLint Comments Work With Other Linter Comments #2537

Closed
wants to merge 5 commits into from
Closed

PyLint Comments Work With Other Linter Comments #2537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
277a27c
PyLint Comments Work With Other Linter Comments
DevynCJohnson Oct 7, 2018
6892c6e
PyLint Comments Work With Other Linter Comments
DevynCJohnson Oct 7, 2018
1d67786
PyLint Comments Work With Other Linter Comments
DevynCJohnson Oct 7, 2018
c06bcc8
Added a Contributor
DevynCJohnson Oct 7, 2018
8093191
PyLint Comments Work With Other Linter Comments
DevynCJohnson Oct 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CONTRIBUTORS.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,5 @@ contributors:
* Scott Worley: contributor

* Michael Hudson-Doyle

* Devyn Collier Johnson: Fixes bugs
10 changes: 10 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ What's New in Pylint 2.2?

Release date: TBA

* PyLint comments now officially work with other linter comments

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure your PR actually fixes all these bugs? From what I'm noticing:

Unless you are testing with a completely different code than the one posted in the aforementioned comment, I don't think your PR "fixes" these issues, as they are already fixed. What instead is fixing is that it correctly supports long lines and additional disables, but that's a separate bug.

This is the solution that officially closes [#2297](https://github.com/PyCQA/pylint/issues/2297),
[#2470](https://github.com/PyCQA/pylint/issues/2470),
and [#2485](https://github.com/PyCQA/pylint/issues/2485).

Close #2297
Close #2470
Close #2485

* ``deprecated-method`` can use the attribute name for identifying a deprecated method

Previously we were using the fully qualified name, which we still do, but the fully
Expand Down
9 changes: 7 additions & 2 deletions pylint/checkers/format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1260,8 +1260,13 @@ def check_line(line, i):
_msg_id.strip() for _msg_id in back_of_equal.split(",")
}:
return None
line = line.rsplit("#", 1)[0].rstrip()

# Find the Pylint comment among all other special comments on the line
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a better solution would be to use re.sub:

line = OPTION_RGX.sub("", line)

_comments = line.rsplit(r'#')
for _msg in _comments:
_msg = _msg.strip()
if _msg.startswith(r'pylint:') and r'disable' in _msg:
line = _msg
break
if len(line) > max_chars and not ignore_long_line.search(line):
self.add_message("line-too-long", line=i, args=(len(line), max_chars))
return i + 1
Expand Down
8 changes: 7 additions & 1 deletion pylint/lint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,13 @@ def process_tokens(self, tokens):
match = utils.OPTION_RGX.search(content)
if match is None:
continue

# Find the Pylint comment among all other special comments on the line
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense, OPTION_RGX should be enough to find the corresponding pragma control. It also assumes that we only have disable, which is not the case.

_comments = content.rsplit(r'#')
for _msg in _comments:
_msg = _msg.strip()
if _msg.startswith(r'pylint:') and r'disable' in _msg:
content = r'#' + _msg
break
first_group = match.group(1)
if (
first_group.strip() == "disable-all"
Expand Down
2 changes: 1 addition & 1 deletion pylint/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
# Allow stopping after the first semicolon/hash encountered,
# so that an option can be continued with the reasons
# why it is active or disabled.
OPTION_RGX = re.compile(r"\s*#.*\bpylint:\s*([^;#]+)[;#]{0,1}")
OPTION_RGX = re.compile(r'.*#\s*pylint:\s*([^;#]+)[;#]?.*')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think replacing .* with \s* from after the hash makes sense, but not the rest of the changes.


# The line/node distinction does not apply to fatal errors and reports.
_SCOPE_EXEMPT = "FR"
Expand Down