Drop dependency on bash-parser
#72
Comments
|
It was difficult to even find such a package, let alone a recent or actively maintained one. On the other hand, it provides functionality that makes Knip quite powerful in finding dependencies and entry files. This test shows quite some cases it can handle, and the scripts can come from I would also be happy to replace bash-parser with something more light-weight and well-maintained, but I'm not willing to consider losing this type of functionality. Perhaps it could be some combination of bash-parser becoming an optional dependency and opt-out by configuration or something. Just for reference, I started out with a bunch of regexes, but that's far from elegant, limited and hard to maintain. On the other hand, we're talking Bash here, syntax doesn't really change at all. It parses strings in configuration files. I wonder how can this package can actually be a threat or exploited? If it is I should perhaps add a warning to the docs. Very open to ideas and suggestions here, I want Knip to be a good citizen in anyone's |
|
Thanks for the detailed explanation! Since knip is a dev dependency, I am not worried but I simply would like to get rid of unmaintained deps as much as possible. Reporting issues to multiple repositories to move the ecosystem forward. No need to be actionable on this right away, let's keep it open for reference and maybe a better alternative will come around for the package or someone might have a recommendation! |
|
It's causing errors for me |
|
Any chance you could look up the script that contains the "less than" ( The issue could be in Knip as well, and at least it should try to work around it. |
|
For me it errored on an Removing this script entry fixed it for me (though different issue than the reported one above) |
|
I fixed that it throws so hard on bash parser errors by swallowing them in v2.1.2. Errors still show up in |
|
On a related note, While not actively addressing the maintainance issue on which this issue is based, it might make sense to fork/vendor |
|
Since the latest release of vorpaljs/bash-parser is 6 years ago I wouldn't mind depending on a fork. Still on-topic, does anybody have an idea about possible alternatives? |
|
Thanks to @ericcornelissen, v2.15.1 should be less problematic in terms of dependencies and licenses. Thanks again, Eric. Still thinking it could be a nice weekend project to let some model do the hard work here and build a modern alternative to bash parser. |
|
After seeing tree-sitter again the other day I tried using it in Knip with the Bash grammar to replace Pros:
Cons:
Although we may not like the age of bash-parser, it did a very decent job. So here I am asking for your opinion, security assessment, and hopefully a test run on your own project(s). You can install v2.17.2-ts.0 with the |
|
Looking at tree-sitter, its fork and bash-parser I am coming to the conclusion: Probably your best bet would be to ask for a new release for tree-sitter-bash as there are somewhat frequent commits on the repo recently. That dependency pulls in way more dependencies on its own though than bash-parser ~(68 vs 28) Forks don't make sense most of the time because they are rarely ever maintained over a longer period of time. All 3 have CVEs, so really not a great choice here. |
|
A new release was requested in October 2022 in tree-sitter/tree-sitter-bash#134. Tried to nudge the owner now. At this point I think tree-sitter is the best option. |
|
🚀 This issue has been resolved in v2.19.6. See Release 2.19.6 for release notes. |
|
Since [email protected] was released the other day I've pulled the trigger here as well. Would deserve either a major bump or just a patch release. As I think it shouldn't cause friction anywhere I opted for the latter (I do expect an occasional issue around arguments w/ quotes here and there, let's see). |
[Wrong parsing for `${!#}` · Issue #160 · tree-sitter/tree-sitter-bash](tree-sitter/tree-sitter-bash#160)
> Same problem here, with a similar variable replacement:
> `display="${display%|*}"`
> It makes the syntax broken for the rest of the file
[Fixes by amaanq · Pull Request #186 · tree-sitter/tree-sitter-bash](tree-sitter/tree-sitter-bash#186)
[Drop dependency on `bash-parser` · Issue #72 · webpro/knip](webpro-nl/knip#72)
> Since [email protected] was released the other day I've pulled the
> trigger here as well. Would deserve either a major bump or just a patch
> release. As I think it shouldn't cause friction anywhere I opted for the
> latter (I do expect an occasional issue around arguments w/ quotes here
> and there, let's see).
>
> 🚀 This issue has been resolved in v2.19.6. See Release 2.19.6 for
> release notes.
|
🚀 This change has been reverted in v2.19.11. See Release 2.19.11 for release notes. |
We recently started using https://socket.dev/ as part of our dependencies maintenance and the tool has detected
bash-parseras problematic due to being unmaintained for 6 years: https://socket.dev/npm/package/bash-parserbash-parseritself also uses old and unmaintained dependencies, e.g. https://socket.dev/npm/package/deep-freeze which is a repo that has a non-existent npm account associated to it.The text was updated successfully, but these errors were encountered: