Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of missing orelse block #175

Open
machow opened this issue Dec 22, 2016 · 2 comments
Open

Fix handling of missing orelse block #175

machow opened this issue Dec 22, 2016 · 2 comments
Labels

Comments

@machow
Copy link
Contributor

machow commented Dec 22, 2016

in most AST nodes, when a missing attribute,

  1. usually only corresponds to a single node, it is None
  2. usually corresponds to one or more nodes, it is an empty list

However, the orelse node lives a double life. Sometimes it is a list of expressions, as in...

if True: pass
else:
    "a"
    "b"

Sometimes it is a list containing only another If node, as in...

if True: 1
elif False: 2
else: 3

Because of this, it's currently impossible to test if someone omitted the else block in the statement above. check_part looks for cases where it is None, but will receive an empty list. Should be a quick fix.

@machow machow added the bug label Dec 22, 2016
@machow machow self-assigned this Dec 22, 2016
@machow
Copy link
Contributor Author

machow commented Jan 2, 2017

I tested out a fix here: https://github.com/datacamp/pythonwhat/tree/fix-check-orelse

The only problem is that internally, functions like test_while_loop use check_orelse, since they may also have else blocks. Since the else block is missing in most examples, test_while_loop will inappropriately fail.

Should be able to fix by modifying has_part to fail only when no orelse part in both student and submission.

machow added a commit that referenced this issue Mar 2, 2017
@machow
Copy link
Contributor Author

machow commented Mar 2, 2017

Not that the patch in the fix-check-orelse branch fails for functions like test_for_loop, since they allow for pieces to be empty. (e.g. get the ifs in a list comp, and don't fail if there aren't any).

@machow machow self-assigned this May 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant