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

Use a better python problem matcher #420

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eric-wieser
Copy link

@eric-wieser eric-wieser commented Jun 7, 2022

Description:

The old matcher only worked if the error was raised with raise Exception('single quotes').

This represents a miniscule fraction of errors; for instance, l[37] on a short list l can raise IndexError, and any call to a builtin C function is not going to trace back to a raise call.

Instead, this just matches the first line without fail that comes after the context line.

Note that this is still not foolproof; in Python 3.10, SyntaxErrors are produced as

  File "<stdin>", line 1
    foo(x, z for z in range(10), t, w)
           ^^^^^^^^^^^^^^^^^^^^
SyntaxError: Generator expression must be parenthesized

This matcher will incorrectly pick up ^^^^^^^^^^^^^^^^^^^^ as the error message, but the previous behavior was to not pick up any error message at all.

As far as I can tell, this is impossible to handle correctly; the grammar of problem matchers is far too limiting.

Some other changes:

  • Remove nonsensical double-escaping of ". " is not a regex character, it needs only to be escaped once for JSON
  • Require the traceback to appear in the first column, as this is how python usually handles it. There is no way to track indentation between lines, but we are forced to make a choice between catching errors not caused by raise (common) and catching errors which are printed by a custom handler (uncommon).

Related issue:
This follows on from

Check list:

The old matcher only worked if the error was raised with `raise Exception('single quotes')`.

This represents a miniscule fraction of errors; for instance, `l[37]` on a short list `l` can raise `IndexError`, and any call to a builtin C function is not going to trace back to a `raise` call.

Instead, this just matches the first line without fail that comes after the context line.

Note that this is still not foolproof; in Python 3.10, `SyntaxError`s are produced as 
```
  File "<stdin>", line 1
    foo(x, z for z in range(10), t, w)
           ^^^^^^^^^^^^^^^^^^^^
SyntaxError: Generator expression must be parenthesized
```

This matcher will incorrectly pick up `           ^^^^^^^^^^^^^^^^^^^^` as the error message, but the previous behavior was to not pick up any error message at all.

As far as I can tell, this is impossible to handle correctly; the grammar of problem matchers is far too limiting.
@eric-wieser eric-wieser requested a review from a team June 7, 2022 17:25
@brcrista brcrista requested review from a team and removed request for a team January 4, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant