Skip to content

[ESLint] Drive lint warnings to zero - #4090

Merged
DmitrySharabin merged 1 commit into
v2from
eslint-warnings-cleanup
Jun 26, 2026
Merged

[ESLint] Drive lint warnings to zero#4090
DmitrySharabin merged 1 commit into
v2from
eslint-warnings-cleanup

Conversation

@DmitrySharabin

@DmitrySharabin DmitrySharabin commented Jun 26, 2026

Copy link
Copy Markdown
Member

Summary

Restores the team's intentional ESLint regex policy and drives npm run lint:ci warnings from 869 → 0 with zero churn in language grammars.

The root cause was migration drift: during the ESLint v9 flat-config migration (cbfeb53d), regexpPlugin.configs.recommended.rules was spread alongside eslint-comments/recommended and the (now-removed) TypeScript recommended configs. The pre-v9 .eslintrc.js had never extended plugin:regexp/recommended — the team had a hand-curated 29-rule list at the top of the JS rules block. Restoring the deletion makes the curated list the active policy again.

Changes to eslint.config.mjs

  1. Remove the regexp/recommended spread — restores the curated regex policy. ~830 stylistic warnings vanish, no code touched.
  2. Add 11 bug-catching rules from regexp/recommended that survived the audit (see below).
  3. Disable prefer-const — matches the team convention of reusing let bindings (already disabled on the simplify branch in b4366b2b).
  4. Add demo-page globals for src/plugins/**/demo.js (Prism, JSZip, components, saveAs) plus no-unused-vars: off (these scripts expose functions consumed by HTML attributes like data-adapter="…").

Audit — which regexp/recommended rules we kept and why

Rather than restore the whole preset, I read every currently-active warning in actual Prism source. Every one of the 32 currently-firing warnings fires on an intentional Prism pattern, not a bug:

Rule Hits What it actually catches Verdict
regexp/no-contradiction-with-assertion 15 ^...$[\s\S]*?^\1 multi-line block matchers (asciidoc, autoit, d, hcl) — work correctly Drop
regexp/no-misleading-capturing-group 11 8× textile.js <MOD> template substitution (rule can't see substitution); 3 valid edge cases that work for valid input Drop
regexp/no-empty-character-class 4 [^\s\S] / [] deliberately used as recursion terminators in ftl/icu-message-format/lilypond/rust grammars Drop
regexp/no-extra-lookaround-assertions 1 Single mild optimization in solution-file.js Drop
regexp/no-misleading-unicode-character 1 [\r\n] in systemd.js — rule is for grapheme clusters, not control chars Drop

The 11 rules we kept all have zero current warnings but guard real bug classes Prism could introduce later: no-empty-group, no-empty-string-literal, no-escape-backspace, no-invalid-regexp, no-invisible-character, no-legacy-features, no-missing-g-flag, no-non-standard-flag, no-potentially-useless-backreference, no-useless-backreference, no-useless-dollar-replacements.

Code changes outside the config

After the config changes, 11 warnings remained. Auto-fix handled most:

  • src/languages/python.js — sorted lazy alphabetically into the keyword alternation (safe; \b boundaries handle prefix collisions)
  • src/plugins/autoloader/demo.js — escaped \} in /\{id\}/g
  • src/plugins/toolbar/demo.jsonClick: function (env) {onClick (env) {
  • src/util/async.js — removed trailing blank doc-comment line
  • src/languages/markdown.js — removed 2 dead imports left over from a recent refactor (b8180e33)

Four inline-disabled with justifying comments:

  • src/languages/docker.js:18no-dupe-disjunctions false positive (the (?!["']) lookahead makes the unquoted alternative disjoint from <STR>)
  • src/languages/mongodb.js:291, src/languages/promql.js:42, src/languages/promql.js:89sort-alternatives on arrays intentionally ordered by semantics/docs rather than alphabet

Verification

  • npm run lint:ci — 0 warnings (--max-warnings 0 passes)
  • npm test — full suite (test:components/core/identifiers/languages/patterns/plugins/runner)
  • npm run regex-coverage — 980 passing
  • npm run typecheck — clean (core + tests)
  • npm run build — clean

Not in scope

  • Bumping eslint-plugin-regexp 2.7.0 → 3.1.1 (separate PR; v3 likely tightens rules)
  • Adjusting --max-warnings 0 (the threshold is correct; goal was to make the count zero)
  • Any regex literal in src/languages/**

Test plan

  • npm run lint:ci passes on CI (verified locally and in CI lint job)
  • All other CI checks (build, test, typecheck, regex-coverage) still green

`regexp/recommended` was added unintentionally during the ESLint v9 migration
(cbfeb53) and overrode the curated regex ruleset. Remove that spread so the
hand-picked list at the top of eslint.config.mjs is the active policy. Re-add
11 bug-catching rules from `recommended` that don't fight Prism's grammar
idioms. Disable `prefer-const` to match team convention. Add globals for
`src/plugins/**/demo.js`. Apply minor auto-fixes and inline-disable 4
intentional regex patterns.

869 warnings → 0; lint:ci, build, typecheck, tests, and regex-coverage all pass.
@netlify

netlify Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploy Preview for dev-prismjs-com ready!

Name Link
🔨 Latest commit dc67651
🔍 Latest deploy log https://app.netlify.com/projects/dev-prismjs-com/deploys/6a3ec903048e640008244bb7
😎 Deploy Preview https://deploy-preview-4090--dev-prismjs-com.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

Copy link
Copy Markdown

No JS Changes

Generated by 🚫 dangerJS against dc67651

@DmitrySharabin
DmitrySharabin merged commit ea428b3 into v2 Jun 26, 2026
22 of 25 checks passed
@DmitrySharabin
DmitrySharabin deleted the eslint-warnings-cleanup branch June 26, 2026 19:16
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.

2 participants