mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.1k stars 336 forks source link

cmd/shfmt: option to check syntax and not produce any output #621

Open zackw opened 3 years ago

zackw commented 3 years ago

The readme suggests using shfmt script > /dev/null to check a script for syntax errors. For very large scripts, it would be more efficient if there were an option that has shfmt only check syntax and not bother with the pretty-printing and formatting that is just going to be thrown away anyway.

mvdan commented 3 years ago

If you're after efficiency, have you considered just using the parser directly? See https://pkg.go.dev/mvdan.cc/sh/v3/syntax#example-package.

It's quite literally a few lines of Go, so I'm not convinced we should provide a program just for it. For those with simple enough use cases, shfmt is probably good enough.

It's also worth noting that parsing is significantly more costly than printing, so I don't imagine the speed-up would be more than about 20%. Unless you mean a parser mode where the syntax tree isn't built at all, but that would be a pretty significant refactor of the parser.

We could add another flag to shfmt, but I really want to avoid cluttering the tool with dozens of potential use cases. It's a formatter and it already has lots of flags as it is.

zackw commented 3 years ago

I am after efficiency, but in the context where I'd be using this, I don't have the ability to compile Go programs myself. shfmt may be preinstalled on the system or it may not; if it's available I'd use it, otherwise I'd fall back to bash -n.

I appreciate the desire to keep the flags to a minimum, but a "just parse, no output" mode is common in compilers and in shell utilities where the output may or may not be useful (e.g. cmp) so I don't think it's too much of an ask.

mvdan commented 3 years ago

Have you benchmarked or measured if shfmt is a bottleneck, or how much you would win by not printing? I'm still not convinced this is worth the effort. If you truly wanted a "just parse, and do nothing else" mode, that would be a significant rewrite. A mode that constructs the syntax tree and doesn't print would win you very little, I'm willing to bet.

And if you really do care about shaving off a few percent of cpu time, then I think you're in the camp where building custom software seems like a reasonable thing to do.

mvdan commented 3 years ago

I should clarify that adding a shfmt -n mode which truly is faster because it doesn't construct the syntax tree might be interesting in the future. It could be 2x or even 3x as fast, by avoiding all the memory allocations and GC work as well. That seems more deserving of an extra flag than simply "don't print anything and save a bit of time".

That said, it would be a non-trivial rewrite, so I want to properly balance that against the other issues and features.

mvdan commented 3 years ago

Thinking outloud some more, we could end up with a companion API to the Parse method:

func (p *Parser) Parse(r io.Reader, name string) (*File, error)
func (p *Parser) Check(r io.Reader, name string) error