Browse code

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

Ian Wienand authored on 2014/02/21 14:14:29
Showing 3 changed files
... ...
@@ -37,4 +37,4 @@ function neutron_plugin_configure_service() {
37 37
 }
38 38
 
39 39
 # Restore xtrace
40
-$MY_XTRACE
41 40
\ No newline at end of file
41
+$MY_XTRACE
... ...
@@ -58,40 +58,40 @@ EOF
58 58
 
59 59
     if is_fedora || is_suse; then
60 60
         if is_fedora && [[ $DISTRO =~ (rhel6) || "$os_RELEASE" -le "17" ]]; then
61
-            sudo bash -c "cat <<EOF >/etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
61
+            cat <<EOF | sudo tee /etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
62 62
 [libvirt Management Access]
63 63
 Identity=unix-group:$LIBVIRT_GROUP
64 64
 Action=org.libvirt.unix.manage
65 65
 ResultAny=yes
66 66
 ResultInactive=yes
67 67
 ResultActive=yes
68
-EOF"
68
+EOF
69 69
         elif is_suse && [[ $os_RELEASE = 12.2 || "$os_VENDOR" = "SUSE LINUX" ]]; then
70 70
             # openSUSE < 12.3 or SLE
71 71
             # Work around the fact that polkit-default-privs overrules pklas
72 72
             # with 'unix-group:$group'.
73
-            sudo bash -c "cat <<EOF >/etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
73
+            cat <<EOF | sudo tee /etc/polkit-1/localauthority/50-local.d/50-libvirt-remote-access.pkla
74 74
 [libvirt Management Access]
75 75
 Identity=unix-user:$STACK_USER
76 76
 Action=org.libvirt.unix.manage
77 77
 ResultAny=yes
78 78
 ResultInactive=yes
79 79
 ResultActive=yes
80
-EOF"
80
+EOF
81 81
         else
82 82
             # Starting with fedora 18 and opensuse-12.3 enable stack-user to
83 83
             # virsh -c qemu:///system by creating a policy-kit rule for
84 84
             # stack-user using the new Javascript syntax
85 85
             rules_dir=/etc/polkit-1/rules.d
86 86
             sudo mkdir -p $rules_dir
87
-            sudo bash -c "cat <<EOF > $rules_dir/50-libvirt-$STACK_USER.rules
87
+            cat <<EOF | sudo tee $rules_dir/50-libvirt-$STACK_USER.rules
88 88
 polkit.addRule(function(action, subject) {
89 89
     if (action.id == 'org.libvirt.unix.manage' &&
90 90
         subject.user == '"$STACK_USER"') {
91 91
         return polkit.Result.YES;
92 92
     }
93 93
 });
94
-EOF"
94
+EOF
95 95
             unset rules_dir
96 96
         fi
97 97
     fi
... ...
@@ -25,6 +25,7 @@
25 25
 # - E001: check that lines do not end with trailing whitespace
26 26
 # - E002: ensure that indents are only spaces, and not hard tabs
27 27
 # - E003: ensure all indents are a multiple of 4 spaces
28
+# - E004: file did not end with a newline
28 29
 #
29 30
 # Structure errors
30 31
 #
... ...
@@ -34,6 +35,7 @@
34 34
 #
35 35
 # - E010: *do* not on the same line as *for*
36 36
 # - E011: *then* not on the same line as *if*
37
+# - E012: heredoc didn't end before EOF
37 38
 
38 39
 import argparse
39 40
 import fileinput
... ...
@@ -54,11 +56,16 @@ def should_ignore(error):
54 54
     return IGNORE and re.search(IGNORE, error)
55 55
 
56 56
 
57
-def print_error(error, line):
57
+def print_error(error, line,
58
+                filename=None, filelineno=None):
59
+    if not filename:
60
+        filename = fileinput.filename()
61
+    if not filelineno:
62
+        filelineno = fileinput.filelineno()
58 63
     global ERRORS
59 64
     ERRORS = ERRORS + 1
60 65
     print("%s: '%s'" % (error, line.rstrip('\n')))
61
-    print(" - %s: L%s" % (fileinput.filename(), fileinput.filelineno()))
66
+    print(" - %s: L%s" % (filename, filelineno))
62 67
 
63 68
 
64 69
 def not_continuation(line):
... ...
@@ -112,17 +119,44 @@ def end_of_multiline(line, token):
112 112
 
113 113
 def check_files(files, verbose):
114 114
     in_multiline = False
115
+    multiline_start = 0
116
+    multiline_line = ""
115 117
     logical_line = ""
116 118
     token = False
119
+    prev_file = None
120
+    prev_line = ""
121
+    prev_lineno = 0
122
+
117 123
     for line in fileinput.input(files):
118
-        if verbose and fileinput.isfirstline():
119
-            print "Running bash8 on %s" % fileinput.filename()
124
+        if fileinput.isfirstline():
125
+            # if in_multiline when the new file starts then we didn't
126
+            # find the end of a heredoc in the last file.
127
+            if in_multiline:
128
+                print_error('E012: heredoc did not end before EOF',
129
+                            multiline_line,
130
+                            filename=prev_file, filelineno=multiline_start)
131
+                in_multiline = False
132
+
133
+            # last line of a previous file should always end with a
134
+            # newline
135
+            if prev_file and not prev_line.endswith('\n'):
136
+                print_error('E004: file did not end with a newline',
137
+                            prev_line,
138
+                            filename=prev_file, filelineno=prev_lineno)
139
+
140
+            prev_file = fileinput.filename()
141
+
142
+            if verbose:
143
+                print "Running bash8 on %s" % fileinput.filename()
144
+
120 145
         # NOTE(sdague): multiline processing of heredocs is interesting
121 146
         if not in_multiline:
122 147
             logical_line = line
123 148
             token = starts_multiline(line)
124 149
             if token:
125 150
                 in_multiline = True
151
+                multiline_start = fileinput.filelineno()
152
+                multiline_line = line
126 153
                 continue
127 154
         else:
128 155
             logical_line = logical_line + line
... ...
@@ -136,6 +170,8 @@ def check_files(files, verbose):
136 136
         check_for_do(logical_line)
137 137
         check_if_then(logical_line)
138 138
 
139
+        prev_line = logical_line
140
+        prev_lineno = fileinput.filelineno()
139 141
 
140 142
 def get_options():
141 143
     parser = argparse.ArgumentParser(