Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(no-control-regex): Only warn on control char literals #1340

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lionel-rowe
Copy link

Fixes #1143.

Old and new behavior both diverge from ESLint but in different ways:

  • ESLint detects both control char escapes and control char literals
  • Old deno_lint behavior only detected control char escapes but failed to detect control char literals
  • New deno_lint behavior only detects control char literals and intentionally overlooks control char escapes (reasoning per the linked issue)

In all cases (including ESLint), the checking is done against the regex source string rather than the JS/TS source code. This gives the following behavior:

JS/TS source code ESLint deno_lint
old
deno_lint
new
/�/ (literal 0x1f char) ⚠️ 🆗 ⚠️
/\x1f/ ⚠️ ⚠️ 🆗
new RegExp('�') (literal 0x1f char) ⚠️ 🆗 ⚠️
new RegExp('\x1f') ⚠️ 🆗 ⚠️
new RegExp('\\x1f') ⚠️ ⚠️ 🆗

Also fixes the following 2 bugs noted in the linked issue:

  • Formatting of the message fixed such that code point 0x00 is rendered \x00 not \x0
  • Added \x7f DEL to the detected control chars

@CLAassistant
Copy link

CLAassistant commented Nov 7, 2024

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule suggestion: remove no-control-regex from defaults
2 participants