Skip to content

Conversation

@the-13th-letter
Copy link
Contributor

@the-13th-letter the-13th-letter commented Jun 1, 2025

Fixes #4421, yet again, this time taking care of compound with-statements with parentheses, which Python 3.9/LL(1) would parse as simple with-statements with a tuple as the single context manager expression.

I don't know why this didn't trigger when I originally tested my fix in #4423, but it does now: on pretty much every hypothesis.given(...), I get an AttributeError in hypothesis.internal.conjecture.engine.ConjectureRunner.run.

This PR now splits up all compound with-statements into nested simple ones and adds a comment on why they shouldn't be combined again. As before, I only tackle the src directory, not the tests or otherwise. I double-checked every with-statement in src I could find for false positives/negatives and forgotten constellations.

@Zac-HD
Copy link
Member

Zac-HD commented Jun 1, 2025

I don't want to have to do this again, so we need a CI config which will test on the LL1 parser before I'll be happy to merge this.

@the-13th-letter
Copy link
Contributor Author

the-13th-letter commented Jun 2, 2025

I don't want to have to do this again, so we need a CI config which will test on the LL1 parser before I'll be happy to merge this.

I don't think there's much more to this other than setting up a normal Python 3.9 environment, and setting PYTHONOLDPARSER=1 during every run…

That said, I only tackled the with-statements in src/. So if you're potentially running your whole test suite under the LL(1) parser, then we'll want to update tests/ as well…

@Zac-HD
Copy link
Member

Zac-HD commented Jun 3, 2025

Yeah, I think this is just running our tests/cover/ tests with that set; if you want to make any edits we need there (and in tests/common/), I'll do the configuration.

As a follow-up to 422425f, the legacy
LL(1) parser also parses `with (expr1, expr2):` as simple
with-statements, with a single tuple as the context manager expression.
Such an expression can easily arise accidentally due to the use of an
auto-formatter. Guard against this by rewriting these statements as
nested with-statements and adding an appropriate comment about LL(1)
parsability.
@the-13th-letter the-13th-letter force-pushed the restore-py39-compatibility-LL1-parser branch from b83c52e to 6285f22 Compare June 4, 2025 19:51
@the-13th-letter
Copy link
Contributor Author

Rebased on master (hypothesis-python-6.135.0), squashed into one commit, and extended to deal with tests/ as well.

The failed check-coverage CI test appears to be unrelated: it concerns a timeout, not a syntax error.

@Zac-HD Zac-HD requested a review from Liam-DeVoe June 5, 2025 18:05
Copy link
Member

@Liam-DeVoe Liam-DeVoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! (@Zac-HD I'll give you a bit to check my tox configuration if you want to)

@Liam-DeVoe
Copy link
Member

[I was going to merge this now, but realized it requires a code-owner review]

@Zac-HD Zac-HD merged commit 1b8adb2 into HypothesisWorks:master Jun 8, 2025
61 checks passed
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.

Hypothesis ≥ 6.130.13 on Python 3.9 doesn't support the LL(1) parser anymore

3 participants