function definition should allow compound command

[mmjo]

For bugs

• Rule Id: SC1073, SC1064, SC1072
• My shellcheck version (shellcheck --version or 'online'): online
• [x] I tried on shellcheck.net and verified that this is still a problem on the latest commit

For new checks and feature suggestions

• [x] shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this
• [x] I searched through https://github.com/koalaman/shellcheck/issues and didn't find anything related

Here's a snippet or screenshot that shows the problem:

#!/bin/bash
f() [[ 1 = 1 ]]

Here's what shellcheck currently says:

Line 2	SC1073: Couldn't parse this function. Fix to allow more checks.
Line 2	SC1064: Expected a { to open the function definition.
Line 2	SC1072:  Fix any mentioned problems and try again.

Here's what I wanted or expected to see:

No warnings.

Musings

It seems from src/Parser.hs in readFunctionDefinition that shellcheck only allows f() {...} and f() (...).

Though both bash and posix allow a compound command as function body.

Note that the readCompoundCommand could have been useful. But unfortunately it includes readFunctionDefinition itself.

[matthewpersico]

What would this do? If you try something useful:

 $ f() echo hello
-bash: syntax error near unexpected token `echo'

[mmjo]

'echo hello' is not a compound command, so the error is expected.

A slightly more useful example: is_ok () [[ "$1" = 42 ]]

Compare against the verbose version (no need for a return, but do not forget the semicolon): is_ok () { [[ "$1" = 42 ]]; }

You can even write: is_ok () if [ "$1" = 42 ]; then return 0; else return 1; fi

Instead of an 'if'-statement, a case/while/until/select/for statement is also possible. [but these i never use]

Last possibility is arithmetic. E.g. is_ok () (( "$1" - 42 == 0 ))

[matthewpersico]

TIL

[TinCanTech]

Last possibility is arithmetic. E.g. is_ok () (( "$1" - 42 == 0 ))

According to Douglas Adams, The Answer is not 42 because The Question is:

• What is five times nine ?

42 was a Deep-Thought error. Which was the reason for Earth.

[oguz-ismail]

 $ f() echo hello
-bash: syntax error near unexpected token `echo'

@matthewpersico most shells (including the ones below and also /bin/sh on FreeBSD, NetBSD, Solaris 10, and UnixWare 7.1.4) accept a pipeline as function body, bash and yash are the exceptions.

$ for sh in bash bosh 'busybox sh' dash gwsh mksh ksh oksh yash zsh; do echo $sh; $sh -c 'f() echo a; f'; done
bash
bash: -c: line 1: syntax error near unexpected token `echo'
bash: -c: line 1: `f() echo a; f'
bosh
a
busybox sh
a
dash
a
gwsh
a
mksh
a
ksh
a
oksh
a
yash
yash -c:1: syntax error: a function body must be a compound command
zsh
a

[koalaman]

Note that the readCompoundCommand could have been useful. But unfortunately it includes readFunctionDefinition itself.

This is not a problem for dash:

$ f() f() { echo foo; }
$ f
$ f
foo

The original reason for not allowing anything other than {} and () was that ShellCheck kept giving misleading advice when it interpreted bad array assignments and similar as possible function definitions. However, a lot of that parsing was rewritten in 0.7.2, so it may be feasible to allow this again.

[dermoth]

Bash function can be any compound commands which can have any of these forms. All of them are well known to shellcheck already, but it isn't expecting anything other than (list) and { list; } right now:

• (list)
• { list; }
• ((expression))
• [[ expression ]]
• for name [ [ in [ word ... ] ] ; ] do list ; done
• for (( expr1 ; expr2 ; expr3 )) ; do list ; done
• select name [ in word ] ; do list ; done
• case word in [ [(] pattern [ | pattern ] ... ) list ;; ] ... esac
• if list; then list; [ elif list; then list; ] ... [ else list; ] fi
• while list-1; do list-2; done
• until list-1; do list-2; done

NB: f() ((expression)) may appears to not create errors but I think it's actually parsing it as f() ( (list) ) which isn't correct either.

[dermoth]

Note that the readCompoundCommand could have been useful. But unfortunately it includes readFunctionDefinition itself.

And it has to, a function can be defined within a function. Can't this be recursive?