check-files.py: clean up class structure
Line issue trackers are conceptually a subclass of file issue
trackers: they're file issue trackers where issues arise from checking
each line independently. So make it an actual subclass.
Pylint pointed out the design smell: there was an abstract method that
wasn't always overridden in concrete child classes.
diff --git a/tests/scripts/check-files.py b/tests/scripts/check-files.py
index c11188c..38a6ba5 100755
--- a/tests/scripts/check-files.py
+++ b/tests/scripts/check-files.py
@@ -19,10 +19,12 @@
import sys
-class IssueTracker(object):
- """Base class for issue tracking. Issues should inherit from this and
- overwrite either issue_with_line if they check the file line by line, or
- overwrite check_file_for_issue if they check the file as a whole."""
+class FileIssueTracker(object):
+ """Base class for file-wide issue tracking.
+
+ To implement a checker that processes a file as a whole, inherit from
+ this class and implement `check_file_for_issue`.
+ """
def __init__(self):
self.heading = ""
@@ -35,23 +37,14 @@
return False
return True
- def issue_with_line(self, line):
- raise NotImplementedError
-
def check_file_for_issue(self, filepath):
- with open(filepath, "rb") as f:
- for i, line in enumerate(iter(f.readline, b"")):
- self.check_file_line(filepath, line, i + 1)
+ raise NotImplementedError
def record_issue(self, filepath, line_number):
if filepath not in self.files_with_issues.keys():
self.files_with_issues[filepath] = []
self.files_with_issues[filepath].append(line_number)
- def check_file_line(self, filepath, line, line_number):
- if self.issue_with_line(line):
- self.record_issue(filepath, line_number)
-
def output_file_issues(self, logger):
if self.files_with_issues.values():
logger.info(self.heading)
@@ -64,8 +57,26 @@
logger.info(filename)
logger.info("")
+class LineIssueTracker(FileIssueTracker):
+ """Base class for line-by-line issue tracking.
-class PermissionIssueTracker(IssueTracker):
+ To implement a checker that processes files line by line, inherit from
+ this class and implement `line_with_issue`.
+ """
+
+ def issue_with_line(self, line, filepath):
+ raise NotImplementedError
+
+ def check_file_line(self, filepath, line, line_number):
+ if self.issue_with_line(line, filepath):
+ self.record_issue(filepath, line_number)
+
+ def check_file_for_issue(self, filepath):
+ with open(filepath, "rb") as f:
+ for i, line in enumerate(iter(f.readline, b"")):
+ self.check_file_line(filepath, line, i + 1)
+
+class PermissionIssueTracker(FileIssueTracker):
"""Track files with bad permissions.
Files that are not executable scripts must not be executable."""
@@ -80,7 +91,7 @@
self.files_with_issues[filepath] = None
-class EndOfFileNewlineIssueTracker(IssueTracker):
+class EndOfFileNewlineIssueTracker(FileIssueTracker):
"""Track files that end with an incomplete line
(no newline character at the end of the last line)."""
@@ -94,7 +105,7 @@
self.files_with_issues[filepath] = None
-class Utf8BomIssueTracker(IssueTracker):
+class Utf8BomIssueTracker(FileIssueTracker):
"""Track files that start with a UTF-8 BOM.
Files should be ASCII or UTF-8. Valid UTF-8 does not start with a BOM."""
@@ -108,18 +119,18 @@
self.files_with_issues[filepath] = None
-class LineEndingIssueTracker(IssueTracker):
+class LineEndingIssueTracker(LineIssueTracker):
"""Track files with non-Unix line endings (i.e. files with CR)."""
def __init__(self):
super().__init__()
self.heading = "Non Unix line endings:"
- def issue_with_line(self, line):
+ def issue_with_line(self, line, _filepath):
return b"\r" in line
-class TrailingWhitespaceIssueTracker(IssueTracker):
+class TrailingWhitespaceIssueTracker(LineIssueTracker):
"""Track lines with trailing whitespace."""
def __init__(self):
@@ -127,11 +138,11 @@
self.heading = "Trailing whitespace:"
self.files_exemptions = [".md"]
- def issue_with_line(self, line):
+ def issue_with_line(self, line, _filepath):
return line.rstrip(b"\r\n") != line.rstrip()
-class TabIssueTracker(IssueTracker):
+class TabIssueTracker(LineIssueTracker):
"""Track lines with tabs."""
def __init__(self):
@@ -141,11 +152,11 @@
"Makefile", "generate_visualc_files.pl"
]
- def issue_with_line(self, line):
+ def issue_with_line(self, line, _filepath):
return b"\t" in line
-class MergeArtifactIssueTracker(IssueTracker):
+class MergeArtifactIssueTracker(LineIssueTracker):
"""Track lines with merge artifacts.
These are leftovers from a ``git merge`` that wasn't fully edited."""
@@ -153,22 +164,18 @@
super().__init__()
self.heading = "Merge artifact:"
- def issue_with_line(self, filepath, line):
+ def issue_with_line(self, line, _filepath):
# Detect leftover git conflict markers.
if line.startswith(b'<<<<<<< ') or line.startswith(b'>>>>>>> '):
return True
if line.startswith(b'||||||| '): # from merge.conflictStyle=diff3
return True
if line.rstrip(b'\r\n') == b'=======' and \
- not filepath.endswith('.md'):
+ not _filepath.endswith('.md'):
return True
return False
- def check_file_line(self, filepath, line, line_number):
- if self.issue_with_line(filepath, line):
- self.record_issue(filepath, line_number)
-
-class TodoIssueTracker(IssueTracker):
+class TodoIssueTracker(LineIssueTracker):
"""Track lines containing ``TODO``."""
def __init__(self):
@@ -178,7 +185,7 @@
__file__, "benchmark.c", "pull_request_template.md"
]
- def issue_with_line(self, line):
+ def issue_with_line(self, line, _filepath):
return b"todo" in line.lower()