We don't need to do anything for [ since it's actually just a command with arguments.

On the other hand though, [[ from Bash is a reserved word and should be parsed as a separate node. We parse it as a command right now, just like [.

It is possible to craft a valid Bash program to cause the parser to break because of this:

[[ foo =~ bar(a) ]]

Would you consider publishing this to the homebrew OSX package manager? I am able to get it running from go source, but I think making a binary available through homebrew would greatly increase the reach of this package.

Aside: I'm also attempting to write an atom text editor plugin to format a shell script through shfmt.

If bash's extended globbing is on, some programs are suddenly valid:

shopt -s extglob

echo foo@(bar)

Normally, this fails as ( is a token:

 $ echo 'echo @(foo)' | bash
bash: line 1: syntax error near unexpected token `('
bash: line 1: `echo @(foo)'
 $ echo 'echo @(foo)' | shfmt
1:7: a command can only contain words and redirects

I see two possible options:

• Change the behaviour of the parser whenever a shopt statement is parsed (ugh)
• Always parse extended globbing stuff in the bash mode

The first option would make the parser even more complex, as it would change behaviour not only depending on the context, but also depending on what statements it has parsed.

The second option is somewhat cleaner, but I see some minor disadvantages:

• Some invalid bash programs will be accepted (which is okay for shfmt)
• If this parser is ever used for anything serious, another package (like an interpreter/executor) would have to take care of erroring if it finds an extended globbing node when not in extglob mode

When running:

shfmt -i 4 integration/assert.sh

On this repo: https://github.com/weaveworks/build-tools/blob/master/integration/assert.sh

It didn't used to happen: only noticed it yesterday, although due to caching the version of shfmt could be a week or so old.

Right now in extended tests we simply read a word after =~. This works most of the time, but it can be broken with some forms of ERE, by using characters that would otherwise be tokens (like () or spaces.

[[ foo =~ bar(a) ]]
[[ foo =~ ba[ r] ]]

 $ echo '<<$(<<EOF)' | shfmt
[proceeds to run until it has done too many recursive calls]

What we do is print the word (unquoted) to get the stop line for the heredoc body.

Interestingly enough, it seems that Bash does something different, as any nested statements are kept intact as they were read:

 $ echo '<<foo"bar"' | bash
bash: line 1: warning: here-document at line 1 delimited by end-of-file (wanted `foobar')
 $ echo '<<$(foo   bar)' | bash
bash: line 1: warning: here-document at line 1 delimited by end-of-file (wanted `$(foo   bar)')

 $ foo=12345; echo ${foo:1-1:(3*1)}
123

Similar to #45. A little trickier as :- and :+ are tokens on inputs like ${foo:-bar}.

foo[bar]=value

Right now we treat foo[bar] as a literal. This is fine for POSIX, but not bash. Besides being wrong, it can also mean the parser mistakingly errors:

 $ echo 'foo[(1)]=bar' | bash
 $ echo 'foo[(1)]=bar' | shfmt
1:1: "foo(" must be followed by )

shfmt incorrectly reformats

git_lines="$( ( echo "$git_data";
    cat "$(dirname "$0")/foo.txt") | sort |uniq)"

to

git_lines="$((
    echo "$git_data"
    cat "$(dirname "$0")/foo.txt"
) | sort | uniq)"

Which breaks; $((stuff)) is arithmetic expansion, but $( (stuff) ) is a nested subshell inside a command substitution.

Should it support multiline arrays? Like this:

var=( 
    1 
    2 
    3
)
echo $var

Right now it's failing:

$ shfmt test.sh
test.sh:3:5: reached literal without matching token ( with )

I'm asking because in the canonical example there is nothing like a multiline array.

By the way, really nice tool!

The following should be allowed:

let() { foo; }

Same goes for the rest of bash-only reserved words: eval, local, declare, etc.

This isn't as simple to fix as it may seem at first sight. It's tricky because some statements like let are followed by arithmetic expressions, not regular words. So we cannot tokenize past the first word to see if a ( follows, because we might start tokenizing the following element as a word instead of as an arithmetic expression.

Right now the code:

[ -z "$HOME" ] && foo || bar
[ -z "$HOME" ] &&
    foo ||
    bar

will be formatted:

[ -z "$HOME" ] && foo || bar
[ -z "$HOME" ] \
    && foo \
    || bar

Can you add an option or something to control the formatting of the line breaks ?

Much like gofmt -s, which removes bits of code that aren't necessary such as repeated types.

Rules that come to mind, which would be useful in the shell:

• Get rid of backticks - `foo` -> $(foo)
• Remove useless word parts - foo""bar -> foobar (applies to "", '', $() and perhaps more)
• Force functions to use blocks - foo() bar -> foo() { bar; } (the first works under some shells)

Some more rules that might be too pedantic/opinionated:

• Force portable functions - function foo() { bar; } -> foo() { bar; }
• Replace ${foo} with $foo where it means the same
• Remove quotes where they're unnecessary - "foo" -> foo

Feedback and ideas welcome @focusaurus @ebfe @fwip @mmlb @legionus @mkovacs @seletskiy

I understand that this doesn't affect the correctness of shfmt, and also that the AST likely wasn't designed to be used outside of this repository. Still, a separate sh-ast package would be super useful.

To reproduce, parse the following snippet, then inspect the AST.

function generate_piece {
  while true; do
    let pos=RANDOM%fields_total
    let board[$pos] || {
      let value=RANDOM%10?2:4
      board[$pos]=$value
      last_added=$pos
      printf "Generated new piece with value $value at position [$pos]\n" >&3
      break;
    }
  done
  let pieces++
}

I think the offending statement is a LetClause of the form:

let word || {
  stmt
}

When recursively walking a directory, we take all files that either have a valid extension (.sh, .bash) or a valid shebang (any combination of env, sh and bash).

Whether or not we restrict to POSIX is bound by the -p flag. shfmt could be smarter and derive the right restriction depending on whether the extension/shebang is bash or shfmt.

I see various options about this detection:

• Have this always on by default.
• Have this enabled by a flag, something like -a. The default would be as now, never restrict to POSIX.

The first option is tricky because bash scripts in general have the .sh extension and a bash shebang. So we would need some extra logic around it.

The second option is a more conservative change, and the behaviour of shfmt will be more clear. At the same time, -a might seem like an unnecessary flag that should always be on.

In any case, -p is here to stay because otherwise there is no way to force POSIX on a file with no extension nor shebang to derive it.

 $ foo=abc; echo ${foo: 1: 3 }
bc
 $ echo '${foo: 1: 3 }' | shfmt
${foo: 1: 3 }

Spaces are ignored; we are not ignoring them at the moment. We should be removing them when formatting.

Seems like most of the format enforced by print/shfmt is widely used and accepted, with the exception of indentation. Most importantly, it seems like spaces are more used in the wild.

For example, take the Google code style for shell: https://google.github.io/styleguide/shell.xml#Indentation

Perhaps the indentation style should be configurable, leaving tabs as the default.

The following script:

#/bin/sh
cat file >&2

Produces this error:

$ shfmt -p cat.sh
cat.sh:2:10: > must be followed by a word

Strangely, in non-posix mode it is fine?

Maybe I have a misconception of what is allowed for redirection in posix, but this (http://pubs.opengroup.org/onlinepubs/007904875/utilities/xcu_chap02.html section 2.7.5), seems to indicate this is valid (and it runs).

Relevant documentation: http://wiki.bash-hackers.org/commands/builtin/let

Here's a program to reproduce:

package main

import (
    "fmt"
    "reflect"
    "strings"

    "github.com/mvdan/sh"
)

func main() {
    text :=
        `let x || {
            y=1
            echo "test"
        }`
    ast, err := sh.Parse(strings.NewReader(text), "test.sh", sh.ParseComments)
    if err != nil {
        fmt.Println("error: ", err)
    }
    fmt.Println(reflect.TypeOf(ast.Stmts[0].Cmd))
}

Not high-priority as this doesn't matter to the printer, hence not marking as a bug.

Still, it is something that needs doing.

According to http://www.tldp.org/LDP/abs/html/functions.html and http://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html#tag_02_09_05 for bash and sh respectively function definitions can be either:

function function_name {
...
}

(bash only)
or

function_name() {
...
}

Note that when the function keyword exists there is no () after the name, but there must be one in sh mode.

[manny@maxwell ~]$ cat t.sh 
((5==5))
[manny@maxwell ~]$ source t.sh; echo $?
0
[manny@maxwell ~]$ cat f.sh 
((5>6))
[manny@maxwell ~]$ source f.sh; echo $?
1

but shfmt is adamant about inserting a space between the leading double parens, probably due to 5979268 which fixed #16

can't tell what this repo does from the README. a visualization would be great?

Right now we just skip the test if -short was given, and we use -short on travis since the bash version there is too old.

Do this properly. We can't have go test breaking just because someone's environment is different.

I commonly do something like:

foo \
bar \
baz \
another \
;

so that I can just add new entry/entries anywhere and then visually select and sort in vim. Unfortunately shfmt drops the final \ and the ; which then messes up my workflow when something new is sorted at the end. This is very likely a niche annoyance so I'd like to know your thoughts.

bisected down to commit d1df3bc

[manny@manny shfmt]$ echo 'echo "$(($(echo 5)+5))"' | bash
10
[manny@manny shfmt]$ git checkout -q d1df3bcdec78e0fee97ffb5d19b008e3ab038d81^
[manny@manny shfmt]$ go build &&  echo 'echo "$(($(echo 5)+5))"' | ./shfmt 
echo "$(($(echo 5) + 5))"
[manny@manny shfmt]$ git checkout -q d1df3bcdec78e0fee97ffb5d19b008e3ab038d81
[manny@manny shfmt]$ go build &&  echo 'echo "$(($(echo 5)+5))"' | ./shfmt 
echo "$( ($(echo 5)+5))"

Quite a few projects out there implement their own shell/bash parsers. Not only the shells themselves, but also some programs that work with shell scripts such as linters.

It's really a little silly for each one of them, especially tooling around shell scripts, to have to implement their own. Sadly, the shells out there don't allow their use as libraries (that I know of).

This is a library, so it does solve that problem. But being written in Go, it's not easily usable from other languages like Python or Shell itself.

Some ideas come to mind:

• Building the packages as shared libraries, linkable by C: http://blog.ralch.com/tutorial/golang-sharing-libraries/
• Adding an executable alongside shfmt that parses an input file and outputs the AST in json or some other easy-to-import format

Both of those could be extended to also include the printer.

Any input will be appreciated, especially from people who would use something like this.

input

 #!/bin/sh

 if    [ "x$foo" == "x"   ]; then
TEST=1
 if    [ "y$bar" == "y"   ]; then
TEST=2
else
  fi
else
  fi

output

#!/bin/sh

if [ "x$foo" == "x" ]; then
    TEST=1
    if [ "y$bar" == "y" ]; then
        TEST=2

    fi

fi

Just a note that I've published a plugin for the atom text editor that will run the current editor text through shfmt. If you feel so inclined to link to it in the README, that would be great.

https://atom.io/packages/format-shell

$(( marks the start of an arithmetic expansion, $( marks the start of a command substitution and ( marks the start of a subshell. Examples:

 $ echo -n 'echo $((1+2))' | bash
3
 $ echo -n 'echo $( (echo foo) | cat)' | bash
foo

This is easy, because we know how we should parse what follows the starting tokens directly. In other words, this is a LL(k) language - we don't have to backtrack.

The fun begins here:

 $ echo -n 'echo $((echo foo) | cat)' | bash
foo

Even though this should be parsed as an arithmetic expansion, it's parsed as a subshell inside a command substitution. Our parser fails at this, because it does no backtracking at all and errors on the invalid arithmetic expression:

 $ echo -n 'echo $((echo foo) | cat)' | shfmt
1:14: not a valid arithmetic operator: echo

This is clearly a bash thing, not a POSIX Shell thing:

 $ echo -n 'echo $((echo foo) | cat)' | dash
dash: 1: Syntax error: Missing '))'
 $ echo -n 'echo $((echo foo) | cat)' | busybox ash
ash: syntax error: missing '))'

Sounds to me like POSIX Shell is indeed LL(k), but that bash is LL(*). In other words, we may have to consume an unknown number of characters before we know what we should do with the following token.

Should we make our parser backtrack?

When run with the following file as input, function declarations after _seq aren't processed at all.
https://github.com/mydzor/bash2048/blob/master/bash2048.sh

First issue is that : is not allowed in the function names, however, it's valid (at least in bash 4+).

Second issue:

tests.sh:164:14: reached < without matching token ${ with }

Right now there is no operator precedence, which works for shfmt but will not for much else.

for fastq in $SAMPLE_NAME*.fastq.gz; do
    zcat $fastq \
        >filtered_`basename $fastq .gz`;
done

becomes

for fastq in $SAMPLE_NAME*.fastq.gz; do
    zcat $fastq \
        `basename $fastq .gz` >filtered_
done

I'm not exactly sure what combination triggers this, but it doesn't seem to happen without output redirection. Using $() syntax also seems to be handled correctly, which surprised me.

shfmt is totally fine with this code:

#!/bin/bash

do_fallthrough() {

        case $1 in
                'foo')
                        echo foo
                        ;;
                'bar')
                        echo bar
                        ;;
                *)
                        echo baz
                        ;;
        esac
}

echo
do_fallthrough foo

echo
do_fallthrough bar

echo
do_fallthrough blah

But with this code:

#!/bin/bash

do_fallthrough() {

        case $1 in
                'foo')
                        echo foo
                        ;;
                'bar')
                        echo bar
                        ;&
                *)
                        echo baz
                        ;;
        esac
}

echo
do_fallthrough foo

echo
do_fallthrough bar

echo
do_fallthrough blah

I get this (I believe) erroneous error:
sample.sh:11:4: ; can only immediately follow a statement

Relevant Documentation:
https://www.gnu.org/software/bash/manual/bashref.html#index-case

Maybe a misunderstanding on my part, but the documentation suggest the Offset field of token.Position is 0 based

type Position struct {
        Offset int // offset, starting at 0

while the following example seems to suggest it's 1 based:

$ cat x.sh
ls
$ cat x.go
package main

import (
    "fmt"
    "io/ioutil"

    "github.com/mvdan/sh/parser"
)

func main() {
    buf, err := ioutil.ReadFile("x.sh")
    if err != nil {
        panic(err)
    }

    ast, err := parser.Parse(buf, "x.sh", parser.ParseComments)
    if err != nil {
        panic(err)
    }

    callexpr := ast.Stmts[0].Cmd

    pos := ast.Position(callexpr.Pos())
    end := ast.Position(callexpr.End())

    fmt.Print(string(buf[pos.Offset:end.Offset]))
}
$ go run x.go
s
$

This is valid, if the array is associative (like a map from string to x).

 $ echo 'declare -A foo; ${foo[some key]}' | bash
 $ echo 'declare -A foo; ${foo[some key]}' | shfmt
1:28: not a valid arithmetic operator: key

Worth noting that we can't know this statically - bash does this at run-time:

 $ echo '${foo[some key]}' | bash
bash: line 1: some key: syntax error in expression (error token is "key")

This is similar to #37. In both cases, the parser should change behaviour depending on what has been run, not simply what has been parsed. So we have two options:

• Not support associative arrays fully (i.e. only say things will work well if literals - names - are used)
• Fall back to accepting the problematic chars to support these associative array keys

The latter will be tricky, because then ${foo[1+2]} will result in the arithmetic expression 1 + 2 even though if it were an associative array it should have been 1+2 as a literal.

It gets even trickier if we start mixing in the printer. In the case above, the printer would probably break programs by introducing or removing spaces.

The first option is the only sane to me. The second will do better in some weird cases, but at the cost of the tool becoming more complex and even unreliable.

$ cat <<EOF
> $(echo foo)
> EOF
foo

Right now we're just reading lines and treaging everything as literals, as if it were in between single quotes. This is more like as if it were in between double quotes.

 $ foo=(1 2 3); echo ${foo[1+1]}
3

Right now we just read a word. This produces the wrong AST and could lead to errors if we see tokens that are otherwise meaningful inside a parameter expansion, like -.

Thankfully, this doesn't seem to be widely used.

input

cat foo.txt | while read line; do
    echo 1
done

output

cat foo.txt | while read line; do
        echo 1
    done

We retry $(( as $( (. Turns out we need to do the same for ((. I can't find this on the newest posix spec, but even dash supports this.

 $ echo '((echo foo); echo bar)' | bash
foo
bar
 $ echo '((echo foo); echo bar)' | shfmt
1:8: not a valid arithmetic operator: foo