Make pylint consider `disable params` when checking for line length

[DanielNoord]

Current problem

As discussed in #1682 pylint currently does not consider the length of the disable params when checking for line length.
For example, when max-length is set to 80 the following code would not raise a line-too-long warning.

Line with length of 80 columns  # pylint: disable=not-callable

According to the issue, this is due to:
https://github.com/PyCQA/pylint/blob/14da5ce258d3818037304a6e0f61aba1462e251d/pylint/checkers/format.py#L1004
Which seems fair.

However, this is not what users might intuitively expect from this warning, as a comment in that issue shows. Some users might want the max-length to also include comments.
Although #4797 fixes some of the issues in the original issue, it does not solve this particular issue. This issue serves to track it.

Desired solution

Make pylint consider comments when checking for max-length of lines.

Open question:
What should be default behaviour? Including comments or excluding comments? Current behaviour is excluding, but I would argue that it would be more intuitive to include them. Especially now that users can use disable-next to split comments and code over multiple lines.

[Pierre-Sassoulas]

Wow, I did not understand that the current behavior was to exclude the length of the comment. You're right to have insisted ! Clearly in my mind the default should be to take the real length of the line into account. We could maybe suggest to put the comment on the line above if it's the comment that is causing the problem (and to use disable-next if it's a pylint comment). Might be an issue with other linter like flake8 that do not have # noqa-next ?

[DanielNoord]

Hmm perhaps, but for those cases users will already have disabled line-too-long.
I think we can change the default behaviour and suggest to start using disable-next. Perhaps another maintainer wants to have a say about this?

[Pierre-Sassoulas]

The issue I was seeing is if pylint is suggesting to move a noqa from flake8 to the line above when it's in fact impossible. Maybe we don't need to handle that use case.

@cdce8p do you have an opinion ?

[cdce8p]

There seem to be valid arguments for both sides, with or without comments. Tbh I don't really like either that much.

Should we still have this check when flake does it probably way better than we do? Might be worth removing checks (or moving them to an extension) that are redundant (with flake) as we wouldn't need to support them anymore. If I remember correctly there have been some discussions here #3512 but I didn't follow that closely so don't know what the consensus was.

[Pierre-Sassoulas]

Might be worth removing checks (or moving them to an extension) that are redundant (with flake) as we wouldn't need to support them anymore.

We only document the one that are redundant if you're already using another linter, because some users want to just use pylint exclusively. Because of that removing checkers from the default is only decided on the checker's own merit regardless of another linter doing it or not. With the exception of very hard to maintain checks (like bad-indentation when black can do the formatting). line-too-long is not hard to maintain I think (more work than I thought though 😄 ).

For this issue I suggest we take the real line length into account for line-too-long and add . Use '# pylint: disable-next=x' on the previous line. to the current reported message if the line contain pylint: disable=x in it.