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

2.7.3 regression ignores all disabled warnings #4265

Closed
fmigneault opened this issue Mar 29, 2021 · 6 comments · Fixed by #4268
Closed

2.7.3 regression ignores all disabled warnings #4265

fmigneault opened this issue Mar 29, 2021 · 6 comments · Fixed by #4268
Assignees
Labels
Blocker 🙅 Blocks the next release Regression
Milestone

Comments

@fmigneault
Copy link

Steps to reproduce

Moving from 2.7.2 to 2.7.3, my repo goes from no error to hundreds of problems.
I didn't go through all of them, but many (if not all) seem to be related to ignored errors.
My config: https://github.com/Ouranosinc/Magpie/blob/master/.pylintrc

Current behavior

All these errors are flagged, when they are not just a patch version before.

Messages
--------

+-------------------------------+------------+
|message id                     |occurrences |
+===============================+============+
|unnecessary-lambda             |54          |
+-------------------------------+------------+
|ungrouped-imports              |46          |
+-------------------------------+------------+
|no-self-use                    |30          |
+-------------------------------+------------+
|super-with-arguments           |27          |
+-------------------------------+------------+
|too-many-locals                |25          |
+-------------------------------+------------+
|broad-except                   |21          |
+-------------------------------+------------+
|import-outside-toplevel        |17          |
+-------------------------------+------------+
|cell-var-from-loop             |17          |
+-------------------------------+------------+
|useless-object-inheritance     |14          |
+-------------------------------+------------+
|unused-argument                |12          |
+-------------------------------+------------+
|comparison-with-callable       |11          |
+-------------------------------+------------+
|invalid-envvar-default         |10          |
+-------------------------------+------------+
|wrong-import-order             |9           |
+-------------------------------+------------+
|raise-missing-from             |8           |
+-------------------------------+------------+
|inconsistent-return-statements |6           |
+-------------------------------+------------+
|use-a-generator                |5           |
+-------------------------------+------------+
|too-many-public-methods        |5           |
+-------------------------------+------------+
|len-as-condition               |4           |
+-------------------------------+------------+
|useless-super-delegation       |2           |
+-------------------------------+------------+
|unnecessary-comprehension      |2           |
+-------------------------------+------------+
|too-many-statements            |2           |
+-------------------------------+------------+
|too-many-branches              |2           |
+-------------------------------+------------+
|try-except-raise               |1           |
+-------------------------------+------------+
|import-error                   |1           |
+-------------------------------+------------+

-------------------------------------------------------------------
Your code has been rated at 9.77/10 (previous run: 10.00/10, -0.23)

Expected behavior

No more warning/errors than before, as code didn't change at all.

pylint --version output

Result of pylint --version output:

pylint 2.7.3
astroid 2.5.2
Python 3.7.9 (default, Aug 31 2020, 12:42:55) 
[GCC 7.3.0]
@cdce8p
Copy link
Member

cdce8p commented Mar 30, 2021

Please try reformatting your .pylintrc file:

disable=
    missing-docstring,  # C0111
    too-many-lines,  # C0302

The , after each entry is required (except for the last one). I noticed that it was missing after: no-self-use.

@cdce8p
Copy link
Member

cdce8p commented Mar 30, 2021

@Pierre-Sassoulas f501b87 seems to cause this behavior. My suggestion works, so I'm not sure how to best deal with it.

I used this for testing. Python file a.py, pylintrc: customrc

# pylint: disable=unused-import,import-error,missing-docstring
import const

import typing
[MESSAGES CONTROL]

disable=
    C0111,missing-docstring,
    C0302,too-many-lines,
    C0330,bad-continuation,
    C0411,wrong-import-order,
pylint --rcfile customrc a.py

After the commit, it outputs a wrong-import-order error.

@calvincheng8
Copy link

I have the following config for test.rc,

[MESSAGES CONTROL]
disable=C0111,C0326,W0703

Pylint generates a warning W0703 that should have been ignored on the following file test.py,

try:
    f = open('test')
except Exception:
    pass
$ pylint --rcfile `test.rc` test.py
************* Module test
test.py:3:7: W0703: Catching too general exception Exception (broad-except)

------------------------------------------------------------------
Your code has been rated at 7.50/10 (previous run: 7.50/10, +0.00)

Removing C0326 from the disable list and the disable works as expected.

Pylint probably is failing silently when it encounters C0326.

Running on command line confirms this,

$ pylint -d C0326 test.py

throws a KeyError: 'C0326'.

@Pierre-Sassoulas Pierre-Sassoulas added Regression Blocker 🙅 Blocks the next release labels Mar 30, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.7.4 milestone Mar 30, 2021
@Pierre-Sassoulas Pierre-Sassoulas self-assigned this Mar 30, 2021
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Mar 30, 2021
@Pierre-Sassoulas
Copy link
Member

My bad, I did not add a functional test for disabling with pylintrc when I refactored this. I'm going to release 2.7.4 and 3.0.0a1 immediately as it is very problematic.

@Pierre-Sassoulas Pierre-Sassoulas linked a pull request Mar 30, 2021 that will close this issue
Pierre-Sassoulas added a commit that referenced this issue Mar 30, 2021
* Fix ignores all disabled warnings #4265

* Update changelog and prepare 2.7.4
@fmigneault
Copy link
Author

@cdce8p
Thanks for spotting that!

Is there any chance (or how if already possible) this kind of thing can be detected and flagged as warning?
I imagine the disable field gets parsed and split into individual components, and could easily indicate that some definition is unknown by printing its name. Since it wouldn't be split, it would print no-self-useR0205 in my case and I could quickly figure out that a parsing error is the cause.

@Pierre-Sassoulas
Copy link
Member

I'm working on it right now @fmigneault :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker 🙅 Blocks the next release Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants