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

LGTM.com - false positive - 'noqa' suppression comments #6517

Open
DimitriPapadopoulos opened this issue Aug 19, 2021 · 4 comments
Open

LGTM.com - false positive - 'noqa' suppression comments #6517

DimitriPapadopoulos opened this issue Aug 19, 2021 · 4 comments

Comments

@DimitriPapadopoulos
Copy link

DimitriPapadopoulos commented Aug 19, 2021

Description of the false positive

LGTM.com rightfully complains that the Except block handles 'BaseException'. No worries here.

This issue is about suppression comments. Each linter comes with its own snytax:

  • noqa:E722 for Flake8
  • lgtm [py/catch-base-exception] for LGTM.com

Adding a different comment for each linter may end up with long and confusing lines of code. The standard for Python is Flake8 and its noqa comments. It would make sense to support Flake8 suppression comments in addition to LGTM.com suppression comments, when possible.

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/nilearn/nilearn/snapshot/0cd925e33787e277990b524efeba0c8125d04816/files/nilearn/datasets/func.py?sort=name&dir=ASC&mode=heatmap#x38df926fe033f5ae:1

@DimitriPapadopoulos DimitriPapadopoulos changed the title LGTM.com - false positive - suggestion about 'noqa' suppres comments LGTM.com - false positive - suggestion about 'noqa' suppresion comments Aug 19, 2021
@DimitriPapadopoulos DimitriPapadopoulos changed the title LGTM.com - false positive - suggestion about 'noqa' suppresion comments LGTM.com - false positive - suggestion about 'noqa' suppression comments Aug 19, 2021
@DimitriPapadopoulos DimitriPapadopoulos changed the title LGTM.com - false positive - suggestion about 'noqa' suppression comments LGTM.com - false positive - 'noqa' suppression comments Aug 19, 2021
@tausbn
Copy link
Contributor

tausbn commented Aug 19, 2021

It should already be possible to use noqa as a suppression comment on LGTM.com, exactly for the reasons you specify.

(Implemented here: https://github.com/github/codeql/blob/main/python/ql/src/analysis/AlertSuppression.ql#L74)

@tausbn
Copy link
Contributor

tausbn commented Aug 19, 2021

Ah, I should mention that this only works for the undecorated noqa form, not the one that mentions a specific rule. This is why it isn't working currently in the link you give above.

@DimitriPapadopoulos
Copy link
Author

DimitriPapadopoulos commented Aug 19, 2021

Thanks for the link. While it's not a small endeavour to link Flake8 error codes to LGTM.com ones, when possible, it would be a worthwhile addition.

In the short term, you could detect \\s*noqa:E in addition to \\s*noqa\\s*.

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

Successfully merging a pull request may close this issue.

2 participants