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

"--notes" option ignores note tags that are entirely punctuation #5840

Closed
dbrookman opened this issue Feb 25, 2022 · 4 comments · Fixed by #5859
Closed

"--notes" option ignores note tags that are entirely punctuation #5840

dbrookman opened this issue Feb 25, 2022 · 4 comments · Fixed by #5859
Labels
False Negative 🦋 No message is emitted but something is wrong with the code
Milestone
2.13.0

Comments

@dbrookman
Copy link
Contributor

Bug description

If a note tag specified with the --notes option is entirely punctuation, pylint won't report a fixme warning (W0511).

# YES: yes
# ???: no

pylint test.py --notes="YES,???" will return a fixme warning (W0511) for the first line, but not the second.

Configuration

Default

Command used

pylint test.py --notes="YES,???"

Pylint output

************* Module test
test.py:1:1: W0511: YES: yes (fixme)

Expected behavior

************* Module test
test.py:1:1: W0511: YES: yes (fixme)
test.py:2:1: W0511: ???: no (fixme)

Pylint version

pylint 2.12.2
astroid 2.9.0
Python 3.10.2 (main, Feb  2 2022, 05:51:25) [Clang 13.0.0 (clang-1300.0.29.3)]

OS / Environment

macOS 11.6.1

Additional dependencies

No response
@dbrookman dbrookman added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Feb 25, 2022
@DanielNoord DanielNoord added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 25, 2022
@Pierre-Sassoulas Pierre-Sassoulas added False Negative 🦋 No message is emitted but something is wrong with the code and removed False Positive 🦟 A message is emitted but nothing is wrong with the code labels Feb 26, 2022
@DanielNoord
Copy link
Collaborator

DanielNoord commented Mar 3, 2022 •
edited
Loading

Did a little investigation, this is we're actually converting this option in a regular expression pattern (thereby making it awfully similar to the notes-rgx option). Since ? is a special character in regex this doesn't get picked up. Using \?\?\? in either notes or notes-rgx should work.

dbrookman added a commit to dbrookman/pylint that referenced this issue Mar 3, 2022
@dbrookman
Fix matching note tags with a non-word char last
f13eb0a 
Using "\b" at the end of these patterns will only match note tags that
end in an alphanumeric character, immediately followed by a
non-alphanumeric character, or the end of the string. This is due to
"\b" being defined as a boundary between a word character ("\w") and a
non-word character ("\W"), or the end of the string. This leads to
deviations like "???" being ignored when specified.

Swapping "\b" for a positive lookahead that targets a whitespace, a
colon, or the end of a string accounts for this.

Closes pylint-dev#5840.
@dbrookman dbrookman mentioned this issue Mar 4, 2022
Merged
@dbrookman
Copy link
Contributor Author

Turned out to be the \b ending both regex patterns, which would only match tags if the last character was alphanumeric.

@Pierre-Sassoulas
Copy link
Member

(thereby making it awfully similar to the notes-rgx option

I'm keeping the issue open for this reason, it seems we should not treat notes as a regex.

@Pierre-Sassoulas Pierre-Sassoulas added Minor 💅 Polishing pylint is always nice and removed Minor 💅 Polishing pylint is always nice labels Mar 4, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Mar 4, 2022
@DanielNoord
Copy link
Collaborator

@Pierre-Sassoulas I think I was wrong. Due to the use of re.escape I don't think there are any issues with escape characters and it was indeed the problem @dbrookman found. I think we can close this (after the merge).

Pierre-Sassoulas pushed a commit that referenced this issue Mar 4, 2022
@dbrookman
Fix matching note tags with a non-word char last (#5859)
a1df768 
Using "\b" at the end of these patterns will only match note tags that
end in an alphanumeric character, immediately followed by a
non-alphanumeric character, or the end of the string. This is due to
"\b" being defined as a boundary between a word character ("\w") and a
non-word character ("\W"), or the end of the string. This leads to
deviations like "???" being ignored when specified.

Swapping "\b" for a positive lookahead that targets a whitespace, a
colon, or the end of a string accounts for this.

Closes #5840.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
False Negative 🦋 No message is emitted but something is wrong with the code
Projects
None yet
Milestone
2.13.0
Development

Successfully merging a pull request may close this issue.

Fix matching note tags with a non-word char last dbrookman/pylint
3 participants
@Pierre-Sassoulas @DanielNoord @dbrookman