diff --git a/bashate/bashate.py b/bashate/bashate.py index 47f0ad2..522f202 100755 --- a/bashate/bashate.py +++ b/bashate/bashate.py @@ -320,7 +320,18 @@ def check_files(self, files, verbose, max_line_length=79): # syntax errors when you try to run them. check_syntax(fname, report) + # Remember the last raw line of this file (and where it + # was) so we can check it for a trailing newline once the + # file is exhausted. "line" itself is unusable for that: + # it gets rewritten by inline-comment stripping and by the + # logical_line loop below. + last_line = None + last_lineno = None + for line in fileinput.input(fname): + last_line = line + last_lineno = fileinput.filelineno() + if fileinput.isfirstline(): check_hashbang(line, fileinput.filename(), report) @@ -408,11 +419,17 @@ def check_files(self, files, verbose, max_line_length=79): check_bare_arithmetic(line, report) check_conditional_expression(line, report) - # finished processing the file - - # last line should always end with a newline - if not line.endswith('\n'): - report.print_error(MESSAGES['E004'].msg, line) + # finished processing the file + + # last line should always end with a newline. Note this + # must be inside the "for fname in files" loop: checking + # it after all files have been processed only ever + # examines whichever file happened to be scanned last. + # Pass filename/filelineno explicitly because fileinput + # has been exhausted by now and no longer reports them. + if last_line is not None and not last_line.endswith('\n'): + report.print_error(MESSAGES['E004'].msg, last_line, + filename=fname, filelineno=last_lineno) def main(args=None): diff --git a/bashate/tests/samples/E004_bad2.sh b/bashate/tests/samples/E004_bad2.sh new file mode 100644 index 0000000..973339e --- /dev/null +++ b/bashate/tests/samples/E004_bad2.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +echo "this file also has no newline at end" \ No newline at end of file diff --git a/bashate/tests/test_bashate.py b/bashate/tests/test_bashate.py index 27e4943..30515fc 100644 --- a/bashate/tests/test_bashate.py +++ b/bashate/tests/test_bashate.py @@ -160,6 +160,36 @@ def test_sample_E004_bad(self): self.run.check_files(test_files, False) self.assert_error_found('E004', 3) + def test_sample_E004_bad_multiple_files(self): + # E004 must be reported for every offending file, not just + # whichever one happens to be scanned last. + test_files = ['bashate/tests/samples/E004_bad.sh', + 'bashate/tests/samples/E004_bad2.sh'] + self.run.check_files(test_files, False) + self.assert_error_found('E004', 3) + self.assertEqual(2, self.run.error_count) + + def test_sample_E004_bad_followed_by_good(self): + # A file missing its trailing newline must still be reported + # when a well-formed file is scanned after it. + test_files = ['bashate/tests/samples/E004_bad.sh', + 'bashate/tests/samples/E003_good.sh'] + self.run.check_files(test_files, False) + self.assert_error_found('E004', 3) + self.assertEqual(1, self.run.error_count) + + def test_sample_E004_good_not_flagged(self): + # A file that does end with a newline must not be flagged, + # even when scanned after one that does not. + test_files = ['bashate/tests/samples/E004_bad.sh', + 'bashate/tests/samples/E003_good.sh'] + self.run.check_files(test_files, False) + for call in self.m_log_error.call_args_list: + args = call[0] + if args[0].startswith('E004'): + self.assertEqual('bashate/tests/samples/E004_bad.sh', + args[2]) + def test_sample_E006_bad(self): test_files = ['bashate/tests/samples/E006_bad.sh'] self.run.check_files(test_files, False) diff --git a/releasenotes/notes/fix-e004-per-file-fc12fe9acd699cba.yaml b/releasenotes/notes/fix-e004-per-file-fc12fe9acd699cba.yaml new file mode 100644 index 0000000..902b1da --- /dev/null +++ b/releasenotes/notes/fix-e004-per-file-fc12fe9acd699cba.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + ``E004`` (file did not end with a newline) is now checked once per + file, rather than once per ``bashate`` invocation. Previously the + check ran after every file had been processed, so only whichever + file happened to be scanned last was examined; a file missing its + trailing newline went unreported whenever another file followed it + on the command line. + + This particularly affected callers that batch files into a single + invocation, such as `pre-commit.com `__, + where the same offending file could pass or fail depending only on + how the file list happened to be ordered.