tonybaloney / perflint

Python Linter for performance anti patterns
MIT License
659 stars 10 forks source link

loop-invariant-statement is duplicated and false positive #14

Open amaslenn opened 2 years ago

amaslenn commented 2 years ago

In the example below expression depends on the loop variable and cannot be moved out of the loop.

def foo():
    for f in ["v1", "v2"]:
        if f == "v1":
            obj.f1 = True
        elif f == "v2":
            obj.f2 = True
$ perflint test.py
************* Module test
test.py:4:12: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
test.py:6:12: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
test.py:4:12: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
test.py:6:12: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)

Also warnings are repeated twice for now obvious reason.

tonybaloney commented 2 years ago

It's probably suggesting the expression obj.f1 = True is the invariant one. Since obj is not resolved in this scope I'm not sure how it's going to handle that assertion.

I'll look at reproducing this in the functional tests

cdreimer-thewriter commented 2 years ago

I ran into this issue with a pytest function for a chess program I'm writing. Running pylint with the plugin against ztest.py (see zipped file below) will result in the loop-invariant-statement message being repeated six times in the console output. The loop goes through six piece objects (i.e., bishop, king, knight, pawn, rook, and queen). Line 14 is the last line with the assert statement.

from enums import PieceType
from pieces import factory_piece

def test_dunder_repr() -> None:
    """Test repr string without position.
    """
    pieces = [factory_piece(piece) for piece in PieceType]
    for piece in pieces:
        color, name = piece.color, piece.__class__.__name__
        assert repr(piece) == f"{name} (color={color}, position={None})"

Console output for pylint --load-plugins perflint ztest.py:

************* Module ztest
ztest.py:14:30: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
ztest.py:14:30: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
ztest.py:14:30: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
ztest.py:14:30: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
ztest.py:14:30: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)
ztest.py:14:30: W8201: Consider moving this expression outside of the loop. (loop-invariant-statement)

If I run pylint without the plugin, no issues are found. This message shouldn't be appearing at all (false positive). If it was a legit message, it should appear only once.

cdreimer-thewriter-w8201-files.zip