Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix require-unicode-regexp issues #9212

Merged
merged 2 commits into from
Aug 14, 2020

Conversation

whymarrh
Copy link
Contributor

Refs #8982

See require-unicode-regexp for more information.

This change enables require-unicode-regexp and fixes the issues raised by the rule.

But what is the u flag?

Unicode-aware regular expressions in ES2015 covers the most of it, I'd recommend skimming the sections.

My opinionated summary:

ECMAScript 2015 introduces [...]

  1. [...]
  2. u enables various Unicode-related features.

Impacts of the u flag:

  • Things like \u{1234} are now Unicode code point escapes instead of matching 1234 consecutive u symbols
  • Things like \a (where a is not an escape character) now error because \a is not a reserved escape sequence
  • The . now matches unicode symbols
  • Quantifiers e.g. *, +, ?, and {2}, {2,}, {2,4} now work on unicode symbols
  • Character classes and negated character classes work on unicode symbols
  • Combining the u and i flags is weird because it does case folding

Recommendations for developers

  • Use the u flag for every regular expression you write from now on.
  • …but don’t blindly add the u flag to existing regular expressions, as it might change their meaning in subtle ways.
  • Avoid combining the u and i flags. It’s better to be explicit and include all letter cases in your regular expression itself than to be surprised by implicit case folding.

See [`require-unicode-regexp`](https://eslint.org/docs/rules/require-unicode-regexp) for more information.

This change enables `require-unicode-regexp` and fixes the issues raised by the rule.
@whymarrh whymarrh marked this pull request as ready for review August 13, 2020 18:38
@whymarrh whymarrh requested review from kumavis and a team as code owners August 13, 2020 18:38
@whymarrh whymarrh requested a review from tmashuang August 13, 2020 18:38
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I can't see any cases of invalid escape sequences, and I can't think of any reason we'd want to avoid matching unicode characters in any of these cases.

@whymarrh whymarrh merged commit 5d42a9b into MetaMask:develop Aug 14, 2020
@whymarrh whymarrh deleted the enable-require-unicode-regexp branch August 14, 2020 11:48
Gudahtt added a commit that referenced this pull request Aug 14, 2020
* origin/develop: (107 commits)
  Clear Account Details in AppState (#9238)
  Permit all-caps addresses (#9227)
  Send web3 usage metrics once per origin/property (#9237)
  Fix import/no-extraneous-dependencies issues (#9232)
  Remove unused buyEth fn from bg (#9236)
  Fix max-statements-per-line issues (#9218)
  Consolidate ESLint config files (#9231)
  Delete page-container.component.test.js (#9229)
  Tidy up getAccountLink (#9223)
  Tidy ConnectHardwareForm#checkIfUnlocked (#9224)
  Fix require-unicode-regexp issues (#9212)
  Fix no-negated-condition issues (#9222)
  Fix no-empty-function issues (#9216)
  Fix import/extensions issues (#9217)
  Dedupe glob-parent versions (#9220)
  Fix no-template-curly-in-string issues (#9221)
  Fix no-process-exit issues (#9219)
  Fix prefer-rest-params issues (#9215)
  Fix no-prototype-builtins issues (#9213)
  Fix no-nested-ternary issues (#9214)
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants