Skip to content

Commit

Permalink
syntax: don't parse $foo#bar as a comment
Browse files Browse the repository at this point in the history
And the same goes for:

    ${foo}#bar
    'foo'#bar
    "foo"#bar

Note that this isn't a complete fix, per the added TODOs,
since we don't have a fix for command substitutions.
However, this still fixes four cases where we got it wrong.

Fixes #1003.
  • Loading branch information
mvdan committed Jun 10, 2023
1 parent 39a33f6 commit 829b814
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 0 deletions.
7 changes: 7 additions & 0 deletions syntax/filetests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1743,6 +1743,13 @@ var fileTests = []testCase{
Strs: []string{"foo#bar"},
common: litWord("foo#bar"),
},
{
Strs: []string{"$foo#bar foo#$bar"},
common: call(
word(litParamExp("foo"), lit("#bar")),
word(lit("foo#"), litParamExp("bar")),
),
},
{
Strs: []string{"{ echo } }; }"},
common: block(litStmt("echo", "}", "}")),
Expand Down
12 changes: 12 additions & 0 deletions syntax/lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,18 @@ skipSpace:
case ';', '"', '\'', '(', ')', '$', '|', '&', '>', '<', '`':
p.tok = p.regToken(r)
case '#':
// If we're parsing $foo#bar, ${foo}#bar, 'foo'#bar, or "foo"#bar,
// #bar is a continuation of the same word, not a comment.
// TODO: support $(foo)#bar and `foo`#bar as well, which is slightly tricky,
// as we can't easily tell them apart from (foo)#bar and `#bar`,
// where #bar should remain a comment.
if !p.spaced {
switch p.tok {
case _LitWord, rightBrace, sglQuote, dblQuote:
p.advanceLitNone(r)
return
}
}
r = p.rune()
p.newLit(r)
runeLoop:
Expand Down
7 changes: 7 additions & 0 deletions syntax/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,13 @@ var printTests = []printCase{
},
samePrint("(\n\t((foo++))\n)"),
samePrint("(foo && bar)"),
samePrint(`$foo#bar ${foo}#bar 'foo'#bar "foo"#bar`),
// TODO: support cases with command substitutions as well
// {
// "`foo`#bar",
// "$(foo)#bar",
// },
// samePrint(`$("foo"#bar)#bar`),
}

func TestPrintWeirdFormat(t *testing.T) {
Expand Down

0 comments on commit 829b814

Please sign in to comment.