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

Match in Python trips up Python parser in tests #386

Open
JSAbrahams opened this issue Dec 23, 2022 · 1 comment
Open

Match in Python trips up Python parser in tests #386

JSAbrahams opened this issue Dec 23, 2022 · 1 comment
Assignees
Labels
test: general General tests

Comments

@JSAbrahams
Copy link
Owner

JSAbrahams commented Dec 23, 2022

Description of Bug

It appears that the python parser trips up as soon as it encounters a match in Python.
When testing, everything before that is parsed.
We can test this by modifying this in one of the test resources and verify that the associate test fails (assuming we don't modify the associate check resource).
However, if we perform modifications after the match, it fails.

How to Reproduce

Take Mamba file resource/valid/error/nested_exception.mamba:

class MyException1(msg: String): Exception(msg)
class MyException2(msg: String): Exception(msg)

def f(x: Int) -> Int raise [MyException1, MyException2] =>
    match x
        0 => 20
        1 => raise MyException1()
        2 => raise MyException2()

def g() -> Int raise [MyException2] =>
    f(2) handle
        err: MyException1 => print("a")

g()
  • Modify anything before the match, e.g. change msg -> msg1, and observe that the test fails by leaving the associate _check.py untouched.
  • Now, modify something in match, e.g. change MyException1 -> MyException2.
    Observe how the test still passes despite the output not being equivalent to the associate check resource.

For reference, this is the associate nested_exception_check,py resource:

class MyException1(Exception):
    def __init__(self, msg: str):
        Exception.__init__(self, msg)

class MyException2(Exception):
    def __init__(self, msg: str):
        Exception.__init__(self, msg)

def f(x: int) -> int:
    match x:
        case 0:
            return 20
        case 1:
            raise MyException1()
        case 2:
            raise MyException2()

def g() -> int:
    try:
        f(2)
    except Exception as err:
        print("a")

g()

Expected behavior

The test should fail as soon as the output deviates from the resource.

Additional context

This is a bug in python parser v 0.1.0.
We could update to version 0.2.0, but this only goes up to Python 3.7 it appears.
Therefore, we may need to search for another crate which parses Python files for us.
Furthermore, there appear breaking changes when going from 0.1.0 to 0.2.0, so even if 0.2.0 does in fact resolve the issue, we do need to perform non-trivial changes.

@JSAbrahams JSAbrahams added the test: general General tests label Dec 23, 2022
@JSAbrahams JSAbrahams added this to the v0.3.6 | Dictionaries milestone Dec 23, 2022
@JSAbrahams JSAbrahams self-assigned this Dec 23, 2022
@JSAbrahams JSAbrahams changed the title Match in Python trips up python parser Match in Python trips up python parser in tests Dec 23, 2022
JSAbrahams added a commit that referenced this issue Dec 23, 2022
The fact that the test still passed means that
something is going wrong here under the hood.
An issue has been made: #386
@JSAbrahams JSAbrahams changed the title Match in Python trips up python parser in tests Match in Python trips up Python parser in tests Dec 23, 2022
@JSAbrahams
Copy link
Owner Author

JSAbrahams commented Dec 26, 2022

Perhaps we should fork that crate and see if we can't implement this ourselves

JSAbrahams added a commit that referenced this issue Dec 27, 2022
* Update logic for raises in handle

We now check in the generate stage whether a
raise is covered correctly.
Looks like we might not even need it in the
unification stage.

* Add expr type to covered expressions in handle

- Fix generate stage to insert return (and assign)
  in proper locations within try except.

* Check whether parents of exception covered

Which is also valid.

Remove `Raises` `Expect`, which gets rid of a lot
of unnecessary logic in the unification stage.

* Cleanup

* Restore logic for same value of two types

* Move shared logic match, handle to constrain_cases

* Dynamically add raises to Environment

Useful if exceptions in Mamba are not dealt with
locally, but say in the function signature, which
is allowed.

* Add return in nested_exception_check.py

The fact that the test still passed means that
something is going wrong here under the hood.
An issue has been made: #386
JSAbrahams added a commit that referenced this issue Dec 28, 2022
* Update logic for raises in handle

We now check in the generate stage whether a
raise is covered correctly.
Looks like we might not even need it in the
unification stage.

* Add expr type to covered expressions in handle

- Fix generate stage to insert return (and assign)
  in proper locations within try except.

* Check whether parents of exception covered

Which is also valid.

Remove `Raises` `Expect`, which gets rid of a lot
of unnecessary logic in the unification stage.

* Cleanup

* Restore logic for same value of two types

* Move shared logic match, handle to constrain_cases

* Dynamically add raises to Environment

Useful if exceptions in Mamba are not dealt with
locally, but say in the function signature, which
is allowed.

* Add return in nested_exception_check.py

The fact that the test still passed means that
something is going wrong here under the hood.
An issue has been made: #386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test: general General tests
Projects
None yet
Development

No branches or pull requests

1 participant