Skip to content

Commit

Permalink
syntax: better error when using "function" in POSIX mode
Browse files Browse the repository at this point in the history
When parsing in POSIX mode, we already gave a good error message
when a user used the "function foo()" Bash syntax:

    $ shfmt -p <<<'function foo() { bar; }'
    <standard input>:1:13: the "function" builtin exists in bash; tried parsing as posix

However, our heuristic depended on the opening parenthesis token,
so it didn't work without the parentheses, which is valid Bash:

    $ shfmt -p <<<'function foo { bar; }'
    <standard input>:1:21: "}" can only be used to close a block

This "can only be used to close a block" error was rather confusing,
and caused multiple users to file bugs thinking the parser was at fault.
We now catch this other common pitfall pattern as well:

    $ shfmt -p <<<'function foo { bar; }'
    <standard input>:1:14: the "function" builtin is a bash feature; tried parsing as posix

We add the logic to the bit of code that handles each call argument,
but there shouldn't be a noticeable impact to most users
since it only kicks in when we're parsing in POSIX mode
and the current argument is a simple "{" literal.

Fixes #993.
  • Loading branch information
mvdan committed Jul 13, 2023
1 parent 1cb4b28 commit 25abe06
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
6 changes: 5 additions & 1 deletion syntax/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2422,6 +2422,10 @@ loop:
ce.Assigns = append(ce.Assigns, p.getAssign(true))
break
}
// Avoid failing later with the confusing "} can only be used to close a block".
if p.lang == LangPOSIX && p.val == "{" && w != nil && w.Lit() == "function" {
p.curErr("the %q builtin is a bash feature; tried parsing as posix", "function")
}
ce.Args = append(ce.Args, p.wordOne(p.lit(p.pos, p.val)))
p.next()
case _Lit:
Expand Down Expand Up @@ -2453,7 +2457,7 @@ loop:
// Note that we'll only keep the first error that happens.
if len(ce.Args) > 0 {
if cmd := ce.Args[0].Lit(); p.lang == LangPOSIX && isBashCompoundCommand(_LitWord, cmd) {
p.curErr("the %q builtin exists in bash; tried parsing as posix", cmd)
p.curErr("the %q builtin is a bash feature; tried parsing as posix", cmd)
}
}
p.curErr("a command can only contain words and redirects; encountered %s", p.tok)
Expand Down
10 changes: 7 additions & 3 deletions syntax/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1856,15 +1856,19 @@ var shellTests = []errorCase{
},
{
in: "function foo() { bar; }",
posix: `1:13: the "function" builtin exists in bash; tried parsing as posix`,
posix: `1:13: the "function" builtin is a bash feature; tried parsing as posix`,
},
{
in: "function foo { bar; }",
posix: `1:14: the "function" builtin is a bash feature; tried parsing as posix`,
},
{
in: "declare foo=(bar)",
posix: `1:13: the "declare" builtin exists in bash; tried parsing as posix`,
posix: `1:13: the "declare" builtin is a bash feature; tried parsing as posix`,
},
{
in: "let foo=(bar)",
posix: `1:9: the "let" builtin exists in bash; tried parsing as posix`,
posix: `1:9: the "let" builtin is a bash feature; tried parsing as posix`,
},
{
in: "echo <(",
Expand Down

0 comments on commit 25abe06

Please sign in to comment.