pylint Useful 'line-too-long' suppression considered useless

Steps to reproduce

Run pylint -e useless-suppression --max-line-length=120 toolong.py where toolong.py's content is:

# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789  # pylint: disable=line-too-long
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789

Current output

************* Module toolong
toolong.py:1:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:2:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:4:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:5:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:3:0: I0021: Useless suppression of 'line-too-long' (useless-suppression)

Expected behavior

************* Module toolong
toolong.py:1:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:2:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:4:0: C0301: Line too long (146/120) (line-too-long)
toolong.py:5:0: C0301: Line too long (146/120) (line-too-long)

pylint --version output

pylint 2.4.4
astroid 2.3.3
Python 3.7.6 (default, Jan  2 2020, 09:57:44) 
[GCC 9.2.0]

Jan 27 '20 16:01 romainletendart

Thanks for the report!

Feb 04 '20 17:02 PCManticore

@PCManticore A TODO in one of our projects reminded me of this issue and it still seems to be around. Any ideas on how to fix it?

Jul 13 '20 08:07 romainletendart

@PCManticore Up ?

Oct 22 '20 15:10 romainletendart

Any news about it ?

Jun 01 '21 14:06 TiphaineLAURENT

Hello, no one worked on it yet, but contributions are welcome :)

Jun 01 '21 14:06 Pierre-Sassoulas

I think something might have changed in the checker. I now get:

❯ cat test.py
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789  # pylint: disable=line-too-long
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
# ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
❯ pylint -e useless-suppression --max-line-length=120 'test.py'
************* Module test
test.py:3:0: I0021: Useless suppression of 'line-too-long' (useless-suppression)

Do we still consider this a false positive?

Sep 27 '21 10:09 DanielNoord

Hi @DanielNoord, We should get as many line-too-long errors as there are lines without the check disabled i.e. 4 line-too-long errors. On your output we get none so there still is a problem here.

Sep 27 '21 11:09 romainletendart

This also affects triple-quoted strings, e.g.:

'''
This line is much too long. It's long enough to cause a warning. ======================
'''  # pylint: disable=line-too-long

This case was described in detail in #5749, which Pierre has very reasonably judged to be a duplicate of this issue.

Feb 02 '22 16:02 finite-state-machine

Output mentioned above

************* Module test
test.py:3:0: I0021: Useless suppression of 'line-too-long' (useless-suppression)

is the output if we run it with pylint -e useless-suppression --max-line-length=120 test.py but what we actually want is:

pylint test.py --enable=all --enable-all-extensions which will show everything

test.py:3:0: I0011: Locally disabling line-too-long (C0301) (locally-disabled)
test.py:1:0: C0301: Line too long (146/100) (line-too-long)
test.py:2:0: C0301: Line too long (146/100) (line-too-long)
test.py:4:0: C0301: Line too long (146/100) (line-too-long)
test.py:5:0: C0301: Line too long (146/100) (line-too-long)
test.py:3:0: I0021: Useless suppression of 'line-too-long' (useless-suppression)

the last line is the FP to target

Nov 30 '22 15:11 clavedeluna

I've narrowed down the issue. This only affects modules with no code, only with comments with # or with """. This can be tested by adding any other line that is code to the module, and you'll see the useless-suppression goes away.

The issue is triggered to do this line. When node is a module with no python code and only comments, node.get_children() has no children, so we do not enter into self._set_state_on_block_lines and a few internal containers are not updated.

Could a maintainer suggest a way to get around this? Either by somehow adding the code "child" to the node via astroid, or by suggesting a way around this. I tried stuff like node.doc_node or node["__doc__"] which I see are used in other doc-based checkers, but none worked for this case.

Nov 30 '22 20:11 clavedeluna

I haven't looked into this, but this might be unfixable. Children of "empty" modules can't really be made up and I'm not in favour of adding them arbitrarily to Module nodes just so we can fix this edge case.

It would be interesting to know whether this issue also occurs if any actual code is in the module, both before and after the comments.

Nov 30 '22 22:11 DanielNoord

It would be interesting to know whether this issue also occurs if any actual code is in the module, both before and after the comments.

The useless-suppression FP does not happen with code before, after, or anywhere in the module. This is decidedly only limited to modules with comments via hash or """ as in one example given in a comment above.

I spent quite a bit of time trying to see how this could reasonably be fixed and I've concluded as you have, it seems unfixable.

I think we could close this issue as won't fix and for documentation purposes, add the FP as a functional test, what do you think?

Dec 01 '22 14:12 clavedeluna

We can document this in the details.rst of line-too-long too.

Dec 01 '22 15:12 Pierre-Sassoulas

Thanks for fixing this @clavedeluna! I think you found a nice solution here!

Dec 03 '22 14:12 DanielNoord

Really satisfying to close this!

Dec 03 '22 15:12 clavedeluna

With apologies for being the bearer of bad news, I'm not sure this issue is (completely) resolved.

Consider:

# pylint: disable=missing-module-docstring,unused-argument
# pylint: disable=missing-function-docstring,unused-variable

def first_case() -> None:
    # next line generates 'line-too-long' error (as it should)
    def my_function(aaa: int, bbb: int, ccc: int, ddd: int) -> int:  # pragma: no cover
        return 0

def second_case() -> None:
    # next line generates 'useless-suppression' (which is wrong)
    def my_function(aaa: int, bbb: int, ccc: int, ddd: int) -> int:  # pragma: no cover  # pylint: disable=line-too-long
        return 0

def third_case() -> None:
    # this case works as expected
    # pylint: disable-next=line-too-long
    def my_function(aaa: int, bbb: int, ccc: int, ddd: int) -> int:  # pragma: no cover
        return 0

Analyzing the above with python3 -m pylint --rcfile=/dev/null --max-line-len=79 --enable=line-too-long,useless-suppression some_filename.py, we get:

************* Module some_fliename
some_filename.py:6:0: C0301: Line too long (87/79) (line-too-long)
some_filename.py:11:0: I0021: Useless suppression of 'line-too-long' (useless-suppression)

------------------------------------------------------------------
Your code has been rated at 8.89/10 (previous run: 2.22/10, +6.67)

As can be seen above, the second_case() function still triggers a useless-suppression warning, which shouldn't be issued: it's useful, as shown by first_case().

[Edited to add version information:]

pylint 2.15.7
astroid 2.12.13
Python 3.8.12 (default, Jan 17 2022, 13:29:00)
[Clang 10.0.1 (clang-1001.0.46.4)]

Dec 03 '22 18:12 finite-state-machine

@finite-state-machine Thanks for the test case. What it shows is actually slightly different than the test cases for this issue, which were for comments only. Could you open a separate issue?

Dec 04 '22 11:12 clavedeluna

Could this be a duplicate of either https://github.com/PyCQA/pylint/issues/4802 or https://github.com/PyCQA/pylint/issues/3367?

Dec 04 '22 11:12 DanielNoord

Maaaybe? Turns out if you run some different commands results vary: pylint test.py --enable=line-too-long,useless-suppression --max-line-length=20

test.py:4:0: C0301: Line too long (25/20) (line-too-long)
test.py:5:0: C0301: Line too long (62/20) (line-too-long)
test.py:6:0: C0301: Line too long (87/20) (line-too-long)
test.py:9:0: C0301: Line too long (26/20) (line-too-long)
test.py:10:0: C0301: Line too long (64/20) (line-too-long)
test.py:14:0: C0301: Line too long (25/20) (line-too-long)
test.py:15:0: C0301: Line too long (33/20) (line-too-long)

worth investigating for sure

Dec 04 '22 11:12 clavedeluna

pylint pylint copied to clipboard

Useful 'line-too-long' suppression considered useless

Steps to reproduce

Current output

Expected behavior

pylint --version output

pylint
pylint copied to clipboard