From 77da51f026800a9ce7ec7dc6f929d4b60392083c Mon Sep 17 00:00:00 2001
From: Nikki Heald <nicky@notnowlewis.com>
Date: Tue, 6 Oct 2015 17:24:28 +0100
Subject: [PATCH] Add shallow syntax checking

`bash -n` will do a shallow syntax check on a bash script,
just checking that the script parses. It's possible to have
scripts that pass all bashate style checks, but don't actually
run because of a syntax error, so this adds error code E040
and reports the syntax errors that `bash -n` outputs.

Change-Id: Ib128c54493221e71cca19a497a6efc4f5fc368c1
---
 README.rst                                 |  1 +
 bashate/bashate.py                         | 26 +++++++++++++++++++++-
 bashate/messages.py                        |  9 ++++++++
 bashate/tests/samples/E040_syntax_error.sh |  7 ++++++
 bashate/tests/test_bashate.py              |  7 ++++++
 5 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 bashate/tests/samples/E040_syntax_error.sh

diff --git a/README.rst b/README.rst
index 6361575..877c7db 100644
--- a/README.rst
+++ b/README.rst
@@ -46,6 +46,7 @@ Obsolete, deprecated or unsafe syntax
 Rules to identify obsolete, deprecated or unsafe syntax that should
 not be used
 
+- E040: Syntax errors reported by `bash -n`
 - E041: Usage of $[ for arithmetic is deprecated for $((
 - E042: local declaration hides errors
 
diff --git a/bashate/bashate.py b/bashate/bashate.py
index 2963f61..3c74ed9 100644
--- a/bashate/bashate.py
+++ b/bashate/bashate.py
@@ -17,6 +17,7 @@ from __future__ import absolute_import
 import argparse
 import fileinput
 import re
+import subprocess
 import sys
 
 from bashate import messages
@@ -111,6 +112,25 @@ def check_hashbang(line, filename, report):
         report.print_error(MESSAGES['E005'].msg, line)
 
 
+def check_syntax(filename, report):
+    # get bash to check the syntax, parse the output for line numbers
+    # and syntax errors to send to the report.
+    syntax_pattern = re.compile('^.*?: line ([0-9]+): (.*)$')
+    proc = subprocess.Popen(
+        ['bash', '-n', filename], stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE)
+    outputs = proc.communicate()
+    if proc.returncode != 0:
+        syntax_errors = [
+            line for line in outputs[1].split('\n') if 'syntax error' in line]
+        for line in syntax_errors:
+            groups = syntax_pattern.match(line).groups()
+            error_message = groups[1]
+            lineno = int(groups[0])
+            msg = '%s: %s' % (MESSAGES['E040'].msg, error_message)
+            report.print_error(msg, filename=filename, filelineno=lineno)
+
+
 class BashateRun(object):
 
     def __init__(self):
@@ -143,7 +163,7 @@ class BashateRun(object):
             return True
         return self.warning_list and re.search(self.warning_list, error)
 
-    def print_error(self, error, line,
+    def print_error(self, error, line='',
                     filename=None, filelineno=None):
         if self.should_ignore(error):
             return
@@ -183,6 +203,10 @@ class BashateRun(object):
         report = self
 
         for fname in files:
+            # simple syntax checking, as files can pass style but still cause
+            # syntax errors when you try to run them.
+            check_syntax(fname, report)
+
             for line in fileinput.input(fname):
                 if fileinput.isfirstline():
                     # if in_multiline when the new file starts then we didn't
diff --git a/bashate/messages.py b/bashate/messages.py
index 26bfadf..a9c9f65 100644
--- a/bashate/messages.py
+++ b/bashate/messages.py
@@ -145,6 +145,15 @@ _messages = {
         """,
         'default': 'E'
     },
+    'E040': {
+        'msg': 'Syntax error',
+        'long_msg':
+        """
+        `bash -n` determined that there was a syntax error preventing
+        the script from parsing correctly and running.
+        """,
+        'default': 'E'
+    },
     'E041': {
         'msg': 'Arithmetic expansion using $[ is deprecated for $((',
         'long_msg':
diff --git a/bashate/tests/samples/E040_syntax_error.sh b/bashate/tests/samples/E040_syntax_error.sh
new file mode 100644
index 0000000..bb9c21a
--- /dev/null
+++ b/bashate/tests/samples/E040_syntax_error.sh
@@ -0,0 +1,7 @@
+#!/bin/bash
+
+function myfn {
+    if [ '' == '' ]; then
+        echo 'Things'
+    fii
+}
diff --git a/bashate/tests/test_bashate.py b/bashate/tests/test_bashate.py
index 5784914..96c38aa 100644
--- a/bashate/tests/test_bashate.py
+++ b/bashate/tests/test_bashate.py
@@ -224,6 +224,13 @@ class TestBashateSamples(base.TestCase):
 
         self.assert_error_found('E005', 1)
 
+    def test_sample_E040(self):
+        test_files = ['bashate/tests/samples/E040_syntax_error.sh']
+        self.run.register_errors('E040')
+        self.run.check_files(test_files, False)
+
+        self.assert_error_found('E040', 7)
+
     def test_sample_warning(self):
         # reuse a couple of the above files to make sure we turn
         # errors down to warnings if requested