Replace heredoc fail-to-end detection E012 with "bash -n"

Currently E012 (detect if heredoc doesn't end) basically doesn't work;
it relies on having multiple files specified to check.  The logic is
currently that if a heredoc is open and we start a new-file, we missed
it.

As we even say in the comments, bash will warn about this.  Luckily,
"bash -n" does pop out this warning when we forget to close a
heredoc.

So modify the existing bash syntax check to catch this too.  Because
it's a warning, bash doesn't exit with !0, so rework things to just
parse output in all cases.  I also found a bit of inconsistency with
output, as described in the comments.

Remove the existing check, and add a testcase.

Change-Id: I292a926d3297a283f74f7d47b7c7aa3c42c9f030

bashate/bashate.py

@@ -114,24 +114,54 @@ def check_hashbang(line, filename, report):
 
 
 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]+): (.*)$')
+    # run the file through "bash -n" to catch basic syntax errors and
+    # other warnings
+    matches = []
+
+    # sample lines we want to match:
+    # foo.sh: line 4: warning: \
+    #    here-document at line 1 delimited by end-of-file (wanted `EOF')
+    # foo.sh: line 9: syntax error: unexpected end of file
+    # foo.sh: line 7: syntax error near unexpected token `}'
+    #
+    # i.e. consistency with ":"'s isn't constant, so just do our
+    # best...
+    r = re.compile(
+        '^(?P<file>.*): line (?P<lineno>[0-9]+): (?P<error>.*)')
+    # we are parsing the error message, so force it to ignore the
+    # system locale so we don't get messages in another language
     bash_environment = os.environ
     bash_environment['LC_ALL'] = 'C'
     proc = subprocess.Popen(
         ['bash', '-n', filename], stdout=subprocess.PIPE,
         stderr=subprocess.PIPE, env=bash_environment)
     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)
+
+    for line in outputs[1].split('\n'):
+        m = r.match(line)
+        if m:
+            matches.append(m)
+
+    for m in matches:
+        if 'syntax error' in m.group('error'):
+            msg = '%s: %s' % (MESSAGES['E040'].msg, m.group('error'))
+            report.print_error(msg, filename=filename,
+                               filelineno=int(m.group('lineno')))
+
+        # Matching output from bash warning about here-documents not
+        # ending.
+        # FIXME: are there other warnings that might come out
+        # with "bash -n"?  A quick scan of the source code suggests
+        # no, but there might be other interesting things we could
+        # catch.
+        if 'warning:' in m.group('error'):
+            if 'delimited by end-of-file' in m.group('error'):
+                start = re.match('^.*line (?P<start>[0-9]+).*$',
+                                 m.group('error'))
+                report.print_error(
+                    MESSAGES['E012'].msg % int(start.group('start')),
+                    filename=filename,
+                    filelineno=int(m.group('lineno')))
 
 
 class BashateRun(object):
@@ -193,8 +223,6 @@ class BashateRun(object):
 
     def check_files(self, files, verbose):
         in_multiline = False
-        multiline_start = 0
-        multiline_line = ""
         logical_line = ""
         token = False
         prev_file = None
@@ -212,15 +240,6 @@ class BashateRun(object):
 
             for line in fileinput.input(fname):
                 if fileinput.isfirstline():
-                    # if in_multiline when the new file starts then we didn't
-                    # find the end of a heredoc in the last file.
-                    if in_multiline:
-                        report.print_error(
-                            MESSAGES['E012'].msg,
-                            multiline_line,
-                            filename=prev_file,
-                            filelineno=multiline_start)
-                        in_multiline = False
 
                     # last line of a previous file should always end with a
                     # newline
@@ -244,8 +263,6 @@ class BashateRun(object):
                     token = starts_multiline(line)
                     if token:
                         in_multiline = True
-                        multiline_start = fileinput.filelineno()
-                        multiline_line = line
                         continue
                 else:
                     logical_line = logical_line + line

bashate/messages.py

@@ -126,7 +126,7 @@ _messages = {
         'default': 'E'
     },
     'E012': {
-        'msg': 'heredoc did not end before EOF',
+        'msg': 'here-document at line %d delimited by end-of-file',
         'long_msg':
         """
         This check ensures the closure of heredocs (<<EOF directives).

bashate/tests/samples/E012_bad.sh

@@ -0,0 +1,9 @@
+#unexpected end-of-file on this
+if foo; then
+
+FOO=<<EOF
+blah foo moo
+
+
+# heredoc not ending
+

bashate/tests/test_bashate.py

@@ -190,6 +190,13 @@ class TestBashateSamples(base.TestCase):
         self.assert_error_found('E011', 3)
         self.assert_error_found('E011', 6)
 
+    def test_sample_E012(self):
+        test_files = ['bashate/tests/samples/E012_bad.sh']
+        self.run.check_files(test_files, False)
+
+        self.assert_error_found('E012', 9)
+        self.assert_error_found('E040', 10)
+
     def test_sample_E041(self):
         test_files = ['bashate/tests/samples/E041_bad.sh']
         self.run.check_files(test_files, False)
@@ -257,8 +264,6 @@ class TestBashateSamples(base.TestCase):
         test_files = ['bashate/tests/samples/legacy_sample.sh']
         self.run.check_files(test_files, False)
 
-        # NOTE(mrodden): E012 actually requires iterating more than one
-        # file to detect at the moment; this is bug
         expected_errors = [
             ('E002', 4),
             ('E003', 6),