Skip to content

[RFE] warn about unused Pylint disablings #4757

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
Closed
stanislavlevin opened this issue Jul 27, 2021 · 5 comments · Fixed by #4779
Closed

[RFE] warn about unused Pylint disablings #4757

stanislavlevin opened this issue Jul 27, 2021 · 5 comments · Fixed by #4779
Assignees
@Pierre-Sassoulas
Labels
Discussion 🤔 Documentation 📗 Enhancement ✨ Improvement to a component
Milestone
2.10.0

Comments

@stanislavlevin
Copy link
Contributor

Current problem

The project which actively uses Pylint can contain many disabling of Pylint checkers via the code comments:
http://pylint.pycqa.org/en/latest/user_guide/message-control.html

  • this can significantly litter a code with tons of comments (Pylint, flake, mypy, etc., individually or all together)
  • this can hide actual code errors
  • slows down Pylint analysis

Desired solution

In my opinion, people disable checkers via the code comments in the next common cases:

  • unsupported Pylint feature
  • broken Pylint feature (for example, regression after upgrade)

I think it will be useful to have an option to check such disabling and report if disabling is no longer needed (only code comments).

Inspired by mypy's:

--warn-unused-ignores
This flag will make mypy report an error whenever your code uses a # type: ignore comment on a line that is not actually generating an error message.

This flag, along with the --warn-redundant-casts flag, are both particularly useful when you are upgrading mypy. Previously, you may have needed to add casts or # type: ignore annotations to work around bugs in mypy or missing stubs for 3rd party libraries.

These two flags let you discover cases where either workarounds are no longer necessary.

https://mypy.readthedocs.io/en/stable/command_line.html?highlight=unused-ignores#cmdoption-mypy-warn-unused-ignores

(Additional context)

No response
@stanislavlevin stanislavlevin added Enhancement ✨ Improvement to a component Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jul 27, 2021
@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Jul 27, 2021 •
edited
Loading

This already exists you need enable=useless-suppression in the configuration. Maybe it should become a default or be more visible in the documentation, what do you think ?

@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 Documentation 📗 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jul 27, 2021
@stanislavlevin
Copy link
Contributor Author

Note: double p in the name (useless-suppression) ;-)
It seems that this is exactly what I want. Unfortunately, I didn't find any docs on this.

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Jul 29, 2021 •
edited
Loading

Thanks for the correction :) Where would you expect to find it ? Or alternatively I think we could make this one a default message as it permits to really clean code when upgrading pylint.

@stanislavlevin
Copy link
Contributor Author

In my opinion, as a default checker for relatively new projects it will be useful, but for old ones, this may be cumbersome at the very first time.
For example, in freeipa case:

[builder@localhost freeipa]$ grep -c useless-suppression log
152

though all of the occurrences will be fixed.

Probably, I would search for such functionality in
http://pylint.pycqa.org/en/latest/user_guide/message-control.html

@Pierre-Sassoulas Pierre-Sassoulas self-assigned this Jul 30, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.10.0 milestone Jul 30, 2021
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Jul 31, 2021
@Pierre-Sassoulas
Add documentation for useless-suppression
ffcfa5d 
Closes pylint-dev#4757
@Pierre-Sassoulas Pierre-Sassoulas mentioned this issue Jul 31, 2021
Merged
Pierre-Sassoulas added a commit that referenced this issue Jul 31, 2021
@Pierre-Sassoulas
Add documentation for useless-suppression
a31e6bc 
Closes #4757
cdce8p added a commit to cdce8p/pylint that referenced this issue Aug 1, 2021
@cdce8p
PR 4783 - nested attributes
b0164a6 
Squashed commit of the following:

commit 49c4bba
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Aug 1 19:56:51 2021 +0800

    Fix crash for `unused-private-member` when there are nested attributes

commit 2ad8247
Merge: 8ceb26d 1d09bc8
Author: yushao2 <[email protected]>
Date:   Sun Aug 1 20:13:05 2021 +0800

    Merge pull request pylint-dev#4709 from yushao2/fix-unused-private-member-4673

    [unused-private-member] add logic for checking nested functions

commit 1d09bc8
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Aug 1 20:03:42 2021 +0800

    update pr based on review

commit a4198cd
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Aug 1 19:21:36 2021 +0800

    Update pr based on review

commit c8b2cbb
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Sun Jul 25 05:20:42 2021 +0000

    [pre-commit.ci] auto fixes from pre-commit.com hooks

    for more information, see https://pre-commit.ci

commit 4ffea0b
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Jul 25 13:12:29 2021 +0800

    Update pr based on review

commit e4d6243
Author: Marc Mueller <[email protected]>
Date:   Sun Jul 18 17:23:31 2021 +0200

    Remove empty line

commit cce5833
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Jul 18 22:40:54 2021 +0800

    update PR based on comments

commit 712dc2b
Author: Yu Shao, Pang <[email protected]>
Date:   Wed Jul 14 16:42:25 2021 +0800

    [unused-private-member] add logic for checking nested functions

    also, improve error message for nested functions

commit 8ceb26d
Author: Michal Vasilek <[email protected]>
Date:   Sun Aug 1 08:14:58 2021 +0200

    Fix IsADirectoryError in tests/lint/unittest_lint (pylint-dev#4781)

    pylintd is a directory, so os.remove throws IsADirectoryError

commit a31e6bc
Author: Pierre Sassoulas <[email protected]>
Date:   Sat Jul 31 11:21:46 2021 +0200

    Add documentation for useless-suppression

    Closes pylint-dev#4757

commit b71be8a
Author: Mark Byrne <[email protected]>
Date:   Fri Jul 30 20:21:02 2021 +0200

    Handle has-a relationships for type-hinted arguments in class diagrams (pylint-dev#4745)

    * Pyreverse - Show class has-a relationships inferred from type-hints

    Closes pylint-dev#4744

    Co-authored-by: Pierre Sassoulas <[email protected]>

commit 5e5f48d
Author: Daniël van Noord <[email protected]>
Date:   Thu Jul 29 23:44:30 2021 +0200

    Add ``forgotten-debug-statement`` checker (pylint-dev#4771)

    * Add ``no-breakpoint`` checker this adds a checker for calls to ``breakpoint()``, ``pdb.set_trace()``, or ``sys.breakpointhook()``.

    Closes pylint-dev#3692

    Co-authored-by: Marc Mueller <[email protected]>
    Co-authored-by: Pierre Sassoulas <[email protected]>
@stanislavlevin
Copy link
Contributor Author

Great! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Pierre-Sassoulas Pierre-Sassoulas

Labels
Discussion 🤔 Documentation 📗 Enhancement ✨ Improvement to a component
Projects
None yet
Milestone
2.10.0
Development

Successfully merging a pull request may close this issue.

Add documentation for useless-suppression Pierre-Sassoulas/pylint
2 participants
@Pierre-Sassoulas @stanislavlevin