Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syntax: redirect operators without spaces can confuse the parser #369

Closed
GuillaumeBibaut opened this issue Mar 15, 2019 · 17 comments
Closed
Milestone

Comments

@GuillaumeBibaut
Copy link

I'm not sure that's a real issue or not since I'm not sure it's correct to write shell script that way, although it works perfectly.

This script can be formatted:

#!/bin/sh

if : ; then
    echo "test if-fi"
    cat <<DONE
aaa
bbb
ccc
DONE
fi

result:

$ cat if-cat-fi.sh | shfmt -i 4 -p
#!/bin/sh

if :; then
    echo "test if-fi"
    cat <<DONE
aaa
bbb
ccc
DONE
fi

but this one cannot:

#!/bin/sh

if : ; then
    echo "test if-fi"
    while read _arg; do
        echo "a: ${_arg}"
    done<<DONE
aaa
bbb
ccc
DONE
fi

result:

$ cat if-while-fi.sh | shfmt -i 4
<standard input>:12:1: "fi" can only be used to end an if

I'm using shfmt on FreeBSD:

$ pkg info shfmt
shfmt-2.6.2
Name           : shfmt
Version        : 2.6.2
Installed on   : Fri Mar 15 11:46:23 2019 CET
Origin         : devel/shfmt
Architecture   : FreeBSD:12:amd64
Prefix         : /usr/local
Categories     : devel
Licenses       : BSD3CLAUSE
Maintainer     : tobik@FreeBSD.org
WWW            : https://github.com/mvdan/sh
Comment        : Format shell scripts
Annotations    :
        FreeBSD_version: 1200086
        repo_type      : binary
        repository     : FreeBSD
Flat size      : 3.49MiB
Description    :
Shfmt is a shell script formatter.  It supports POSIX Shell, Bash,
and mksh.

WWW: https://github.com/mvdan/sh
@GuillaumeBibaut
Copy link
Author

Sorry for nothing, found my problem.

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

Sorry, what was the problem? I started investigating just now, as it turns out.

@GuillaumeBibaut
Copy link
Author

--- if-while-fi-old.sh  2019-03-15 14:58:37.718619000 +0100
+++ if-while-fi.sh      2019-03-15 14:54:07.073654000 +0100
@@ -4,7 +4,7 @@
     echo "test if-fi"
     while read _arg; do
         echo "a: ${_arg}"
-    done<<DONE
+    done <<DONE
 aaa
 bbb
 ccc

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

Thanks. I still think there's a parser bug here; if bash and all other shells accept the original program, but shfmt, doesn't, there's something to fix.

Smaller repro:

if true; then
        cat
fi<<EOF
body
EOF

@mvdan mvdan reopened this Mar 15, 2019
@mvdan mvdan changed the title Redirection operator syntax: redirect operators without spaces can confuse the parser Mar 15, 2019
@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

Even smaller:

if true; then
        cat
fi>/dev/null

@GuillaumeBibaut
Copy link
Author

found another one, not sure it is related, I can open a new issue if you think it's not the same bug:

#!/bin/sh

# incorrect redirection to stderr, has to be at the end of the command
echo 1>&2 "stderr"
# incorrect redirection to stderr, has to be at the end of the command
echo -n 1>&2 "stderr"

result with shfmt -i4 -p:

#!/bin/sh

# incorrect redirection to stderr, has to be at the end of the command
echo 1>&2 "stderr"
# incorrect redirection to stderr, has to be at the end of the command
echo -n "stderr" 1>&2

expected:

#!/bin/sh

# incorrect redirection to stderr, has to be at the end of the command
echo "stderr" 1>&2
# incorrect redirection to stderr, has to be at the end of the command
echo -n "stderr" 1>&2

new bug ?

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

Ah, that's not a bug; redirections are allowed either at the end, or after the first argument of a command. This is because some people prefer the program >file arg1 arg2 ... format.

I can see this is a bit confusing with echo versus echo -n, but the syntax package doesn't know anything about builtins. To it, echo is a command just like foo or bar.

@mvdan mvdan added this to the 3.0 milestone Mar 15, 2019
@mvdan mvdan closed this as completed in 567d7e9 Mar 15, 2019
@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

I've fixed the original bug in the master branch. It's easy to work around, and the canonical format keeps a space anyway, so I'm not going to backport this fix into a stable release for now.

@GuillaumeBibaut
Copy link
Author

GuillaumeBibaut commented Mar 15, 2019

the redirect operator for a standard stream redirection to another stream has to be at the end of a command:

  • echo 1>&2 "stderr" does not redirect to stderr
  • echo "stderr" 1>&2 does redirect to stderr

and it's also true for:

  • program 1>&2 >file arg1 arg2 ...
  • program >file arg1 arg2 ... 1>&2

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

Sorry, I'm not sure I follow. The first example you posted in this last comment does redirect to stderr properly for me on Bash:

$ echo 1>&2 "stderr" | cat >/dev/null
stderr

@GuillaumeBibaut
Copy link
Author

but not in POSIX sh, /bin/sh on Linux is a bash link that's supposed to parse the code POSIXly. But on FreeBSD, /bin/sh is a real POSIX implementation.

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

I've tried dash, busybox ash, mksh, and Bash in POSIX mode, but all give the same output as Bash. Perhaps this is a bug in just some POSIX shells?

If you think this is how POSIX shell should behave, could you quote a document like http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html?

@GuillaumeBibaut
Copy link
Author

#!/bin/sh
set -v

echo 1>&2 "stderr" >stdout 2>stderr
cat stdout
cat stderr

echo "stderr" >stdout 2>stderr 1>&2
cat stdout
cat stderr

This script outputs:

$ sh echo-stderr.sh

echo 1>&2 "stderr" >stdout 2>stderr
cat stdout
stderr
cat stderr

echo "stderr" >stdout 2>stderr 1>&2
cat stdout
cat stderr
stderr

@GuillaumeBibaut
Copy link
Author

FreeBSD man sh(1)

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

Right; it seems like what matters is the order of the redirections. I'm not sure what point you're trying to make; shfmt can move redirections, but it shouldn't change their order.

In other words, can shfmt break a program when formatting it? Or what kind of bug are you trying to raise?

@GuillaumeBibaut
Copy link
Author

What I just know is that this script shell only correctly output to stderr when the 1>&2 redirection is at the end. I was wondering if the first issue with the no space between fi and the redirection could have been the same bug. I can open another issue if needed, I just found it easier to ask you here.

@mvdan
Copy link
Owner

mvdan commented Mar 15, 2019

I don't think it's related to the fixed bug. Feel free to open a new issue if there's something you think shfmt should do differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants