fix: replace parse-glob

[joeyparrish]

This is a rebased version of #827 by @josundt, with the JS binaries regenerated as well. It can be used directly as a dependency (github:joeyparrish/HTMLHint#1c3a7e8b) while waiting for the fix to be merged upstream in HTMLHint.

Closes #664

---

This PR replaces the abandoned, security-vulnerable parse-glob package with a custom parse-glob.js module/function and thereby fixes issue #664

The added module exactly mimics the behavior of the function from the parse-glob package, but only includes the subset that is required by HTMLHint in the returned object. It uses is-glob to validate the glob.

Changes:

• Added parse-glob.ts to replace the vulnerable package

• Added a simple unit test to prove that my new module returns the same result as the replaced package for the required properties used by HTMLHint.

• package.json:

  • Removed the @types/parse-glob package from devDependencies
  • Moved parse-glob from dependencies to devDependencies (currently needed for the added unit test)
  • Added is-glob as dependency

This PR is meant as an intermediate step to prove that the parse-glob package has been properly replaced.

As a next step it would make sense to:

• Remove the parse-glob devDependency - and the simple unit test
• Simplify the returned object from the new parseGlob function.

[stuartbale]

Please can someone review and approve this!!!

[joeyparrish]

Ping! @thedaviddias, can you please take a look or assign someone else who can?

[thedaviddias]

Nice work @joeyparrish !

[kho-kho-kho]

Are the failing checks going to prevent this from being merged? Looks like the test infra is still expecting to check parse-glob as part of its suite.

[joeyparrish]

The tests @josundt wrote for the parse-glob replacement were written using expect.js. I updated them to use the same framework as the other tests, and they pass now. Sorry I overlooked that!

[codecov]

Codecov Report

Merging #927 (bed4e53) into master (8a38cf3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #927   +/-   ##
=======================================
  Coverage   97.10%   97.11%           
=======================================
  Files           1        2    +1     
  Lines        1626     1627    +1     
  Branches      288      289    +1     
=======================================
+ Hits         1579     1580    +1     
  Misses         47       47           

Impacted Files	Coverage Δ
src/cli/parse-glob.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e4967c...bed4e53. Read the comment docs.

[stuartbale]

@joeyparrish - appears there is a failing test - any chance you can investigate please?
https://github.com/htmlhint/HTMLHint/runs/5557533268?check_suite_focus=true

[joeyparrish]

CLI › Formatter: compact › should have stdout output with formatter compact

thrown: "Exceeded timeout of 30000 ms for a test.

Only appears on one builder, and doesn't appear to be related to the changes. I can't reproduce it locally. I would suggest re-running the tests to see if it was just a flaky test or a flaky test runner.

[stuartbale]

Thanks @joeyparrish
what do you make of this failure?
https://github.com/htmlhint/HTMLHint/runs/5557533268?check_suite_focus=true#step:8:328

[joeyparrish]

I really don't know. I can't reproduce it locally. I expect the two errors (fewer lines of output than expected from the CLI and a timeout in the overall test execution) could be related, but I can't figure out an easy way to prove that. But that test should never take 30s, none of the other 11 jobs had that issue on any other combination of OS, and nothing about it should be node-version-dependent or OS-dependent... so I think something must have been wrong with that VM at the time it was run.

Have you tried re-running that one job to see if it's a transient issue in GitHub's VMs? For admins, there should be a small "refresh"-type button to the right of the job when you hover over it, which says "re-run this job". Here's a screenshot from one of my repos:

[stuartbale]

Sorry @joeyparrish I don't have access to the repo as Admin - I'm just a hopeful voyeur who has been watching this thread (and others like it) in the hope that this issue can be resolved.

Interestingly it fails in the MacOS area - I use Mac so I'll pull down this branch locally and see if I can replicate the issue - unless you have already tested on Mac?

[joeyparrish]

Ah, I see. Well, my best guess is that the job needs to be re-run.

@thedaviddias, can you take a look, please? I think this is safe to merge, and that a re-run of that failing job would pass.

[joeyparrish]

Thank you!

[thedaviddias]

🎉 This PR is included in version 1.1.3 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀