pylint
pylint copied to clipboard
Disables immediately after an else clause do not work properly.
def my_func(x):
if x:
pass
else:
# pylint: disable=protected-access
return x._foo
The disable has no effect here and a warning is generated. If any statement is insert between the else and the disable then it works properly. I suspect the line numbering for the else portion of the ast starts at the first child of the else clause rather than the else keyword itself.
Investigating a little further, it seems like the culprit is BlockRangeMixIn._elsed_block_range:
https://github.com/PyCQA/astroid/blob/master/astroid/mixins.py#L43
Specifically, the logic here is that when an else clause is present, anything before the first line of the first else statement is considered to have occurred before the else. Would it make more sense for this check to be based on the last line of the last statement of the body of the if? I suppose that has the problem that a disable at the end of the body would impact the else...
if ..
foo()
# pylint: disable=protected-access
else:
return x._foo # disabled because of the comment above
It would be great if we had the line number of the else keyword itself, though I suspect that's a limitation from the underlying python parser.
I'm also not sure about the repercussions for chained if/elif/else statements or the other uses of BlockRangeMixIn (Try, While, etc).
Indeed, this is definitely problematic, which is one of the reasons we will try to switch to other parsers in https://github.com/PyCQA/astroid/issues/329. Right now I think that trying to fine tune the numbering with the builtin ast module will still result in these kind of peculiar cases, where we don't have an exact state of source code.
The issue is still here.
See https://github.com/PyCQA/pylint/issues/3898#issuecomment-755332618 if you want to fix this:
I think the issue ultimately comes from astroid. From what I can tell, the disable pragmas are assigned to line ranges based on the associated node's block_range (in file_state.py). In this case the node is an If but the line number corresponds to the orelse portion; the orelse attribute of the If node is just a list of the expressions in the block. Since the pylint comment comes before those expressions, _elsed_block_range ends returning a range for the orelse block that doesn't actually include the expressions in it (since the only way to determine the line range of the orelse is to take the start of the first expression and the end of the last)
Not sure if this is helpful, but pylint:disable
also didn't work directly above an else
in my case (pylint 2.8.2):
class PolarPoint:
_private: int
x = PolarPoint()
y = PolarPoint()
# Fails
if x == y:
pass
else:
# pylint:disable=protected-access
print(x._private)
# Fails
if x == y:
pass
# pylint:disable=protected-access
else:
print(x._private)
# Success
# pylint:disable=protected-access
if x == y:
pass
else:
print(x._private)
I have opened an astroid
PR (https://github.com/pylint-dev/astroid/pull/1480) that would allows us to retrieve the position of the else
keyword. That should make this fixable.