Add a [[ checker
This adds a basic check for [[ when using non-POSIX comparisions such as =~, <, >. This is a fairly common typo often not picked up until runtime. Since it's usually part of an "if [ $foo =~ bar ]; then" statement it doesn't trigger "-e" and the failure just falls through. As mentioned in the code; this is one of those things that starts requiring a complete bash parser to get 100% right. However, I believe this detects the most common failures without setting off false alarms. Change-Id: I5478032800cdf175cb10ce25dc8b6afce6ebd0dd
This commit is contained in:
parent
06dd472f0a
commit
c977b0827d
@ -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
|
||||
~~~~~~~~
|
||||
|
@ -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
|
||||
|
||||
|
@ -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 = {}
|
||||
|
41
bashate/tests/samples/E044_bad.sh
Normal file
41
bashate/tests/samples/E044_bad.sh
Normal file
@ -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"
|
@ -270,6 +270,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
|
||||
|
Loading…
Reference in New Issue
Block a user