oxc-project / oxc

⚓ A collection of JavaScript tools written in Rust.
https://oxc.rs
MIT License
10.64k stars 391 forks source link

bug(linter) incorrect fixer for `no-useless-escape` #5227

Open camc314 opened 2 weeks ago

camc314 commented 2 weeks ago
const regex = /(https?:\/\/github\.com\/(([^\s]+)\/([^\s]+))\/([^\s]+\/)?(issues|pull)\/([0-9]+))|(([^\s]+)\/([^\s]+))?#([1-9][0-9]*)($|[\s\:\;\-\(\=])/;

is being fixed to

const regex = /(https?:\/\/github\.com\/(([^\s]+)\/([^\s]+))\/([^\s]+\/)?(issues|pull)\/([0-9]+))|(([^\s]+)\/([^\s]+))?#([1-9][0-9]*)($|[\s:;-(=])/;

which is an invalid regex


  × Invalid regular expression: Character class range out of order
   ╭─[./src/vs/workbench/api/test/browser/extHostDocumentData.test.ts:2:142]
 1 │ const regex = /(https?:\/\/github\.com\/(([^\s]+)\/([^\s]+))\/([^\s]+\/)?(issues|pull)\/([0-9]+))|(([^\s]+)\/([^\s]+))?#([1-9][0-9]*)($|[\s:;-(=])/;
   ·                                                                                                                                              ──
   ╰────

Finished in 3ms on 1 file with 93 rules using 12 threads.
Found 0 warnings and 1 error.
camc314 commented 2 weeks ago

looks like eslint doesn't report an error for this case (escacpe on the \-), but we do.

gonna leave the fixer alone and fix this false positive

oxlint >> https://oxc-project.github.io/oxc/playground/?code=3YCAAICggICAgICAgICxG0qZRraXTnShZDZHIj18ztvE%2FrAEWU6BK3tu8l%2BruCZLm6bBN39t5QCA

eslint >> https://eslint.org/play/#eyJ0ZXh0IjoiY29uc3QgcmVnZXgwID0gL1tcXHNcXDpcXDtcXC1cXChcXD1dLzsiLCJvcHRpb25zIjp7InJ1bGVzIjp7Im5vLXVzZWxlc3MtZXNjYXBlIjpbImVycm9yIl19LCJsYW5ndWFnZU9wdGlvbnMiOnsicGFyc2VyT3B0aW9ucyI6eyJlY21hRmVhdHVyZXMiOnt9fX19fQ==

Screenshot 2024-08-26 at 15 43 11

minimal test case:

/[\s\-(]/;

update: i think it's going to be better if we wait till the linter has access to the pared regex ast and do it that way, else it's just going to be duplicating some of the regex parsing code

Boshen commented 5 days ago

I think it's going to be better if we wait till the linter has access to the pared regex ast and do it that way, else it's just going to be duplicating some of the regex parsing code

We do now 😁