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
6.98k stars 332 forks source link

syntax: `$var#...` is incorrectly parsed as a comment #1003

Closed emilazy closed 1 year ago

emilazy commented 1 year ago

Example using shfmt:

emily@yuyuko ~> cat test.bash
foo=abc
bar=def
echo foo#bar
echo foo#$bar
echo $foo#bar
echo $foo#$bar
emily@yuyuko ~> bash test.bash
foo#bar
foo#def
abc#bar
abc#def
emily@yuyuko ~> shfmt test.bash
foo=abc
bar=def
echo foo#bar
echo foo#$bar
echo $foo #bar
echo $foo #$bar
emily@yuyuko ~> shfmt test.bash | bash
foo#bar
foo#def
abc
abc

None of these contain comments, but the latter two examples get turned into comments (and this actually came up in practice; see https://github.com/bouk/babelfish/issues/23). I'm not sure if it's just variable substitutions or if there are other circumstances where this incorrect parse happens.

bouk commented 1 year ago

Seems like adding a spaced check here is all that's needed: https://github.com/mvdan/sh/blob/9e323b8552f73daa5fd63f7dff0dfbc3f7bc3af8/syntax/lexer.go#L272

mvdan commented 1 year ago

Nice find - genuinely surprised that noone spotted this bug before. @bouk has the right idea, but unfortunately the fix isn't that easy. I'm investigating.

mvdan commented 1 year ago

I spent quite a bit of time overthinking different ways to fix this, but I landed on something relatively simple: a small variation on @bouk's idea. It's not a complete fix, as two other cases still don't parse the way they should, but the cases you showed in this issue are all fixed.

emilazy commented 1 year ago

Thanks for this! Would it be good to open an issue for the remaining cases? I haven't seen nix shell $(calculate-flake-ref)#... type constructions appear in a script yet, but I think more out of luck than anything else.

mvdan commented 1 year ago

I left TODOs in the code as a personal reminder, but please feel free to open another issue if you think you'll run into it soon. Particularly if you already have any real example :)

emilazy commented 1 year ago

I'll wait to run into it :)

bouk commented 1 year ago

@mvdan if you cut a release I'll update babelfish!

mvdan commented 1 year ago

done :)