feat: Glob validation for Node/WASM bindings
This PR implements glob validation for the Node/WASM bindings. Currently this is achieved using the existing mechanism for accessing system functions instead of one of the approaches described in https://github.com/mpalmer/action-validator/issues/40.
Since this is branched off from https://github.com/mpalmer/action-validator/pull/86, I've marked it as a draft until this is merged. I'll then rebase this PR and mark it as being ready for review.
@mpalmer now that https://github.com/mpalmer/action-validator/pull/86 has landed, I've rebased this PR and it's now ready for review
The PR itself looks reasonable. Given the number of "globbing doesn't work exactly the same as GitHub's" issues that have been filed, I can't help but wonder: how similar are the matching semantics of the JS fast-glob package, compared to the glob crate? My concern is that we'll end up with a bunch more "globbing doesn't work right" issues, but they'll be native/WASM-specific issues.
To clarify, I'm not against merging this as-is, I guess I just want to know what to expect in the issue tracker once this lands.
Given the number of "globbing doesn't work exactly the same as GitHub's" issues that have been filed, I can't help but wonder: how similar are the matching semantics of the JS
fast-globpackage, compared to theglobcrate?
Good question!
I have added some tests based on GitHub's documentation for their "filter patterns" (see https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#filter-pattern-cheat-sheet).
As far as I can tell, the behavior is the same for fast-glob as for the glob crate, except that fast-glob supports the extended gitignore syntax (e.g. *.{js,jsx,ts,tsx}) while glob does not. This would actually a bug in the Node version of AV since GitHub doesn't support the extended syntax. I'd be happy to look into a fix for this when I have some time. Would you consider this a blocker for the current PR or something which can be fixed in a followup?
As it stands, it seems most of the GitHub syntax is supported by both AV native and node, which I've asserted with the addition of this test.
Currently supported:
- "*" ✅
- "**" ✅
- "?" ✅
- "+" 🚫
- "[]" ✅
- "!" ✅
I've also added a test for rejecting extended gitignore syntax, but this is currently ignored when running npm test because of the aforementioned bug.
Would you consider this a blocker for the current PR or something which can be fixed in a followup?
No, our globbing support is already at odds with GitHub's; "differently broken" isn't a blocker, I just like to know what to say when the next round of "this globbing corner case doesn't work" issues come in.