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

useless-suppression false positive with ungrouped-imports #2366

Closed
mthuurne opened this issue Jul 30, 2018 · 4 comments · Fixed by #5063
Closed

useless-suppression false positive with ungrouped-imports #2366

mthuurne opened this issue Jul 30, 2018 · 4 comments · Fixed by #5063
Labels
Bug 🪲
Milestone
2.12.0

Comments

@mthuurne
Copy link
Contributor

Steps to reproduce

Run pylint on the following code:

# pylint: enable=useless-suppression
from pylint import run_pylint
import astroid
from pylint import run_pyreverse # pylint: disable=ungrouped-imports

Current behavior

This message is issued:

4:0: I0021: Useless suppression of 'ungrouped-imports' (useless-suppression)

However, if the comment is removed from line 4, this message is issued:

4:0: C0412: Imports from package pylint are not grouped (ungrouped-imports)

Expected behavior

Since there is an actual message being suppressed, I would expect to get no useless-suppression message.

pylint --version output

pylint 2.0.1
astroid 2.0.1
Python 3.4.6 (default, Mar 22 2017, 12:26:13) [GCC]
@PCManticore
Copy link
Contributor

Thanks, I can reproduce this bug. While useless-supression is inoffensive in the sense that it is not consider towards any score or not considered as an error per se, this bug shows that there is some inconsistency somewhere.

@brycepg
Copy link
Contributor

brycepg commented Aug 4, 2018 •
edited
Loading

So this particular check as a quit early conditional before adding the message:

if not self.linter.is_message_enabled('ungrouped-imports', import_node.fromlineno):
      continue

Removing the line fixes the issue. Do we want to remove these guards to allow add_message to go through or disable useless-suppression for that check if locally disabled?

@bersbersbers
Copy link

bersbersbers commented Apr 29, 2020 •
edited
Loading

This is still an issue with

pylint==2.5.0
astroid==2.4.0

on Python 3.8.2.

I believe my example code is functionally identical to the OP's:

"""False-positive useless-suppression."""
# pylint: enable=useless-suppression
from numpy import array
from pandas import DataFrame

if True and True:
    from numpy import zeros  # pylint:disable=ungrouped-imports

print(array, DataFrame, zeros)

This code gives

bug.py:7:0: I0021: Useless suppression of 'ungrouped-imports' (useless-suppression)

If you remove # pylint:disable=ungrouped-imports, you get

bug.py:7:4: C0412: Imports from package numpy are not grouped (ungrouped-imports)

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Dec 21, 2020
@The-Compiler
utils: Reorder imports
05e6ef8 
Fixes pylint properly and avoids pylint-dev/pylint#2366
@bersbersbers bersbersbers mentioned this issue Apr 15, 2021
Closed
@bersbersbers
Copy link

Please know that the example in #2366 (comment) is functionally different because according to #4212 (comment), the ungrouped-imports is actually unexpected there, so the useless-suppression is expected. I filed #4360.

nishakm pushed a commit to nishakm/tern that referenced this issue Sep 1, 2021
Prospector 1.4.1 fixes
7c61a6d 
Major changes:
- Return None in mount_single_layer function when something goes
  wrong. Do a check for the target_dir string existing before passing
  it on for analysis.
- Return None for DriverManager try-except calls when there is a
  NoMatches exception. This modifies all instantiations of Stevedore's
  DriverManager class.
- Iterate over the dictionary using .items() rather than just the plain
  dictionary.
- Use "with" to open files and subprocess pipes. This may result in
  certain errors not being raised, but it is hard to know at this
  point.

Minor changes:
- Used the "maxsplit" argument in __main__.py as we only need the
  first item.
- Used [] rather than list() to initialize lists.
- Specify encoding as 'utf-8' when opening files.
- Update re-raises to explicitly state which exception they are
  re-raising from using "from".
- Use "enumerate" for looping through list's index while throwing
  out the unused value.
- Suppressed the "useless-suppression" message along with
  "too-many-branches" suppression due to pylint bug:
  pylint-dev/pylint#2366
- Wraped some lines that were too long, except in the HTML format
  file due to the need to keep the literal HTML formatting.
- Updated the year on some modified files.

Signed-off-by: Nisha K <nishak@vmware.com>
@nishakm nishakm mentioned this issue Sep 1, 2021
Merged
rnjudge pushed a commit to tern-tools/tern that referenced this issue Sep 2, 2021
@rnjudge
Prospector 1.4.1 fixes
4c4b2a8 
Major changes:
- Return None in mount_single_layer function when something goes
  wrong. Do a check for the target_dir string existing before passing
  it on for analysis.
- Return None for DriverManager try-except calls when there is a
  NoMatches exception. This modifies all instantiations of Stevedore's
  DriverManager class.
- Iterate over the dictionary using .items() rather than just the plain
  dictionary.
- Use "with" to open files and subprocess pipes. This may result in
  certain errors not being raised, but it is hard to know at this
  point.

Minor changes:
- Used the "maxsplit" argument in __main__.py as we only need the
  first item.
- Used [] rather than list() to initialize lists.
- Specify encoding as 'utf-8' when opening files.
- Update re-raises to explicitly state which exception they are
  re-raising from using "from".
- Use "enumerate" for looping through list's index while throwing
  out the unused value.
- Suppressed the "useless-suppression" message along with
  "too-many-branches" suppression due to pylint bug:
  pylint-dev/pylint#2366
- Wraped some lines that were too long, except in the HTML format
  file due to the need to keep the literal HTML formatting.
- Updated the year on some modified files.

Signed-off-by: Nisha K <nishak@vmware.com>
@Pierre-Sassoulas Pierre-Sassoulas mentioned this issue Sep 22, 2021
Closed
DanielNoord added a commit to DanielNoord/pylint that referenced this issue Sep 22, 2021
@DanielNoord
Fix useless-suppression for wrong-import-order
f95a42d 
This also adds a new method to ``MessagesHandlerMixIn`` which
adds messages to the list of the ignored messages without doing anything
else. This will be useful for some other ``useless-suppression``
false positives.
This closes pylint-dev#2366
@DanielNoord DanielNoord mentioned this issue Sep 22, 2021
Merged
4 tasks
DanielNoord added a commit that referenced this issue Sep 23, 2021
@DanielNoord
Fix useless-suppression for wrong-import-order (#5063)
09a0b50 
This also adds a new method to ``MessagesHandlerMixIn`` which
adds messages to the list of the ignored messages without doing anything
else. This can be used to avoid ``useless-suppression`` false positives.
This closes #2366
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.11.2 milestone Sep 23, 2021
@jacobtylerwalls jacobtylerwalls mentioned this issue Mar 12, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug 🪲
Projects
None yet
Milestone
2.12.0
Development

Successfully merging a pull request may close this issue.

Fix useless-suppression for wrong-import-order DanielNoord/pylint
5 participants
@mthuurne @brycepg @PCManticore @Pierre-Sassoulas @bersbersbers