Skip to content

fix: borrow value in optional string pattern check - #21

Merged
jrandolf merged 1 commit into
mathematic-inc:mainfrom
connyay:cjh-optional-string-match
Jul 14, 2026
Merged

fix: borrow value in optional string pattern check#21
jrandolf merged 1 commit into
mathematic-inc:mainfrom
connyay:cjh-optional-string-match

Conversation

@connyay

@connyay connyay commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

An explicit-presence string field (optional string / editions field_presence = EXPLICIT) is lowered to an owned let v: String binding before running the shared string-rule emitter. The string.pattern check passed v to Regex::is_match, which takes &str. An owned String does not coerce in argument position, so the generated validator failed to compile (expected &str, found String).

The shared emit_string_checks_on is also used for oneof members, where the match arm binds v as &String (which coerces), so the bug only hit the optional case. Conformance never caught it because its string.pattern cases are all plain non-optional fields.

Pass v.as_str() so the check compiles whether v is an owned String or a &String. Add a render-based regression test covering both paths.

An explicit-presence string field (`optional string` / editions
`field_presence = EXPLICIT`) is lowered to an owned `let v: String`
binding before running the shared string-rule emitter. The
`string.pattern` check passed `v` to `Regex::is_match`, which takes
`&str`. An owned `String` does not coerce in argument position, so the
generated validator failed to compile (`expected &str, found String`).

The shared `emit_string_checks_on` is also used for oneof members, where
the match arm binds `v` as `&String` (which coerces), so the bug only hit
the `optional` case. Conformance never caught it because its
`string.pattern` cases are all plain non-optional fields.

Pass `v.as_str()` so the check compiles whether `v` is an owned `String`
or a `&String`. Add a render-based regression test covering both paths.
@jrandolf
jrandolf merged commit 5e58b3d into mathematic-inc:main Jul 14, 2026
6 of 7 checks passed
@jrandolf jrandolf mentioned this pull request Jul 14, 2026
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.

2 participants