From b8e250232ec55b946d2fd7e4237f12632408bdcc Mon Sep 17 00:00:00 2001
From: Ian Wienand <iwienand@redhat.com>
Date: Fri, 21 Feb 2014 16:14:29 +1100
Subject: [PATCH] Add end-of-file checks to bash8

Add two end-of-file checks to bash8.  Firstly, alert if heredoc hasn't
finished.  Some heredocs were done like:

---
sudo bash -c "cat <<EOF > foo
...
EOF"
---

(A better way to do this is "cat <<EOF | sudo tee ..." as it retains
the usual heredoc layout in the code).

The trailing quote was throwing the matching in bash8 off and it kept
appending the next file as if it was still part of the heredoc.  To
avoid this, we check if we're still in a heredoc when we start a new
file; if so raise an error and reset the heredoc status fresh.  We
track the state of the previous file, line and lineno so we can give a
good error.

---
E012: heredoc did not end before EOF: 'cat <<EOF'
 - lib/trove: L221
---

This includes fixes for the existing problem heredocs.

A similar EOF check is to ensure the previous file ended with a
newline.

---
E004: file did not end with a newline: '$MY_XTRACE'
 - lib/neutron_plugins/embrane: L40
---

This requires only one fix

Change-Id: I5e547d87b3921fc7ce6588c28f074e5c9f489c1f
---
 lib/neutron_plugins/embrane         |  2 +-
 lib/nova_plugins/hypervisor-libvirt | 12 ++++----
 tools/bash8.py                      | 44 ++++++++++++++++++++++++++---
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/lib/neutron_plugins/embrane b/lib/neutron_plugins/embrane
index 4206a2053c..325e9397e6 100644
--- a/lib/neutron_plugins/embrane
+++ b/lib/neutron_plugins/embrane
@@ -37,4 +37,4 @@ function neutron_plugin_configure_service() {
 }
 
 # Restore xtrace
-$MY_XTRACE
\ No newline at end of file
+$MY_XTRACE
diff --git a/lib/nova_plugins/hypervisor-libvirt b/lib/nova_plugins/hypervisor-libvirt
index 415244ffae..a0552b47f3 100644
--- a/lib/nova_plugins/hypervisor-libvirt
+++ b/lib/nova_plugins/hypervisor-libvirt
@@ -58,40 +58,40 @@ EOF
 
     if is_fedora || is_suse; then
         if is_fedora && [[ $DISTRO =~ (rhel6) || "$os_RELEASE" -le "17" ]]; then
-            sudo bash -c "cat <<EOF >/etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
+            cat <<EOF | sudo tee /etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
 [libvirt Management Access]
 Identity=unix-group:$LIBVIRT_GROUP
 Action=org.libvirt.unix.manage
 ResultAny=yes
 ResultInactive=yes
 ResultActive=yes
-EOF"
+EOF
         elif is_suse && [[ $os_RELEASE = 12.2 || "$os_VENDOR" = "SUSE LINUX" ]]; then
             # openSUSE < 12.3 or SLE
             # Work around the fact that polkit-default-privs overrules pklas
             # with 'unix-group:$group'.
-            sudo bash -c "cat <<EOF >/etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
+            cat <<EOF | sudo tee /etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
 [libvirt Management Access]
 Identity=unix-user:$STACK_USER
 Action=org.libvirt.unix.manage
 ResultAny=yes
 ResultInactive=yes
 ResultActive=yes
-EOF"
+EOF
         else
             # Starting with fedora 18 and opensuse-12.3 enable stack-user to
             # virsh -c qemu:///system by creating a policy-kit rule for
             # stack-user using the new Javascript syntax
             rules_dir=/etc/polkit-1/rules.d
             sudo mkdir -p $rules_dir
-            sudo bash -c "cat <<EOF > $rules_dir/50-libvirt-$STACK_USER.rules
+            cat <<EOF | sudo tee $rules_dir/50-libvirt-$STACK_USER.rules
 polkit.addRule(function(action, subject) {
     if (action.id == 'org.libvirt.unix.manage' &&
         subject.user == '"$STACK_USER"') {
         return polkit.Result.YES;
     }
 });
-EOF"
+EOF
             unset rules_dir
         fi
     fi
diff --git a/tools/bash8.py b/tools/bash8.py
index ca0abd964a..f89b24169c 100755
--- a/tools/bash8.py
+++ b/tools/bash8.py
@@ -25,6 +25,7 @@
 # - E001: check that lines do not end with trailing whitespace
 # - E002: ensure that indents are only spaces, and not hard tabs
 # - E003: ensure all indents are a multiple of 4 spaces
+# - E004: file did not end with a newline
 #
 # Structure errors
 #
@@ -34,6 +35,7 @@
 #
 # - E010: *do* not on the same line as *for*
 # - E011: *then* not on the same line as *if*
+# - E012: heredoc didn't end before EOF
 
 import argparse
 import fileinput
@@ -54,11 +56,16 @@ def should_ignore(error):
     return IGNORE and re.search(IGNORE, error)
 
 
-def print_error(error, line):
+def print_error(error, line,
+                filename=None, filelineno=None):
+    if not filename:
+        filename = fileinput.filename()
+    if not filelineno:
+        filelineno = fileinput.filelineno()
     global ERRORS
     ERRORS = ERRORS + 1
     print("%s: '%s'" % (error, line.rstrip('\n')))
-    print(" - %s: L%s" % (fileinput.filename(), fileinput.filelineno()))
+    print(" - %s: L%s" % (filename, filelineno))
 
 
 def not_continuation(line):
@@ -112,17 +119,44 @@ def end_of_multiline(line, token):
 
 def check_files(files, verbose):
     in_multiline = False
+    multiline_start = 0
+    multiline_line = ""
     logical_line = ""
     token = False
+    prev_file = None
+    prev_line = ""
+    prev_lineno = 0
+
     for line in fileinput.input(files):
-        if verbose and fileinput.isfirstline():
-            print "Running bash8 on %s" % fileinput.filename()
+        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:
+                print_error('E012: heredoc did not end before EOF',
+                            multiline_line,
+                            filename=prev_file, filelineno=multiline_start)
+                in_multiline = False
+
+            # last line of a previous file should always end with a
+            # newline
+            if prev_file and not prev_line.endswith('\n'):
+                print_error('E004: file did not end with a newline',
+                            prev_line,
+                            filename=prev_file, filelineno=prev_lineno)
+
+            prev_file = fileinput.filename()
+
+            if verbose:
+                print "Running bash8 on %s" % fileinput.filename()
+
         # NOTE(sdague): multiline processing of heredocs is interesting
         if not in_multiline:
             logical_line = line
             token = starts_multiline(line)
             if token:
                 in_multiline = True
+                multiline_start = fileinput.filelineno()
+                multiline_line = line
                 continue
         else:
             logical_line = logical_line + line
@@ -136,6 +170,8 @@ def check_files(files, verbose):
         check_for_do(logical_line)
         check_if_then(logical_line)
 
+        prev_line = logical_line
+        prev_lineno = fileinput.filelineno()
 
 def get_options():
     parser = argparse.ArgumentParser(