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

ESLint: Added regexp plugin #2924

Merged
merged 3 commits into from
Jun 13, 2021

Conversation

RunDevelopment
Copy link
Member

This PR adds the eslint-plugin-regexp plugin. This plugin statically analyses regex patterns and usages to find potential problems and suggest improvements. (Full disclaimer: I am a maintainer of the plugin.)

Right now, I only enabled a few rules to handle common things in code review (all warnings) and a few rules to detect (potential) errors.


@LeaVerou @Golmote @mAAdhaTTah

There is another really good rule I am unsure about adding, the regexp/no-dupe-disjunctions rule. This rule actually finds more than a dozen bugs in our regexes (not fixed here) and even points out likely causes of exp backtracking directly and live in the IDE.

However, the rule is slow. Right now, it increases the total linting time 5x (npm run lint), from 5s to 25s on my machine. This doesn't seem to be a problem for live code editing/viewing.

Should I also enable the regexp/no-dupe-disjunctions rule?

@github-actions
Copy link

github-actions bot commented May 31, 2021

JS File Size Changes (gzipped)

A total of 42 files have changed, with a combined diff of -83 B (-0.2%).

file master pull size diff % diff
components/prism-ada.min.js 548 B 544 B -4 B -0.7%
components/prism-aspnet.min.js 504 B 504 B 0 Bytes 0%
components/prism-autohotkey.min.js 4.13 KB 4.13 KB -3 B -0.1%
components/prism-bash.min.js 2.87 KB 2.87 KB -1 B -0.0%
components/prism-basic.min.js 1.12 KB 1.12 KB -4 B -0.4%
components/prism-bsl.min.js 747 B 741 B -6 B -0.8%
components/prism-cil.min.js 1.12 KB 1.11 KB -5 B -0.4%
components/prism-csp.min.js 486 B 485 B -1 B -0.2%
components/prism-css-extras.min.js 1.46 KB 1.45 KB -6 B -0.4%
components/prism-dataweave.min.js 509 B 508 B -1 B -0.2%
components/prism-elixir.min.js 845 B 844 B -1 B -0.1%
components/prism-false.min.js 260 B 260 B 0 Bytes 0%
components/prism-firestore-security-rules.min.js 416 B 418 B +2 B +0.5%
components/prism-gherkin.min.js 5.16 KB 5.16 KB -2 B -0.0%
components/prism-groovy.min.js 898 B 898 B 0 Bytes 0%
components/prism-ichigojam.min.js 561 B 556 B -5 B -0.9%
components/prism-java.min.js 991 B 991 B 0 Bytes 0%
components/prism-javascript.min.js 1.48 KB 1.48 KB +2 B +0.1%
components/prism-javastacktrace.min.js 614 B 613 B -1 B -0.2%
components/prism-jexl.min.js 320 B 319 B -1 B -0.3%
components/prism-jsx.min.js 957 B 961 B +4 B +0.4%
components/prism-latex.min.js 480 B 474 B -6 B -1.3%
components/prism-latte.min.js 522 B 520 B -2 B -0.4%
components/prism-markdown.min.js 1.76 KB 1.76 KB -3 B -0.2%
components/prism-nasm.min.js 475 B 473 B -2 B -0.4%
components/prism-nevod.min.js 735 B 730 B -5 B -0.7%
components/prism-perl.min.js 1 KB 996 B -6 B -0.6%
components/prism-php.min.js 1.96 KB 1.96 KB -2 B -0.1%
components/prism-promql.min.js 654 B 648 B -6 B -0.9%
components/prism-properties.min.js 192 B 190 B -2 B -1.0%
components/prism-renpy.min.js 2.04 KB 2.04 KB -1 B -0.0%
components/prism-rest.min.js 1.07 KB 1.07 KB -1 B -0.1%
components/prism-sas.min.js 3.02 KB 3.01 KB -9 B -0.3%
components/prism-scss.min.js 630 B 629 B -1 B -0.2%
components/prism-squirrel.min.js 597 B 600 B +3 B +0.5%
components/prism-stan.min.js 699 B 697 B -2 B -0.3%
components/prism-stylus.min.js 1.61 KB 1.61 KB -2 B -0.1%
components/prism-tcl.min.js 852 B 850 B -2 B -0.2%
components/prism-v.min.js 977 B 975 B -2 B -0.2%
components/prism-vala.min.js 985 B 985 B 0 Bytes 0%
components/prism-wasm.min.js 675 B 674 B -1 B -0.1%
plugins/line-highlight/prism-line-highlight.min.js 1.43 KB 1.43 KB +2 B +0.1%

Generated by 🚫 dangerJS against 2e4dd47

@mAAdhaTTah
Copy link
Member

This is kinda awesome, and I love how it makes every language slightly smaller. As for the other rule, maybe open in a separate PR and discuss there? Would be good to see the impact on the code.

@RunDevelopment
Copy link
Member Author

I love how it makes every language slightly smaller

In that case, we might also want to add the regexp/order-in-character-class rule which sorts the elements of character classes. This will make it easier for gzip to compress our regexes.

As for the other rule, maybe open in a separate PR and discuss there? Would be good to see the impact on the code.

Good idea. There are also a few other rules I would like to add. I'll make PRs for each.

@RunDevelopment RunDevelopment merged commit 18a0082 into PrismJS:master Jun 13, 2021
@RunDevelopment RunDevelopment deleted the eslint-regexp branch June 13, 2021 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants