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

Make #novermin comments work better with other linter comments #43

Closed
gousaiyang opened this issue Sep 20, 2020 · 2 comments
Closed

Make #novermin comments work better with other linter comments #43

gousaiyang opened this issue Sep 20, 2020 · 2 comments

Comments

@gousaiyang
Copy link
Contributor

Sometimes we need to use multiple linter comments in one line (see also pylint-dev/pylint#2485). Consider the following code snippet, which has three linter comments in one line:

import os

if hasattr(os, 'sched_getaffinity'):
    CPU_CNT = len(os.sched_getaffinity(0))  # type: ignore[attr-defined] # pylint: disable=no-member # novermin
else:
    ...

It would be good if everything works no matter how these comments are ordered, which is the current behavior of Pylint and Flake8. But for vermin, currently it only works (i.e. correctly exclude the line from analysis) when # novermin is the last comment. # novermin # type: ignore[attr-defined] # pylint: disable=no-member and # type: ignore[attr-defined] # novermin # pylint: disable=no-member do not work (for some reason Mypy requires the type comment to be the first comment, but that's not a vermin problem.)

I see a relevant change in the master branch. A caveat is that extracting comments with this regular expression may introduce false positive # novermin comments if # novermin happens to be in a string literal. Maybe using the tokenize module to detect COMMENT tokens is a better way.

Environment

  • Vermin version 0.10.3
@netromdk
Copy link
Owner

netromdk commented Sep 21, 2020

Thanks, that is an interesting proposal with tokenize!

I've thought about this before; mainly that it's a mess when using several solutions relying on comments at the same time. I do think it's a solution if the different "contexts" are defined as code ((# comment) ..)

netromdk added a commit that referenced this issue Sep 21, 2020
With these changes, "novermin" and "novm" can be detected with multiple comment segments, like:
```
 # type: ignore[attr-defined] # novm # pylint: disable=no-member
 import email.parser.FeedParser
```
@netromdk
Copy link
Owner

I got it working. And also double-checked with your input case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants