-
-
Notifications
You must be signed in to change notification settings - Fork 892
Description
What version of Oxlint are you using?
1.51.0
What command did you run?
No response
What does your .oxlintrc.json (or oxlint.config.ts) config file look like?
{
"rules": {
"capitalized-comments": ["error", "always", {
"line": {
"ignorePattern": ".{0,20}$|[a-z]+ ?[0-9A-Z_.(/=:[#-]|std|http|ssh|ftp",
"ignoreInlineComments": true,
"ignoreConsecutiveComments": true
},
"block": {
"ignorePattern": ".*"
}
}]
}
}This is extracted from NodeJS's ESLint config: https://github.com/nodejs/node/blob/f38a739623848a2eb6e7cc9f8d323b063c7ced9b/eslint.config.mjs#L160-L171
What happened?
ignorePattern option is ignored. This produces 1700 errors in NodeJS's repo when using Oxlint.
Diagnosis
ESLint compiles it into a regex with:
const regExp = RegExp(`^\\s*(?:${ignorePatternStr})`, "u");(source)
Our implementation this rule follows the same pattern:
oxc/crates/oxc_linter/src/rules/eslint/capitalized_comments.rs
Lines 94 to 95 in 78f9504
| ignore_pattern: pattern | |
| .and_then(|p| RegexBuilder::new(&format!(r"^\s*(?:{p})")).build().ok()), |
However, the difference is that we use regex crate, which is not entirely compatible with JS regex syntax.
The culprit is the [ inside a [...] block. regex::RegexBuilder considers this an invalid regex, so the ignorePattern is ignored.
Simplified reproduction: "ignorePattern": "[[]".
Impact
The differences in JS RegExp vs regex crate is pretty obscure, and finding the source of the errors in NodeJS was time-consuming. It's not great to inflict this bug-hunt on users.
Most users I'd guess wouldn't even be aware that other regex styles even exist.
Potential solutions
1. Use JS-compatible regex engine
This is the approach that #20111 takes - replace regex with regress, which provides JS-compatible regexes.
Arguments for/against this approach:
For: Eases migration from ESLint to Oxlint.
Against: regress is likely slower than regex crate. We penalize the common case to support fairly uncommon cases.
2. Better errors
Currently we just silently ignore ignorePattern entirely if RegexBuilder says it's invalid. We could instead produce an error which explains what the problem is and how to fix it.
Downside: The differences between regex implementations are fairly subtle. It'd be hard to write good docs explaining exactly how to fix it.