Skip to content

fix: correct upper-Z EXPR conformance fixture - #173

Open
Sanjays2402 wants to merge 1 commit into
OpenJobDescription:mainlinefrom
Sanjays2402:fix/expr-upper-z-fixture
Open

fix: correct upper-Z EXPR conformance fixture#173
Sanjays2402 wants to merge 1 commit into
OpenJobDescription:mainlinefrom
Sanjays2402:fix/expr-upper-z-fixture

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #171

The upper-Z EXPR invalid fixture was byte-for-byte identical to the lower-z fixture and incorrectly exercised \\z. It now uses \\Z, so the conformance corpus tests the upper-case escape named by the file.

This change was prepared with AI assistance; the regression assertion was run locally and fails without the fix.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Sanjays2402
Sanjays2402 requested a review from a team as a code owner August 15, 2026 03:06

@leongdl leongdl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do a git commit --amend -s to sign the commit.

@leongdl leongdl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this — the diagnosis is correct (both fixtures are the identical blob 92d1159 on mainline, so \Z currently has no coverage), but the replacement pattern has one backslash too many.

The file now contains r"llo\\Z" (two literal backslashes). Since EXPR r"..." is a raw string, backslashes pass through literally, so the resulting regex is llo\\Z — an escaped backslash followed by a literal Z. That is a valid pattern in the dialect, so this .invalid fixture no longer exercises the \Z end-of-string anchor that RFC 0006 excludes, and a conforming implementation should accept it — the fixture stops being invalid for the reason its name claims.

The sibling fixtures show the convention: the lower-z file uses r"llo\z" and the backreference file uses r"(a)\1" — always a single backslash. Issue #171's suggested fix is also the single-backslash form.

The line should be:

        - print(r'{{ re_search("hello", r"llo\Z") }}')

(I suspect the \\Z came from copying the issue body's markdown source, where \\Z renders as \Z.)

One more note: a regression assertion that only checks the two files are no longer byte-identical passes for any byte change, including this incorrect one — worth asserting the actual pattern content (\Z, single backslash) instead.

args:
- -c
- print(r'{{ re_search("hello", r"llo\z") }}')
- print(r'{{ re_search("hello", r"llo\\Z") }}')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be a single backslash: r"llo\Z". In an EXPR raw string, \\Z is two literal backslashes — as a regex that's an escaped backslash plus a literal Z, which is valid in the dialect, so the fixture no longer tests rejection of the \Z anchor. Compare the lower-z sibling (r"llo\z") and the backreference fixture (r"(a)\1"), which both use single backslashes.

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.

Conformance: re-backslash-lower-z and upper-Z EXPR fixtures are byte-identical, so backslash-Z is never tested

2 participants