Useful 'line-too-long' suppression considered useless

[romainletendart]

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]

[PCManticore]

Thanks for the report!

[romainletendart]

@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?

[romainletendart]

@PCManticore Up ?

[TiphaineLAURENT]

Any news about it ?

[Pierre-Sassoulas]

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

[DanielNoord]

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?

[romainletendart]

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.

[finite-state-machine]

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.

[clavedeluna]

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

[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.

[DanielNoord]

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.

[clavedeluna]

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?

[Pierre-Sassoulas]

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

[DanielNoord]

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

[clavedeluna]

Really satisfying to close this!

[finite-state-machine]

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)]

[clavedeluna]

@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?

[DanielNoord]

Could this be a duplicate of either #4802 or #3367?

[clavedeluna]

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