Detect setting locals with subshell commands

The return status of "local" is always 0, so something like

 local foo=$(failing command)

doesn't trigger a failure under "set -e".  Missing "failing command"'s
failure has been a source of problems within devstack.

This is added as a warning

Change-Id: Ia0957b47187c3dcadd46154b17022c4213781112
This commit is contained in:
Ian Wienand 2015-10-06 19:50:28 +11:00
parent e04cb43ca5
commit 1d949d6e3e
5 changed files with 40 additions and 3 deletions

View File

@ -40,12 +40,14 @@ unrolling that is kind of "interesting"
- E012: heredoc didn't end before EOF
- E020: Function declaration not in format ``^function name {$``
Obsolete and deprecated syntax
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Obsolete, deprecated or unsafe syntax
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Rules to identify obsolete and deprecated syntax that should not be used
Rules to identify obsolete, deprecated or unsafe syntax that should
not be used
- E041: Usage of $[ for arithmetic is deprecated for $((
- E042: local declaration hides errors
See also
~~~~~~~~

View File

@ -98,6 +98,12 @@ def check_arithmetic(line, report):
report.print_error(MESSAGES['E041'].msg, line)
def check_local_subshell(line, report):
if line.lstrip().startswith('local ') and \
("=$(" in line or "=`" in line):
report.print_error(MESSAGES['E042'].msg, line)
def check_hashbang(line, filename, report):
# this check only runs on the first line
# maybe this should check for shell?
@ -249,6 +255,7 @@ class BashateRun(object):
check_if_then(logical_line, report)
check_function_decl(logical_line, report)
check_arithmetic(logical_line, report)
check_local_subshell(logical_line, report)
prev_line = logical_line
prev_lineno = fileinput.filelineno()

View File

@ -154,6 +154,16 @@ _messages = {
""",
'default': 'E'
},
'E042': {
'msg': 'local declaration hides errors',
'long_msg':
"""
The return value of "local" is always 0; errors in subshells
used for declaration are thus hidden and will not trigger "set -e".
""",
'default': 'W',
},
}
MESSAGES = {}

View File

@ -0,0 +1,10 @@
function hello {
local foo=$(ls)
}
function hello_too {
local foo=`ls`
}
hello
hello_too

View File

@ -196,6 +196,14 @@ class TestBashateSamples(base.TestCase):
self.assert_error_found('E041', 4)
def test_sample_E042(self):
test_files = ['bashate/tests/samples/E042_bad.sh']
self.run.register_errors('E042')
self.run.check_files(test_files, False)
self.assert_error_found('E042', 2)
self.assert_error_found('E042', 6)
def test_sample_for_loops(self):
test_files = ['bashate/tests/samples/for_loops.sh']
self.run.check_files(test_files, False)