tonybaloney / perflint

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

IndexError: pop from empty list #3

Closed aroberge closed 2 years ago

aroberge commented 2 years ago

After seeing a tweet to Will McGugan, I thought I should try it on my project and got a traceback

> pylint friendly_traceback/ --load-plugins=perflint > pylint_result.txt
Traceback (most recent call last):
  File "C:\Users\Andre\AppData\Local\Programs\Python\Python310\lib\site-packages\pylint\utils\ast_walker.py", line 77, in walk
    callback(astroid)
  File "C:\Users\Andre\AppData\Local\Programs\Python\Python310\lib\site-packages\perflint\for_loop_checker.py", line 170, in leave_for
    self._leave_loop(node)
  File "C:\Users\Andre\AppData\Local\Programs\Python\Python310\lib\site-packages\perflint\for_loop_checker.py", line 179, in _leave_loop
    assigned_names = self._loop_assignments.pop()
IndexError: pop from empty list

perflint did give me some useful pointers about loop invariants that I plan to investigate. Thank you for making this plugin.

tonybaloney commented 2 years ago

Thanks for the report. I can see how to quickly fix this, but I'd like to add a regression test.

Are you able to share the code (most likely a for loop) that kicked off this error? Running with -v might give a hint

aroberge commented 2 years ago

Sorry for the previously rather terse bug report; as you have no doubt noted, I redirected the standard output to a file, as pylint is flagging too many things in my library. (I usually only use flake8.) I've redone the test and looked more carefully at the output and saw that a crash file had been generated.

I have been able to comment out most of the code in the offending file to a single class which does include a for loop and still generate a pylint error. I've appended the crash file to this report.

pylint-crash-2022-03-12-08.txt

The original offending code can be found at https://github.com/friendly-traceback/friendly-traceback/blob/main/friendly_traceback/utils.py

tonybaloney commented 2 years ago

Ah I see. My money is on line 58 then! I've never seen a for loop assigning to a property of self (although that's perfectly valid)

The PR would fix it. I'll add a corresponding test

tonybaloney commented 2 years ago
def loop_invariance_in_self_assignment():
    class Foo:
        n = 1

        def loop(self):
            i = 4
            for self.n in range(4):
                print(self.n)
                len(i)  # [loop-invariance]

    def test(): #@
        f = Foo()
        f.loop()
    test()

This reproduces the bug.

tonybaloney commented 2 years ago

Update to 0.1.1