Allow custom character classes to begin with :
#269
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ICU and Oniguruma allow custom character classes to
begin with `:`, and only lex a POSIX character
property if they detect a closing `:]`. However
their behavior for this differs:
- ICU will consider *any* `:]` in the regex as a
closing delimiter, even e.g `[[:a]][:]`.
- Oniguruma will stop if it hits a `]`, so
`[[:a]][:]` is treated as a custom character class.
However it only scans ahead 20 chars max, and doesn't
stop for e.g a nested character class opening `[`.
Our detection behavior for this is as follows:
- When `[:` is encountered inside a custom character
class, scan ahead to the closing `:]`.
- While scanning, bail if we see any characters
that are obviously invalid property names. Currently
this includes `[`, `]`, `}`, as well as a second
occurrence of `=`.
- Otherwise, if we end on `:]`, consider that a
POSIX character property.
We could include more metacharacters to bail on,
e.g `{`, `(`, `)`, but for now I'm tending on the
side of lexing an invalid POSIX character property.
We can always relax this in the future (as we'd be
turning invalid code into valid code). Users can
always escape the initial `:` in `[:` if they want
a custom character class. In fact, we may want to
suggest this via a warning, as this behavior can
be pretty subtle.
|
@swift-ci please test |
hamishknight
commented
Apr 12, 2022
Comment on lines
+1119
to
+1125
| // TODO: Start erroring on these? | ||
| parseTest(#"\p{Lx}"#, prop(.other(key: nil, value: "Lx"))) | ||
| parseTest(#"\p{gcL}"#, prop(.other(key: nil, value: "gcL"))) | ||
| parseTest(#"\p{x=y}"#, prop(.other(key: "x", value: "y"))) | ||
| parseTest(#"\p{aaa(b)}"#, prop(.other(key: nil, value: "aaa(b)"))) | ||
| parseTest("[[:a():]]", charClass(posixProp_m(.other(key: nil, value: "a()")))) | ||
| parseTest(#"\p{aaa\p{b}}"#, concat(prop(.other(key: nil, value: #"aaa\p{b"#)), "}")) |
There was a problem hiding this comment.
@milseman Am I right in saying we'll want to start erroring on these unknown cases?
This was referenced Apr 19, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ICU and Oniguruma allow custom character classes to begin with
:, and only lex a POSIX character property if they detect a closing:]. However their behavior for this differs::]in the regex as a closing delimiter, even e.g[[:a]][:].], so[[:a]][:]is treated as a custom character class. However it only scans ahead 20 chars max, and doesn't stop for e.g a nested character class opening[.Our detection behavior for this is as follows:
[:is encountered inside a custom character class, scan ahead to the closing:].[,],}, as well as a second occurrence of=.:], consider that a POSIX character property.We could include more metacharacters to bail on, e.g
{,(,), but for now I'm tending on the side of lexing an invalid POSIX character property. We can always relax this in the future (as we'd be turning invalid code into valid code). Users can always escape the initial:in[:if they want a custom character class. In fact, we may want to suggest this via a warning, as this behavior can be pretty subtle.