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

evalv2: error from invalid regexp is lost when inside pattern constraint #3555

Open
rogpeppe opened this issue Nov 5, 2024 · 1 comment
Open
Labels
evaluator evalv3-win Issues resolved by switching from evalv2 to evalv3 NeedsFix

Comments

@rogpeppe
Copy link
Member

rogpeppe commented Nov 5, 2024

What version of CUE are you using (cue version)?

deed7d5c90d56f7468906f293494eb099f2c37a2

Does this issue reproduce with the latest stable release?

Yes (v0.10.0)

What did you do?

! exec cue vet schema.cue

-- schema.cue --
[=~"BAD)" & =~"x"]: string
y: 123

What did you expect to see?

A passing test. The regular expression BAD) has invalid syntax and should be rejected up front.
evalv3 behaves correctly here.

What did you see instead?

> ! exec cue vet schema.cue
FAIL: z.txtar:1: unexpected command success
@rogpeppe rogpeppe added NeedsFix evaluator evalv3-win Issues resolved by switching from evalv2 to evalv3 labels Nov 5, 2024
cueckoo pushed a commit that referenced this issue Nov 5, 2024
…nd `patternProperties`

There are a couple of issues with the jsonschema-generated code for
`additionalProperties` and `patternProperties`. It generates an extra
regexp to exclude fields that have been explicitly defined, but
it does not quote regexp metacharacters in that pattern (#3551).
When fixing that, I also realised that it does the wrong thing
for empty fields, because it does not omit the constraint when
there are no fields.

Changing the logic here exposed a bug (#3555) in the v2 evaluator where
the following expression would not give an error, even though
`"BAD)"` is not a valid regexp. The regressions in the jsonschema
tests are because of that.

Fixes #3551

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I0e21828f679eaf39afd81eaa38d8035ea3d93c71
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1203588
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
cueckoo pushed a commit that referenced this issue Nov 5, 2024
Issue #3555 shows that invalid regular expressions are ignored
in some circumstances. To avoid differences between evalv2 and evalv3
(and to enable better behaviour when `strictFeatures` is disabled),
check all regular expressions up front rather than relying on the
evaluator to detect them later.

Also detect invalid character classes specifically because that's
something which is technically a missing feature rather than
an invalid regexp.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I616c991bccd82ddcb9c2070404b352f686f58aeb
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1203589
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
@mvdan
Copy link
Member

mvdan commented Nov 5, 2024

Do we want to add an evalv3 regression test before closing this evalv3-win issue like the rest, or are the tests in jsonschema enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluator evalv3-win Issues resolved by switching from evalv2 to evalv3 NeedsFix
Projects
None yet
Development

No branches or pull requests

2 participants