(parser) Improper tokenization with variable names in languages that use Unicode / UTF-8

[EmNudge]

Taking over this issue to track the overall process on this. Original issue is below.

Prework:

• (chore) Clean up all regexs to be UTF-8 compliant/ready #2759 Update all our regexe to be UTF8 compliant (10.4)
• Add Grammar#unicodeRegex to allow per-language UTF8 regex (11.x)
• Performance testing - /u seems MUCH slower in my initial testing. (10.4)

Individual languages may now add support if they don't trigger performance regressions:

• JavaScript
• TypeScript (I presume?)
• All languages depending on ecmascript.js will need to be reviewed (that's where the Javascript-like IDENT_RE is now)
• Java
• C
• XML (XML) Unicode letters should be recognized in element and attribute names #3256
• C++
• R
• Ruby
• Python
• Go
• Rust*
• Kotlin
• C#
• Swift

Perhaps one day:

• Flip the parser to always using u by default (10.4) [needs resolution of performance slowdowns]
• Ensure this change results in no overall regressions (10.4)
• Consider MODES IDENT_RE and UNDERSCORE_IDENT_RE, although I think they should not change.

Before any of this is possible we'll need to start building regexs with u mode. After that it should simply be a matter of altering the appropriate IDENT_REs for the language in question. Actually upon second though we do have IDENT_RE but I'm not confident that it should change (vs this being a per language opt-in)... perhaps an IDENT_RE_UTF8?

A few markup tests should be added to 2 or 3 of the languages (or perhaps unit tests if we add a new mode) to make sure the behavior is as expected.

---

Original issue

Describe the issue
Most modern languages use the Unicode spec to decide what constitutes a proper variable name. Thus, hangul filler characters, even though they seem to be whitespace, are valid variable names.
The variable name thisﾠisﾠaﾠvariableﾠname is a valid variable, although hl.js tokenizes the first word or so as separate from the rest.
In highlighting systems that are aware of this, like Chrome's dev tools, it is parsed correctly. The first snippet uses hangul fillers while the second one uses whitespace (which is why it throws an error):

Which language seems to have the issue?
Every language that follows the Unicode spec for variable names theoretically. I've only tested the ones mentioned in the title, but it might make more sense to just tokenize every language as if it were one of those that followed the unicode spec since this should be the default.

Are you using highlight or highlightAuto?
N/A

Sample Code to Reproduce
N/A

Expected behavior
hl.js should be using ID_Start and ID_Continue for the languages that use it, or at least use it as a default.
The regex to match this is rather large, but it can be shortened significantly in regex engines supporting the u flag (modern JavaScript)

const matchIDStart = char => /\p{ID_Start}/u.test(char);
const matchIDContinue = char => /\p{ID_Continue}/u.test(char);

\p{ID_Start} matches any valid starting character in a variabe name
\p{ID_Continue} matches any valid non-starting character in a variable name (such as numbers and some variation selectors)

[joshgoebel]

hl.js should be using ID_Start and ID_Continue for the languages that use it, or at least use it as a default.

How might we know which languages those are? :-) We have almost 200 languages... Right now our keyword default is \w and we don't really have an "identifier" default... each language kind of provides their own identifier regex... (and many provide their own keyword regex also). I'm thinking this might be something we have to do one language at a time as it comes up... flipping just that language into "utf8 mode" and then making sure it's regex are all still valid, etc...

This might be quite an effort for the whole project. Just a quick attempt to build all our regex with /u and then run just our markup test suite:

  108 passing (716ms)
  173 failing

Evidently the /u flag changes how regex strings are parsed and it's a LOT more strict about a lot of things that work just fine without it. One example:

Valid before:

/[$][\w-:]+/

Now with the u flag it wants the - to be escaped also... are you aware of an "upgrading" guide that covers most of the common pitfalls here (or any pointers)? I've seen just two cases already but I'm sure there are more... Seems a lot of issues about what should be escaped and what should not { always should now (even when sourced from a string)... < and > should NOT despite working just fine escaped prior.

[EmNudge]

How might we know which languages those are? :-) We have almost 200 languages... Right now our keyword default is \w and we don't really have an "identifier" default... each language kind of provides their own identifier regex... (and many provide their own keyword regex also). I'm thinking this might be something we have to do one language at a time as it comes up... flipping just that language into "utf8 mode" and then making sure it's regex are all still valid, etc...

To be honest, I'm not entirely sure. I'd have to do more research on the topic, but most modern imperative/OOP languages use ID_Start and ID_Continue these days as those categories seem to exist explicitly for that purpose.
Some languages, such as Rust, use XID_Start and XID_Continue which are slightly more strict subsets. Rust also has a linter option which turns it off mostly, so that's a weird case. XID vs ID excludes very few characters. You can play around with seeing how they differentiate with https://unicode-lookup.netlify.app by the category search.

To tell you which ones to target specifically, I'd have to check every language that hl.js supports, but I can tell you languages where I've confirmed it to the best of my knowledge (through use and/or the language spec):
JavaScript, Java, C/C++, Ruby, Python, Go, Rust*, Kotlin, C#, and Swift

I haven't used many more languages than that, so that's all I can back up. In my experience, ID_Start is the default, not the anomaly. I'd assume it's different for languages like SQL which likely have stricter rules for identifiers.

Now with the u flag it wants the - to be escaped also... are you aware of an "upgrading" guide that covers most of the common pitfalls here? I've seen just two cases already but I'm sure there are more...

I am not, unfortunately. I understand that fixing it, in general, is frustrating, so it's understandable if it goes unfixed. I just recognized an issue and thought it would be convenient to have a GitHub issue dedicated to it for future reference.

As an aside, if there were some convenient way to switch all regex to use ID_Start and ID_Continue, I'm not sure it would break much, even in languages which require only ASCII-idents, ID_Start still doesn't match for spaces or equals signs, so it's rare that it would match too much.

[joshgoebel]

As an aside, if there were some convenient way to switch all regex to use ID_Start and ID_Continue, I'm not sure it would break much, even in languages which require only ASCII-idents, ID_Start still doesn't match for spaces or equals signs, so it's rare that it would match too much.

Mostly it would all have to be done by hand... so if no one is having issues with a given language then I don't see any harm in languages keeping their older [a-z] style rules... and then when (or if) it DOES provide to be a problem and we have someone on the horn who knows what the rules are for language xyz, then we can deal with them on a one off basis.

I think the first step here would be to slowly clean up our regexs to be UTF8 compatible if possible (avoiding the need for per language flags)... and then at that point it could be decide which languages needed new regex rules to take advantage of what the u flag has to offer.

[joshgoebel]

@klmr Any thoughts on the C/C++ side of this?

[joshgoebel]

@EmNudge I turned this issue into the tracking issue for this whole initiative. Turns out I forgot we have IDENT_RE in our mode library, but still I don't think we should opt all languages into this by default - although I could perhaps be persuaded. I think the question is going to be "What is correct" vs "what does no harm".

[joshgoebel]

This has implications for auto-detect also for some languages... if a language does not declare itself UTF8 aware then it shouldn't potentially claim credit for UTF8 identifiers, it'd be better if those fall to a language that knows how to identify them... although as things stand now I wonder if this is stacked against us since it'd be likely a non-UTF8 language would see MULTIPLE identifiers here (would the utf8s count as word boundaries (\b)?), which would definitely have some bearing on the discussion.

var ident_ascii[utf8]ident_ascii[utf8]ascii = 39;

Grrrrr. :-)

> "testȠtestȠbooger".match(/\b[a-z]+\b/g)
[ 'test', 'test', 'booger' ]

Though thankfully most languages don't match (and give relevance to) random identifiers... though some do (looking at Swift, etc)...

[joshgoebel]

Dumping resources:

https://mathiasbynens.be/notes/es6-unicode-regex