Add #! or .sh test at warning level
Add a test that the first line of a checked file either starts with #! or has a .sh file extension. This is useful for tools such as file, syntax-highlighting with tools such as gerrit and automatic mode-selection for editors. This is added at a warning level, so as not to be enforcing. To accomodate this each message is given a default-level (all others remain at error). long_msg is extended with some rationale. The output of --show is modified to show this. While we're there, flesh out the long_msg for a few other checks as well. A new option "-e" option is also added that turns warnings into errors, should a project decide they wish to enforce particular warning such as this. Change-Id: I0ed3e1dd17810ff826ee9cf5528d7a0805487624
This commit is contained in:
Ian Wienand
@@ -93,11 +93,19 @@ def check_arithmetic(line, report):
|
||||
report.print_error(MESSAGES['E041'].msg, line)
|
||||
|
||||
|
||||
def check_hashbang(line, filename, report):
|
||||
# this check only runs on the first line
|
||||
# maybe this should check for shell?
|
||||
if not line.startswith("#!") and not filename.endswith(".sh"):
|
||||
report.print_error(MESSAGES['E005'].msg, line)
|
||||
|
||||
|
||||
class BashateRun(object):
|
||||
|
||||
def __init__(self):
|
||||
# TODO(mrodden): rename these to match convention
|
||||
self.ERRORS = 0
|
||||
self.ERRORS_LIST = None
|
||||
self.IGNORE_LIST = None
|
||||
self.WARNINGS = 0
|
||||
self.WARNINGS_LIST = None
|
||||
@@ -110,10 +118,19 @@ class BashateRun(object):
|
||||
if warnings:
|
||||
self.WARNINGS_LIST = '^(' + '|'.join(warnings.split(',')) + ')'
|
||||
|
||||
def register_errors(self, errors):
|
||||
if errors:
|
||||
self.ERRORS_LIST = '^(' + '|'.join(errors.split(',')) + ')'
|
||||
|
||||
def should_ignore(self, error):
|
||||
return self.IGNORE_LIST and re.search(self.IGNORE_LIST, error)
|
||||
|
||||
def should_warn(self, error):
|
||||
# if in the errors list, overrides warning level
|
||||
if self.ERRORS_LIST and re.search(self.ERRORS_LIST, error):
|
||||
return False
|
||||
if messages.is_default_warning(error):
|
||||
return True
|
||||
return self.WARNINGS_LIST and re.search(self.WARNINGS_LIST, error)
|
||||
|
||||
def print_error(self, error, line,
|
||||
@@ -179,6 +196,8 @@ class BashateRun(object):
|
||||
|
||||
prev_file = fileinput.filename()
|
||||
|
||||
check_hashbang(line, fileinput.filename(), report)
|
||||
|
||||
if verbose:
|
||||
print("Running bashate on %s" % fileinput.filename())
|
||||
|
||||
@@ -238,7 +257,9 @@ def main():
|
||||
help='files to scan for errors')
|
||||
parser.add_argument('-i', '--ignore', help='Rules to ignore')
|
||||
parser.add_argument('-w', '--warn',
|
||||
help='Rules to warn (rather than error)')
|
||||
help='Rules to always warn (rather than error)')
|
||||
parser.add_argument('-e', '--error',
|
||||
help='Rules to always error (rather than warn)')
|
||||
parser.add_argument('-v', '--verbose', action='store_true', default=False)
|
||||
parser.add_argument('-s', '--show', action='store_true', default=False)
|
||||
opts = parser.parse_args()
|
||||
@@ -255,6 +276,7 @@ def main():
|
||||
run = BashateRun()
|
||||
run.register_ignores(opts.ignore)
|
||||
run.register_warnings(opts.warn)
|
||||
run.register_errors(opts.error)
|
||||
|
||||
try:
|
||||
run.check_files(files, opts.verbose)
|
||||
|
||||
@@ -10,6 +10,9 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import re
|
||||
import textwrap
|
||||
|
||||
|
||||
class _Message:
|
||||
"""An individual bashate message.
|
||||
@@ -26,10 +29,16 @@ class _Message:
|
||||
:param long_msg: A longer more involved message, designed for
|
||||
documentation
|
||||
"""
|
||||
def __init__(self, msg_id, msg_str, long_msg):
|
||||
def __init__(self, msg_id, msg_str, long_msg, default):
|
||||
self.msg_id = msg_id
|
||||
self.msg_str = msg_str
|
||||
self.long_msg = long_msg
|
||||
# clean-up from """ to a plain string
|
||||
if long_msg:
|
||||
self.long_msg = textwrap.dedent(long_msg)
|
||||
self.long_msg = self.long_msg.strip()
|
||||
else:
|
||||
self.long_msg = None
|
||||
self.default = default
|
||||
|
||||
@property
|
||||
def msg(self):
|
||||
@@ -37,55 +46,141 @@ class _Message:
|
||||
# that up as .msg property for quick access.
|
||||
return "%s: %s" % (self.msg_id, self.msg_str)
|
||||
|
||||
|
||||
_messages = {
|
||||
'E001': {
|
||||
'msg': 'Trailing Whitespace',
|
||||
'long_msg': None
|
||||
'long_msg': None,
|
||||
'default': 'E'
|
||||
},
|
||||
'E002': {
|
||||
'msg': 'Tab indents',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
Spaces are preferred to tabs in source files.
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E003': {
|
||||
'msg': 'Indent not multiple of 4',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
Four spaces should be used to offset logical blocks.
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E004': {
|
||||
'msg': 'File did not end with a newline',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
It is conventional to have a single newline ending files.
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E005': {
|
||||
'msg': 'File does not begin with #! or have .sh prefix',
|
||||
'long_msg':
|
||||
"""
|
||||
This can be useful for tools that use either the interpreter
|
||||
directive or the file-exension to select highlighting mode,
|
||||
syntax mode or determine MIME-type, such as file, gerrit and
|
||||
editors.
|
||||
""",
|
||||
'default': 'W'
|
||||
},
|
||||
'E010': {
|
||||
'msg': 'Do not on same line as %s',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
Ensure consistency of "do" directive being on the same line as
|
||||
it's command. For example:
|
||||
|
||||
for i in $(seq 1 100);
|
||||
do
|
||||
echo "hi"
|
||||
done
|
||||
|
||||
will trigger this error
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E011': {
|
||||
'msg': 'Then keyword is not on same line as if or elif keyword',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
Similar to E010, this ensures consistency of if/elif statements
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E012': {
|
||||
'msg': 'heredoc did not end before EOF',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
This check ensures the closure of heredocs (<<EOF directives).
|
||||
Bash will warn when a heredoc is delimited by end-of-file, but
|
||||
it is easily missed and can cause unexpected issues when a
|
||||
file is sourced.
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E020': {
|
||||
'msg': 'Function declaration not in format ^function name {$',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
There are several equivalent ways to define functions in Bash.
|
||||
This check is for consistency.
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
'E041': {
|
||||
'msg': 'Arithmetic expansion using $[ is deprecated for $((',
|
||||
'long_msg': None
|
||||
'long_msg':
|
||||
"""
|
||||
$[ is deprecated and not explained in the Bash manual. $((
|
||||
should be used for arithmetic.
|
||||
""",
|
||||
'default': 'E'
|
||||
},
|
||||
}
|
||||
|
||||
MESSAGES = {}
|
||||
|
||||
_default_errors = []
|
||||
_default_warnings = []
|
||||
|
||||
for k, v in _messages.items():
|
||||
MESSAGES[k] = _Message(k, v['msg'], v['long_msg'])
|
||||
MESSAGES[k] = _Message(k, v['msg'], v['long_msg'], v['default'])
|
||||
|
||||
if v['default'] == 'E':
|
||||
_default_errors.append(k)
|
||||
if v['default'] == 'W':
|
||||
_default_warnings.append(k)
|
||||
|
||||
# convert this to the regex strings. This looks a bit weird
|
||||
# but it fits the current model of error/warning/ignore checking
|
||||
# easily.
|
||||
_default_errors = '^(' + '|'.join(_default_errors) + ')'
|
||||
_default_warnings = '^(' + '|'.join(_default_warnings) + ')'
|
||||
|
||||
|
||||
def is_default_error(error):
|
||||
return re.search(_default_errors, error)
|
||||
|
||||
|
||||
def is_default_warning(error):
|
||||
return re.search(_default_warnings, error)
|
||||
|
||||
|
||||
def print_messages():
|
||||
|
||||
print("\nAvailable bashate checks")
|
||||
print("------------------------")
|
||||
print("------------------------\n")
|
||||
for k, v in MESSAGES.items():
|
||||
print(" %(id)s : %(string)s" % {
|
||||
print(" [%(default)s] %(id)s : %(string)s" % {
|
||||
'default': v.default,
|
||||
'id': v.msg_id,
|
||||
'string': v.msg_str})
|
||||
print("")
|
||||
if v.long_msg:
|
||||
for l in v.long_msg.split('\n'):
|
||||
print(" %s" % l)
|
||||
print("")
|
||||
|
||||
4
bashate/tests/samples/E005_bad
Normal file
4
bashate/tests/samples/E005_bad
Normal file
@@ -0,0 +1,4 @@
|
||||
# this file doesn't start with #!
|
||||
# and doesn't have a .sh extension
|
||||
|
||||
echo hi
|
||||
@@ -203,6 +203,13 @@ class TestBashateSamples(base.TestCase):
|
||||
|
||||
self.assertEqual(0, self.run.ERRORS)
|
||||
|
||||
def test_sample_E005(self):
|
||||
test_files = ['bashate/tests/samples/E005_bad']
|
||||
self.run.register_errors('E005')
|
||||
self.run.check_files(test_files, False)
|
||||
|
||||
self.assert_error_found('E005', 1)
|
||||
|
||||
def test_sample_warning(self):
|
||||
# reuse a couple of the above files to make sure we turn
|
||||
# errors down to warnings if requested
|
||||
|
||||
Reference in New Issue
Block a user