pylint
pylint copied to clipboard
pylint-dev
Disable after module docstring has the scope of the whole file.
Usually a "# pylint: disable=foo" comment appended to a line disables the warning only for that line. However, if the disable appears on the first line after the docstring then it appears to affect the entire file. Is this intentional or a bug?
For example:
"""really long docstring...""" # pylint: disable=line-too-long
a = " some other long string..."
But if the disable happens later then it only affects that one line:
"""My docstring."""
"""really long docstring...""" # pylint: disable=line-too-long
a = " some other long string..."
The line that assigns "a" will trigger an error message.
I don't think this is intentional, it looks like a bug to me. Would you like to investigate and see what's causing this issue? It'll probably wait otherwise for a bit.
I think I understand where the problem is coming from, though I'm not sure how to solve it. All of the real logic is in utils.FileState._collect_block_lines. This does a post-order walk of the AST, expanding enable/disable states to cover additional lines. The first node that overlaps the disable line will process that line and expand it to cover from its own line until the end of the node. So far, so good.
The big problem is that docstrings are stripped from the astroid AST. They appear in the initial python ast as Str nodes, but get stripped out by rebuilder (and the string becomes the doc field of the module node).
Thus if your module looks like:
"""Docstring.""" # pylint: disable=line-too-long A = 123
Then there is a Module node for lines 0-2 (yeah, line 0), and a few other nodes on line 2. The only node that overlaps with line 1 is the top Module node, thus the disable gets applied to every line in the file.
A few different approaches come to mind:
-
Leave the Str nodes in the astroid tree. I think this would easily fix the disable problem, but it likely has huge consequences elsewhere. This isn't something I would even attempt unless you believe it is likely to only cause a few minor issues.
-
Tweak _collect_block_lines to pretend there is a Str node even though there isn't. One caveat is that we probably have lost the actual line number of the docstring. Perhaps save this when building the Module node initially.
-
Sort of a hybrid between #1 and #2. Create an astroid node for the docstring, but tuck it away in a field of the Module rather than as a child. This won't break anything that walks the ast, but _collect_block_lines could have a special case to recurse on the module docstring node.
I just don't have a good feel for how ugly any of these get, or if such a fix is worthwhile. Any thoughts?
From these solutions, 2 and 3 sounds like the best solutions that won't introduce too much backward incompatibilities. 1 can definitely introduce bugs, since it's not expected for that node to occur (and probably some processing are blindly done over the first element of the body). If you tackle this, just remember for the 3) that in some cases the string value needs to be returned instead of the new Docstring node (as it is the case for astroid.interpreter.objectmodel.Module.py__doc__). Unfortunately, any picked solution should go in the 2.0 branch, so we won't have a fix in the minor releases.
