hauntsaninja / pyp

Easily run Python at the shell! Magical, but never mysterious.
MIT License
1.41k stars 39 forks source link

Bugfix: When a variable is defined by a for loop, count that as a definition #3

Closed nickodell closed 4 years ago

nickodell commented 4 years ago

Bug

I ran this command:

cat pollution.csv | python3 pyp.py 'for row in lines: print(row)'

I got the following error:

Traceback (most recent call last):
  File "pyp.py", line 387, in <module>
    main()
  File "pyp.py", line 383, in main
    run_pyp(parse_options(sys.argv[1:]))
  File "pyp.py", line 332, in run_pyp
    exec(compile(tree, filename="<ast>", mode="exec"), {})
  File "<ast>", line 1, in <module>
ModuleNotFoundError: No module named 'row'

Using --explain gives this:

import row  # Bug here
import sys
lines = [x.rstrip('\n') for x in sys.stdin]
for row in lines:
    print(row)

Expected behavior

pyp prints each line in a loop, as if I'd written pyp 'x'

Fix

This function controls what order generic AST nodes are visited:

def order(f_v: Tuple[str, Any]) -> int:

Change the function so that the body of a loop is visited after the variables created by a for loop are defined.

I wrote a test for this issue as well.

PS: This is a great project. I come from an Awk background, and this is a really nice tool.

nickodell commented 4 years ago

Similar issue with the command ls | python3 pyp.py '[a for a in lines if a != "README.md"]'

It evaluates the if a != "README.md" part before assigning a, so it mistakenly imports a.

hauntsaninja commented 4 years ago

Thanks for finding these issues and for the ready-to-merge PR! I really appreciate it.

I think this change can cause issues with try except statements, e.g., python3.8 pyp.py --explain 'try: y = 1' 'except Exception: print(y)', but it's on me for not having a test for that / loops being totally broken is clearly a more pressing problem.

hauntsaninja commented 4 years ago

Try-except is also now fixed on master and I've added more tests. Just cut a new release, so your fix is in if you update. Thanks again!

nickodell commented 4 years ago

Thanks!