diff --git a/README.rst b/README.rst index cb5b063..59610c5 100644 --- a/README.rst +++ b/README.rst @@ -51,6 +51,7 @@ not be used - E041: Usage of $[ for arithmetic is deprecated for $(( - E042: local declaration hides errors - E043: arithmetic compound has inconsistent return semantics +- E044: Use [[ for =~,<,> comparisions See also ~~~~~~~~ diff --git a/bashate/bashate.py b/bashate/bashate.py index 6c72a97..bbe45e2 100755 --- a/bashate/bashate.py +++ b/bashate/bashate.py @@ -18,6 +18,7 @@ import argparse import fileinput import os import re +import shlex import subprocess import sys @@ -153,6 +154,45 @@ def check_hashbang(line, filename, report): report.print_error(MESSAGES['E005'].msg, line) +def check_conditional_expression(line, report): + # We're really starting to push the limits of what we can do without + # a complete bash syntax parser here. For example + # > [[ $foo =~ " [ " ]] && [[ $bar =~ " ] " ]] + # would be valid but mess up a simple regex matcher for "[.*]". + # Let alone dealing with multiple-line-spanning etc... + # + # So we'll KISS and just look for simple, one line, + # > if [ $foo =~ "bar" ]; then + # type statements, which are the vast majority of typo errors. + # + # shlex is pretty helpful in getting us something we can walk to + # find this pattern. It does however have issues with + # unterminated quotes on multi-line strings (e.g.) + # + # foo="bar <-- we only see this bit in "line" + # baz" + # + # So we're just going to ignore parser failures here and move on. + # Possibly in the future we could pull such multi-line strings + # into "logical_line" below, and pass that here and have shlex + # break that up. + try: + toks = shlex.shlex(line) + toks.wordchars = "[]=~" + toks = list(toks) + except ValueError: + return + + in_single_bracket = False + for tok in toks: + if tok == '[': + in_single_bracket = True + elif tok in ('=~', '<', '>') and in_single_bracket: + report.print_error(MESSAGES['E044'].msg, line) + elif tok == ']': + in_single_bracket = False + + def check_syntax(filename, report): # run the file through "bash -n" to catch basic syntax errors and # other warnings @@ -364,6 +404,7 @@ class BashateRun(object): check_arithmetic(line, report) check_local_subshell(line, report) check_bare_arithmetic(line, report) + check_conditional_expression(line, report) # finished processing the file diff --git a/bashate/messages.py b/bashate/messages.py index 021b92e..994b761 100644 --- a/bashate/messages.py +++ b/bashate/messages.py @@ -185,8 +185,16 @@ _messages = { """, 'default': 'W', }, - - + 'E044': { + 'msg': 'Use [[ for non-POSIX comparisions', + 'long_msg': + """ + [ is the POSIX test operator, while [[ is the bash keyword + comparision operator. Comparisons such as =~, < and > require + the use of [[. + """, + 'default': 'E', + }, } MESSAGES = {} diff --git a/bashate/tests/samples/E044_bad.sh b/bashate/tests/samples/E044_bad.sh new file mode 100644 index 0000000..4d39cbb --- /dev/null +++ b/bashate/tests/samples/E044_bad.sh @@ -0,0 +1,41 @@ +# =~ + +if [ "test" =~ "]" ]; then + echo "Does not work!" +fi + +[ "test" =~ "string" ] && echo "Does not work!" + +if [[ $foo == bar || "test" =~ "[" ]]; then + echo "Does work!" +fi + +[[ "test" =~ "string" ]] && echo "Does work" + +# < + +if [ 1 < '2' ]; then + echo "Does not work!" +fi + +[ 1 < 2 ] && echo "Does not work!" + +if [[ 1 < 2 ]]; then + echo "Does work!" +fi + +[[ 1 < 2 ]] && echo "Does work" + +# > + +if [ 1 > 2 ]; then + echo "Does not work!" +fi + +[ 1 > 2 ] && echo "Does not work!" + +if [[ 1 > 2 ]]; then + echo "Does work!" +fi + +[[ 1 > 2 ]] && echo "Does work" diff --git a/bashate/tests/test_bashate.py b/bashate/tests/test_bashate.py index 089728f..83a2939 100644 --- a/bashate/tests/test_bashate.py +++ b/bashate/tests/test_bashate.py @@ -271,6 +271,17 @@ class TestBashateSamples(base.TestCase): self.assert_error_found('E040', 7) + def test_sample_E044(self): + test_files = ['bashate/tests/samples/E044_bad.sh'] + self.run.check_files(test_files, False) + + self.assert_error_found('E044', 3) + self.assert_error_found('E044', 7) + self.assert_error_found('E044', 17) + self.assert_error_found('E044', 21) + self.assert_error_found('E044', 31) + self.assert_error_found('E044', 35) + def test_sample_warning(self): # reuse a couple of the above files to make sure we turn # errors down to warnings if requested