Detect arithmetic compound

As described in comments, ((expr)) has a trap that if expr is 0, the
return value is 1.  This will trigger a failure with "set -e" and can
be very confusing.  It is good defensive programming to avoid this
with explicit assignment.

Change-Id: Id57df3ff8127601429426c177a10e8eaec30c504

README.rst

@@ -49,6 +49,7 @@ not be used
 - E040: Syntax errors reported by `bash -n`
 - E041: Usage of $[ for arithmetic is deprecated for $((
 - E042: local declaration hides errors
+- E043: arithmetic compound has inconsistent return semantics
 
 See also
 ~~~~~~~~

bashate/bashate.py

@@ -132,6 +132,11 @@ def check_arithmetic(line, report):
         report.print_error(MESSAGES['E041'].msg, line)
 
 
+def check_bare_arithmetic(line, report):
+    if line.lstrip().startswith("(("):
+        report.print_error(MESSAGES['E043'].msg, line)
+
+
 def check_local_subshell(line, report):
     # XXX: should we increase the string checking to see if the $( is
     # anywhere with a string being set?  Risk of false positives?x
@@ -372,6 +377,7 @@ class BashateRun(object):
                     check_function_decl(line, report)
                     check_arithmetic(line, report)
                     check_local_subshell(line, report)
+                    check_bare_arithmetic(line, report)
 
                 prev_line = line
                 prev_lineno = fileinput.filelineno()

bashate/messages.py

@@ -172,6 +172,20 @@ _messages = {
         """,
         'default': 'W',
     },
+    'E043': {
+        'msg': 'Arithmetic compound has inconsistent return semantics',
+        'long_msg':
+        """
+        The return value of ((expr)) is 1 if "expr" evalues to zero,
+        otherwise 0.  Combined with "set -e", this can be quite
+        confusing when something like ((counter++)) evaluates to zero,
+        making the arithmetic evaluation return 1 and triggering the
+        an error failure.  It is therefore best to use assignment with
+        the $(( operator.
+        """,
+        'default': 'W',
+    },
+
 
 }
 

bashate/tests/samples/E043_bad.sh

@@ -0,0 +1,10 @@
+count=0
+things="0 1 0 0 1"
+
+for i in $things; do
+    if [ $i == "1" ]; then
+        (( count++ ))
+    fi
+done
+
+echo "Count is ${count}"

bashate/tests/test_bashate.py

@@ -225,6 +225,13 @@ class TestBashateSamples(base.TestCase):
         self.assert_error_found('E042', 10)
         self.assert_error_found('E042', 11)
 
+    def test_sample_E043(self):
+        test_files = ['bashate/tests/samples/E043_bad.sh']
+        self.run.register_errors('E043')
+        self.run.check_files(test_files, False)
+
+        self.assert_error_found('E043', 6)
+
     def test_sample_for_loops(self):
         test_files = ['bashate/tests/samples/for_loops.sh']
         self.run.check_files(test_files, False)