highlightjs / highlightjs/highlight.js

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

Open
#2,756 21 comments 0 reactions 0 assignees View on GitHub
enhancement help welcome language parser
Dominant language
JavaScript
Stars
25k
Forks
3.8k
Avg merge
11h 35m
Merged PRs (30d)
3

Description

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

Prework:

- [x] #2759 Update all our regexe to be UTF8 compliant (10.4)
- [x] Add `Grammar#unicodeRegex` to allow per-language UTF8 regex (11.x)
- [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
- [x] XML #3256
- [ ] C++
- [ ] R
- [ ] Ruby
- [x] 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):
![image](https://user-images.githubusercontent.com/24513691/96193673-a9f47700-0f16-11eb-89bc-f763692fde46.png)

**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)
```js
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)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.