fix: don't backslash-escape non-syntax characters (invalid escape with POSIX classes)

[maximilliangrand]

Fixes #273.

Problem

Any pattern that puts a POSIX class in the same path portion as a ,, -, # or space throws instead of matching:

> minimatch('foo', ',[[:space:]]')
SyntaxError: Invalid regular expression: /^\,[\p{Z}\t\r\n\v\f]$/u: Invalid escape

The issue reports the comma, but it is not comma-specific. Sweeping every printable ASCII character through `a${ch}b[[:digit:]]` on 10.2.6 gives four offenders:

throws (4): " #,-"
fine  (91): !"$%&'()*+./0123456789:;<=>?@ABC…

Space and - are the ones likely to be hit in practice, since they turn up in ordinary filenames — minimatch('x', 'my file/[[:digit:]]') is fine, but minimatch('x', 'my file[[:digit:]]') throws.

Root cause

[[:space:]] compiles to \p{Z}…, and \p requires the u flag, which AST.toMMRegExp duly sets:

const flags = (this.#options.nocase ? 'i' : '') + (uflag ? 'u' : '')

Under u, only syntax characters may follow a backslash — everything else is an IdentityEscape error. But regExpEscape escapes a much wider set (src/ast.ts:172):

s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&')

so , becomes \,, and the pattern fails to compile. The u flag only propagates within a path portion, which is why a,b/[[:digit:]] is fine while a,b[[:digit:]] is not.

Fix

Escape only what is actually regex syntax outside a character class, which is the sole context this output is used in (re += regExpEscape(c) in ast.ts, and the segment map in index.ts — both append to the regex body):

s.replace(/[[\]{}()*+?.\\^$|]/g, '\\$&')

Dropping ,, -, # and \s is safe because none is regex syntax there: - is only special inside a class, , only inside a {n,m} quantifier — unreachable, since { and } are themselves still escaped — and #/whitespace only under the x flag, which JavaScript does not have. The repo already suspected as much; the assertion in test/defaults.js carried an oxlint-disable … no-useless-escape for exactly this.

Why not encode them instead. My first attempt escaped them as , etc., which is valid in both modes. It corrupts literal patterns: the parsed set is produced by unescape(), which reverses an escape by dropping the backslash, so a,b degrades to the literal au002cb and new Minimatch('a,b').set became [['au002cb']]. Only removing the escape keeps that round-trip exact — there's a regression test pinning it.

This also de-duplicates regExpEscape, which was defined identically in both ast.ts and index.ts; index.ts now imports it.

Behaviour

Matching is unchanged — the escaped and unescaped forms are equivalent in non-u mode, so this only affects the generated source text:

"a,b"             /^a\,b$/            ->  /^a,b$/
"a-b[[:digit:]]"  SyntaxError          ->  /^a-b[\p{Nd}]$/u
"a.b"             /^a\.b$/            ->  /^a\.b$/     (unchanged - real syntax)
"a{2}b"           /^a\{2\}b$/         ->  /^a\{2\}b$/  (unchanged)

The four snapshot files change by exactly one thing — a removed backslash before # or -. Every changed line, deduped:

-/^\#[^/]*?$/                     +/^#[^/]*?$/
-/^\[\#a[^/]*?$/                  +/^\[#a[^/]*?$/
-/^(?:x\-(?:(?!(?:y(?:$|\/)))[^/]*?)|z)?$/
+/^(?:x-(?:(?!(?:y(?:$|\/)))[^/]*?)|z)?$/
-/^a\/\[2015\-03\-10T00:23:08\.647Z\]\/z$/
+/^a\/\[2015-03-10T00:23:08\.647Z\]\/z$/

No structural change, and no anchor, class or quantifier is touched.

Tests

New test/posix-class-escape.js covers the reported pattern, every printable ASCII character against a POSIX class, literal matching for the four affected characters, that genuine syntax characters are still escaped (a.b must not match axb, a{2}b must not match aab), and the unescape round-trip.

Verified fail-then-pass against a true baseline — git show HEAD:src/ast.ts/index.ts restored, rebuilt with tshy, and confirmed the rebuilt dist contained none of the change before running:

BASELINE: # { total: 109,  pass: 101,  fail: 8 }   exit 1
    not ok - Invalid regular expression: /^a\ b[\p{Nd}]$/u: Invalid escape
    not ok - Invalid regular expression: /^a\#b[\p{Nd}]$/u: Invalid escape
    not ok - Invalid regular expression: /^a\,b[\p{Nd}]$/u: Invalid escape
    not ok - Invalid regular expression: /^a\-b[\p{Nd}]$/u: Invalid escape
    not ok - literal patterns round-trip through the parsed set

WITH FIX: # { total: 6352, pass: 6352 }            exit 0

Full suite is green at 6352/6352 (that includes test/redos.js, since this touches a regex). Snapshots regenerated with TAP_SNAPSHOT=1 tap. oxlint src test clean, prettier --check clean.

What I did not verify

I did not benchmark. The replacement character class is strictly smaller than the original, so I would expect no regression, but I have not measured it.

[maximilliangrand]

One more symptom of the same root cause, which I found after opening this and which affects a wider set of patterns than the crash does.

When the offending character and the POSIX class are in different path portions, minimatch() is fine — it matches portion by portion — but makeRe() builds one combined regex, and index.ts collects the u flag from every sub-pattern into that whole regex. The invalid escape then hits this:

try {
  this.regexp = new RegExp(re, [...flags].join(''))
  /* c8 ignore start */
} catch {
  // should be impossible
  this.regexp = false
}

So instead of throwing, makeRe silently hands back false. On published 10.2.6:

"a,b/[[:digit:]]"        makeRe -> false        (fixed: /^a,b\/(?!\.)[\p{Nd}]$/u)
"my file/[[:digit:]]"    makeRe -> false        (fixed: /^my file\/(?!\.)[\p{Nd}]$/u)
"a-b/[[:alpha:]]"        makeRe -> false        (fixed: /^a-b\/(?!\.)[\p{L}\p{Nl}]$/u)

false is a legal return for makeRe, so callers get no error and no regex — anything filtering with makeRe(pattern).test(...) just stops matching. Worth noting the // should be impossible comment was accurate about the intent; this is the case that made it reachable.

To be precise about the blast radius, since it would be easy to overstate: minimatch() returns the correct result for all of these, so ordinary matching is unaffected. I checked directly rather than assuming:

minimatch("a,b/5",     "a,b/[[:digit:]]")     -> true   (both 10.2.6 and fixed)
minimatch("my file/5", "my file/[[:digit:]]") -> true   (both)
minimatch("a-b/x",     "a-b/[[:alpha:]]")     -> true   (both)

Only the makeRe path is affected. Both symptoms come from the one escape change already in this PR — no additional code needed. Happy to fold a case for this into test/posix-class-escape.js if you'd like it pinned separately from the same-portion cases.