Improve parser suppport for regexp group names

[rbuckton]

This PR improves the lexer token definition for GROUP_NAME to include valid unicode characters from the Unicode ID_Start and ID_Continue categories.

Fixes #144

[DmitrySoshnikov]

Cool stuff! Thanks for the PR.

Couple of questions and notes inline yet. Also, can you please run all the code through Prettier? You can install this pre-commit hook which should run it automatically.

[DmitrySoshnikov]

Why this needs to be new RegExp(uidRe, ...)? The uidRe is already a regexp, can just pass directly.

[rbuckton]

By adding a /g flag, the lastIndex is preserved between calls, even in the event of an unhandled exception. Rather than constantly mutating a value that is shared between multiple disparate calls, this ensures a fresh RegExp instance is created so that the lastIndex mutations remain local to the function.

[DmitrySoshnikov]

Gotcha, OK.

[DmitrySoshnikov]

So this depends on a Node version, and might be flaky/inconsistent? I'm wondering if it's possible just do a predictable subset, and update it periodically (even if manually).

Also, where/when will we run this script? Is it a part of the build script?

[rbuckton]

It was run manually. Its unlikely this will change often, usually only if you choose to target a new version of NodeJS that itself includes a new version of the Unicode standard.

[DmitrySoshnikov]

OK, we can add it to the build script as one of the steps at some point.

[mathiasbynens]

I'll copy a comment I made on the TypeScript implementation here: microsoft/TypeScript#32096 (comment)

The build script could be simplified and made to not rely on the current Node.js/V8 + Unicode version:

const ID_Start = require('unicode-12.1.0/Binary_Property/ID_Start/code-points.js');
const ID_Continue = require('unicode-12.1.0/Binary_Property/ID_Continue/code-points.js');

// ...then add the other needed characters, and write the two arrays to disk.

Links to how other tooling implements this:

• Esprima: https://github.com/jquery/esprima/blob/master/tools/generate-identifier-regex.js
• @babel/parser: https://github.com/babel/babel/blob/master/packages/babel-parser/scripts/generate-identifier-regex.js
• Acorn: https://github.com/acornjs/acorn/blob/master/bin/generate-identifier-regex.js
• JSHint: https://github.com/jshint/jshint/blob/master/scripts/generate-identifier-data.js
• XRegExp: https://github.com/slevithan/xregexp/blob/master/tools/scripts/utils.js

[rbuckton]

I ran each file in the PR manually through prettier and didn't notice any changes. The pre-commit hook didn't trigger for me, possibly because I'm running on Windows?

[rbuckton]

I just ran prettier --write src/**/*.js to see if I missed something, and it seems like the its the untouched parts of the repository actually have changes. Not sure if the fact I'm running on Windows might be the problem.

[DmitrySoshnikov]

@rbuckton, do you use the same Prettier version installed from the package.json? (we'll update it later to the latest one). If it's a different version, it might change untouched changes. Let's change only your files, and we can update the whole repo to reformat with a new version of Prettier later.

[rbuckton]

Yes, I ran prettier from node_modules/.bin/prettier.cmd. Also, it looks like the pre-commit hook did run, as the code was formatted from what I originally wrote.

[rbuckton]

C:\dev\regexp-tree [fixUnicodeGroupNames ≡]> npm ls prettier
regexp-tree@0.1.11 C:\dev\regexp-tree
`-- prettier@1.17.1

[rbuckton]

I started up bash for windows and ran the following:

rbuckton@rbuckton-book:/mnt/c/dev/regexp-tree$ jsfiles=$(git diff HEAD~2 --name-only --diff-filter=ACM "*.js" | tr '\n' ' ')
rbuckton@rbuckton-book:/mnt/c/dev/regexp-tree$ echo "$jsfiles" | xargs ./node_modules/.bin/prettier --write
scripts/generate-unicode-id-parts.js 172ms
src/compat-transpiler/transforms/compat-named-capturing-groups-transform.js 23ms
src/generator/__tests__/generator-basic-test.js 44ms
src/generator/index.js 46ms
src/parser/__tests__/parser-extended-test.js 55ms
src/parser/__tests__/parser-test262-test.js 53ms
rbuckton@rbuckton-book:/mnt/c/dev/regexp-tree$ git status
On branch fixUnicodeGroupNames
Your branch is up-to-date with 'origin/fixUnicodeGroupNames'.
nothing to commit, working directory clean

As far as prettier is concerned, there's nothing wrong. Is there a specific style issue I should address manually?

[DmitrySoshnikov]

OK, sounds good about Prettier. Later we'll need to setup Travis rule which would check against formatting with pre-defined locked version of the Prettier, so it's consistent.

Thanks, merging this one!

[DmitrySoshnikov]

BTW, we'll need to keep an eye on whether this slows down the lexer significantly. This rule should run only with u flag, right?

[rbuckton]

No, the U flag only determines whether or not a `\u{` CodePoint `}` escape is supported and how `\u` Hex4Digits is handled. A GroupName always supports the Unicode ID_Start and ID_Continue properties: https://tc39.es/ecma262/#prod-RegExpIdentifierStart

GroupName[U] ::
  `<` RegExpIdentifierName[?U] `>`

RegExpIdentifierName[U] ::
  RegExpIdentifierStart[?U]
  RegExpIdentifierName[?U] RegExpIdentifierPart[?U]

RegExpIdentifierStart[U] ::
  UnicodeIDStart
  `$`
  `_`
  `\` RegExpUnicodeEscapeSequence[?U]

RegExpIdentifierPart[U] ::
  UnicodeIDContinue
  `$`
  `\` RegExpUnicodeEscapeSequence[?U]
  <ZWNJ>
  <ZWJ>

RegExpUnicodeEscapeSequence[U] ::
  [+U] `u` LeadSurrogate `\u` TrailSurrogate
  [+U] `u` LeadSurrogate
  [+U] `u` TrailSurrogate
  [+U] `u` NonSurrogate
  [~U] `u` Hex4Digits
  [+U] `u{` CodePoint `}`

As you can see, UnicodeIDStart and UnicodeIDContinue are referenced unconditionally.

[mathiasbynens]

Is it a problem that this matches e.g. '\\u{2021}' (the even-odd problem)?

[rbuckton]

I'm not certain that would be the case. \\ isn't otherwise allowed in a group name, which has already been parsed prior to this regexp being used.