diff options
Diffstat (limited to 'utils/checkstyle.py')
| -rwxr-xr-x | utils/checkstyle.py | 884 |
1 files changed, 606 insertions, 278 deletions
diff --git a/utils/checkstyle.py b/utils/checkstyle.py index b594a19a..f6229bbd 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -1,10 +1,10 @@ -#!/usr/bin/python3 +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (C) 2018, Google Inc. # # Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # -# checkstyle.py - A patch style checker script based on astyle or clang-format +# A patch style checker script based on clang-format # # TODO: # @@ -22,22 +22,7 @@ import shutil import subprocess import sys -astyle_options = ( - '-n', - '--style=linux', - '--indent=force-tab=8', - '--attach-namespaces', - '--attach-extern-c', - '--pad-oper', - '--align-pointer=name', - '--align-reference=name', - '--keep-one-line-blocks', - '--max-code-length=120' -) - dependencies = { - 'astyle': False, - 'clang-format': False, 'git': True, } @@ -182,6 +167,12 @@ def parse_diff(diff): hunk = DiffHunk(line) elif hunk is not None: + # Work around https://github.com/python/cpython/issues/46395 + # See https://www.gnu.org/software/diffutils/manual/html_node/Incomplete-Lines.html + if line[-1] != '\n': + hunk.append(line + '\n') + line = '\\ No newline at end of file\n' + hunk.append(line) if hunk: @@ -191,73 +182,490 @@ def parse_diff(diff): # ------------------------------------------------------------------------------ -# Style Checkers +# Commit, Staged Changes & Amendments # -_style_checkers = [] +class CommitFile: + def __init__(self, name): + info = name.split() + self.__status = info[0][0] -class StyleCheckerRegistry(type): - def __new__(cls, clsname, bases, attrs): - newclass = super(StyleCheckerRegistry, cls).__new__(cls, clsname, bases, attrs) - if clsname != 'StyleChecker': - _style_checkers.append(newclass) - return newclass + # For renamed files, store the new name + if self.__status == 'R': + self.__filename = info[2] + else: + self.__filename = info[1] + + def __repr__(self): + return f'{self.__status} {self.__filename}' + + @property + def filename(self): + return self.__filename + + @property + def status(self): + return self.__status -class StyleChecker(metaclass=StyleCheckerRegistry): +class Commit: + def __init__(self, commit): + self._commit = commit + self._author = None + self._trailers = [] + self._parse() + + def _parse_commit(self): + # Get and parse the commit message. + ret = subprocess.run(['git', 'show', '--format=%H%n%an <%ae>%n%s%n%b', + '--no-patch', self.commit], + stdout=subprocess.PIPE).stdout.decode('utf-8') + lines = ret.splitlines() + + self._commit = lines[0] + self._author = lines[1] + self._title = lines[2] + self._body = lines[3:] + + # Parse the trailers. Feed git-interpret-trailers with a full commit + # message that includes both the title and the body, as it otherwise + # fails to find trailers when the body contains trailers only. + message = self._title + '\n\n' + '\n'.join(self._body) + trailers = subprocess.run(['git', 'interpret-trailers', '--parse'], + input=message.encode('utf-8'), + stdout=subprocess.PIPE).stdout.decode('utf-8') + + self._trailers = trailers.splitlines() + + def _parse(self): + self._parse_commit() + + # Get the list of files. Use an empty format specifier to suppress the + # commit message completely. + ret = subprocess.run(['git', 'show', '--format=', '--name-status', + self.commit], + stdout=subprocess.PIPE).stdout.decode('utf-8') + self._files = [CommitFile(f) for f in ret.splitlines()] + + def __repr__(self): + return '\n'.join([ + f'commit {self.commit}', + f'Author: {self.author}', + f'', + f' {self.title}', + '', + '\n'.join([line and f' {line}' or '' for line in self._body]), + 'Trailers:', + ] + self.trailers) + + def files(self, filter='AMR'): + return [f.filename for f in self._files if f.status in filter] + + @property + def author(self): + return self._author + + @property + def commit(self): + return self._commit + + @property + def title(self): + return self._title + + @property + def trailers(self): + return self._trailers + + def get_diff(self, top_level, filename): + diff = subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit), + '--', '%s/%s' % (top_level, filename)], + stdout=subprocess.PIPE).stdout.decode('utf-8') + return parse_diff(diff.splitlines(True)) + + def get_file(self, filename): + return subprocess.run(['git', 'show', '%s:%s' % (self.commit, filename)], + stdout=subprocess.PIPE).stdout.decode('utf-8') + + +class StagedChanges(Commit): def __init__(self): - pass + Commit.__init__(self, '') + + def _parse(self): + ret = subprocess.run(['git', 'diff', '--staged', '--name-status'], + stdout=subprocess.PIPE).stdout.decode('utf-8') + self._title = 'Staged changes' + self._files = [CommitFile(f) for f in ret.splitlines()] - # - # Class methods - # + def get_diff(self, top_level, filename): + diff = subprocess.run(['git', 'diff', '--staged', '--', + '%s/%s' % (top_level, filename)], + stdout=subprocess.PIPE).stdout.decode('utf-8') + return parse_diff(diff.splitlines(True)) + + +class Amendment(Commit): + def __init__(self): + Commit.__init__(self, 'HEAD') + + def _parse(self): + self._parse_commit() + + self._title = f'Amendment of "{self.title}"' + + # Extract the list of modified files. + ret = subprocess.run(['git', 'diff', '--staged', '--name-status', 'HEAD~'], + stdout=subprocess.PIPE).stdout.decode('utf-8') + self._files = [CommitFile(f) for f in ret.splitlines()] + + def get_diff(self, top_level, filename): + diff = subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--', + '%s/%s' % (top_level, filename)], + stdout=subprocess.PIPE).stdout.decode('utf-8') + return parse_diff(diff.splitlines(True)) + + +# ------------------------------------------------------------------------------ +# Helpers +# + +class ClassRegistry(type): + def __new__(cls, clsname, bases, attrs): + newclass = super().__new__(cls, clsname, bases, attrs) + if bases and bases[0] != CheckerBase: + base = bases[0] + + if not hasattr(base, 'subclasses'): + base.subclasses = [] + base.subclasses.append(newclass) + base.subclasses.sort(key=lambda x: getattr(x, 'priority', 0), + reverse=True) + return newclass + + +class CheckerBase(metaclass=ClassRegistry): @classmethod - def checkers(cls, filename): - for checker in _style_checkers: - if checker.supports(filename): - yield checker + def instances(cls, obj, names): + for instance in cls.subclasses: + if names and instance.__name__ not in names: + continue + if instance.supports(obj): + yield instance @classmethod - def supports(cls, filename): - for pattern in cls.patterns: - if fnmatch.fnmatch(os.path.basename(filename), pattern): - return True + def supports(cls, obj): + if hasattr(cls, 'commit_types'): + return type(obj) in cls.commit_types + + if hasattr(cls, 'patterns'): + for pattern in cls.patterns: + if fnmatch.fnmatch(os.path.basename(obj), pattern): + return True + return False @classmethod def all_patterns(cls): patterns = set() - for checker in _style_checkers: - patterns.update(checker.patterns) + for instance in cls.subclasses: + if hasattr(instance, 'patterns'): + patterns.update(instance.patterns) return patterns + @classmethod + def check_dependencies(cls): + if not hasattr(cls, 'dependencies'): + return [] + + issues = [] + + for command in cls.dependencies: + if command not in dependencies: + dependencies[command] = shutil.which(command) + + if not dependencies[command]: + issues.append(CommitIssue(f'Missing {command} to run {cls.__name__}')) + + return issues + + +# ------------------------------------------------------------------------------ +# Commit Checkers +# + +class CommitChecker(CheckerBase): + pass + + +class CommitIssue(object): + def __init__(self, msg): + self.msg = msg + + def __str__(self): + return f'{Colours.fg(Colours.Yellow)}{self.msg}{Colours.reset()}' + + +class HeaderAddChecker(CommitChecker): + commit_types = (Commit, StagedChanges, Amendment) + + @classmethod + def check(cls, commit, top_level): + issues = [] + + meson_files = [f for f in commit.files() + if os.path.basename(f) == 'meson.build'] + + for filename in commit.files('AR'): + if not filename.startswith('include/libcamera/') or \ + not filename.endswith('.h'): + continue + + meson = os.path.dirname(filename) + '/meson.build' + header = os.path.basename(filename) + + issue = CommitIssue('Header %s added without corresponding update to %s' % + (filename, meson)) + + if meson not in meson_files: + issues.append(issue) + continue + + diff = commit.get_diff(top_level, meson) + found = False + + for hunk in diff: + for line in hunk.lines: + if line[0] != '+': + continue + + if line.find("'%s'" % header) != -1: + found = True + break + + if found: + break + + if not found: + issues.append(issue) + + return issues + + +class TitleChecker(CommitChecker): + commit_types = (Commit,) + + prefix_regex = re.compile(r'^([a-zA-Z0-9_.-]+: )+') + release_regex = re.compile(r'libcamera v[0-9]+\.[0-9]+\.[0-9]+') + + @classmethod + def check(cls, commit, top_level): + title = commit.title + + # Ignore release commits, they don't need a prefix. + if TitleChecker.release_regex.fullmatch(title): + return [] + + prefix_pos = title.find(': ') + if prefix_pos != -1 and prefix_pos != len(title) - 2: + return [] + + # Find prefix candidates by searching the git history + msgs = subprocess.run(['git', 'log', '--no-decorate', '--oneline', '-n100', '--'] + commit.files(), + stdout=subprocess.PIPE).stdout.decode('utf-8') + prefixes = {} + prefixes_count = 0 + for msg in msgs.splitlines(): + prefix = TitleChecker.prefix_regex.match(msg) + if not prefix: + continue + + prefix = prefix.group(0) + if prefix in prefixes: + prefixes[prefix] += 1 + else: + prefixes[prefix] = 1 + + prefixes_count += 1 + + if not prefixes: + return [CommitIssue('Commit title is missing prefix')] + + # Sort the candidates by number of occurrences and pick the best ones. + # When multiple prefixes are possible without a clear winner, we want to + # display the most common options to the user, but without the most + # unlikely options to avoid too long messages. As a heuristic, select + # enough candidates to cover at least 2/3 of the possible prefixes, but + # never more than 4 candidates. + prefixes = list(prefixes.items()) + prefixes.sort(key=lambda x: x[1], reverse=True) + + candidates = [] + candidates_count = 0 + for prefix in prefixes: + candidates.append(f"`{prefix[0]}'") + candidates_count += prefix[1] + if candidates_count >= prefixes_count * 2 / 3 or \ + len(candidates) == 4: + break + + candidates = candidates[:-2] + [' or '.join(candidates[-2:])] + candidates = ', '.join(candidates) + + return [CommitIssue('Commit title is missing prefix, ' + 'possible candidates are ' + candidates)] + + +class TrailersChecker(CommitChecker): + commit_types = (Commit,) + + commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \(".*"\)') + + coverity_regex = re.compile(r'Coverity CID=.*') + + # Simple e-mail address validator regex, with an additional trailing + # comment. The complexity of a full RFC6531 validator isn't worth the + # additional invalid addresses it would reject. + email_regex = re.compile(r'[^<]+ <[^@>]+@[^>]+>( # .*)?') + + link_regex = re.compile(r'https?://.*') + + @staticmethod + def validate_reported_by(value): + if TrailersChecker.email_regex.fullmatch(value): + return True + if TrailersChecker.coverity_regex.fullmatch(value): + return True + return False + + known_trailers = { + 'Acked-by': email_regex, + 'Bug': link_regex, + 'Co-developed-by': email_regex, + 'Fixes': commit_regex, + 'Link': link_regex, + 'Reported-by': validate_reported_by, + 'Reviewed-by': email_regex, + 'Signed-off-by': email_regex, + 'Suggested-by': email_regex, + 'Tested-by': email_regex, + } + + trailer_regex = re.compile(r'([A-Z][a-zA-Z-]*)\s*:\s*(.*)') + + @classmethod + def check(cls, commit, top_level): + issues = [] + + sob_found = False + + for trailer in commit.trailers: + match = TrailersChecker.trailer_regex.fullmatch(trailer) + if not match: + issues.append(CommitIssue(f"Malformed commit trailer '{trailer}'")) + continue + + key, value = match.groups() + + validator = TrailersChecker.known_trailers.get(key) + if not validator: + issues.append(CommitIssue(f"Invalid commit trailer key '{key}'")) + continue + + if isinstance(validator, re.Pattern): + valid = bool(validator.fullmatch(value)) + else: + valid = validator(value) + + if not valid: + issues.append(CommitIssue(f"Malformed value '{value}' for commit trailer '{key}'")) + continue + + if key == 'Signed-off-by': + if value == commit.author: + sob_found = True + + if not sob_found: + issues.append(CommitIssue(f"No 'Signed-off-by' trailer matching author '{commit.author}', see Documentation/contributing.rst")) + + return issues + + +# ------------------------------------------------------------------------------ +# Style Checkers +# + +class StyleChecker(CheckerBase): + pass + class StyleIssue(object): - def __init__(self, line_number, line, msg): + def __init__(self, line_number, position, line, msg): self.line_number = line_number + self.position = position self.line = line self.msg = msg + def __str__(self): + s = [] + s.append(f'{Colours.fg(Colours.Yellow)}#{self.line_number}: {self.msg}{Colours.reset()}') + if self.line is not None: + s.append(f'{Colours.fg(Colours.Yellow)}+{self.line.rstrip()}{Colours.reset()}') + + if self.position is not None: + # Align the position marker by using the original line with + # all characters except for tabs replaced with spaces. This + # ensures proper alignment regardless of how the code is + # indented. + start = self.position[0] + prefix = ''.join([c if c == '\t' else ' ' for c in self.line[:start]]) + length = self.position[1] - start - 1 + s.append(f' {prefix}^{"~" * length}') + + return '\n'.join(s) + + +class HexValueChecker(StyleChecker): + patterns = ('*.c', '*.cpp', '*.h') + + regex = re.compile(r'\b0[xX][0-9a-fA-F]+\b') + + @classmethod + def check(cls, content, line_numbers): + issues = [] + + for line_number in line_numbers: + line = content[line_number - 1] + match = HexValueChecker.regex.search(line) + if not match: + continue + + value = match.group(0) + if value == value.lower(): + continue + + issues.append(StyleIssue(line_number, match.span(0), line, + f'Use lowercase hex constant {value.lower()}')) + + return issues + class IncludeChecker(StyleChecker): patterns = ('*.cpp', '*.h') - headers = ('assert', 'ctype', 'errno', 'fenv', 'float', 'inttypes', - 'limits', 'locale', 'math', 'setjmp', 'signal', 'stdarg', - 'stddef', 'stdint', 'stdio', 'stdlib', 'string', 'time', 'uchar', - 'wchar', 'wctype') - include_regex = re.compile('^#include <c([a-z]*)>') - - def __init__(self, content): - super().__init__() - self.__content = content + headers = ('cassert', 'cctype', 'cerrno', 'cfenv', 'cfloat', 'cinttypes', + 'climits', 'clocale', 'csetjmp', 'csignal', 'cstdarg', 'cstddef', + 'cstdint', 'cstdio', 'cstdlib', 'cstring', 'ctime', 'cuchar', + 'cwchar', 'cwctype', 'math.h') + include_regex = re.compile(r'^#include <([a-z.]*)>') - def check(self, line_numbers): + @classmethod + def check(self, content, line_numbers): issues = [] for line_number in line_numbers: - line = self.__content[line_number - 1] + line = content[line_number - 1] match = IncludeChecker.include_regex.match(line) if not match: continue @@ -266,28 +674,34 @@ class IncludeChecker(StyleChecker): if header not in IncludeChecker.headers: continue - issues.append(StyleIssue(line_number, line, - 'C compatibility header <%s.h> is preferred' % header)) + if header.endswith('.h'): + header_type = 'C++' + header = 'c' + header[:-2] + else: + header_type = 'C compatibility' + header = header[1:] + '.h' + + issues.append(StyleIssue(line_number, match.span(1), line, + f'{header_type} header <{header}> is preferred')) return issues class LogCategoryChecker(StyleChecker): - log_regex = re.compile('\\bLOG\((Debug|Info|Warning|Error|Fatal)\)') + log_regex = re.compile(r'\bLOG\((Debug|Info|Warning|Error|Fatal)\)') patterns = ('*.cpp',) - def __init__(self, content): - super().__init__() - self.__content = content - - def check(self, line_numbers): + @classmethod + def check(cls, content, line_numbers): issues = [] for line_number in line_numbers: - line = self.__content[line_number-1] - if not LogCategoryChecker.log_regex.search(line): + line = content[line_number - 1] + match = LogCategoryChecker.log_regex.search(line) + if not match: continue - issues.append(StyleIssue(line_number, line, 'LOG() should use categories')) + issues.append(StyleIssue(line_number, match.span(1), line, + 'LOG() should use categories')) return issues @@ -295,70 +709,30 @@ class LogCategoryChecker(StyleChecker): class MesonChecker(StyleChecker): patterns = ('meson.build',) - def __init__(self, content): - super().__init__() - self.__content = content - - def check(self, line_numbers): + @classmethod + def check(cls, content, line_numbers): issues = [] for line_number in line_numbers: - line = self.__content[line_number-1] - if line.find('\t') != -1: - issues.append(StyleIssue(line_number, line, 'meson.build should use spaces for indentation')) - return issues - - -class Pep8Checker(StyleChecker): - patterns = ('*.py',) - results_regex = re.compile('stdin:([0-9]+):([0-9]+)(.*)') - - def __init__(self, content): - super().__init__() - self.__content = content - - def check(self, line_numbers): - issues = [] - data = ''.join(self.__content).encode('utf-8') - - try: - ret = subprocess.run(['pycodestyle', '--ignore=E501', '-'], - input=data, stdout=subprocess.PIPE) - except FileNotFoundError: - issues.append(StyleIssue(0, None, "Please install pycodestyle to validate python additions")) - return issues - - results = ret.stdout.decode('utf-8').splitlines() - for item in results: - search = re.search(Pep8Checker.results_regex, item) - line_number = int(search.group(1)) - position = int(search.group(2)) - msg = search.group(3) - - if line_number in line_numbers: - line = self.__content[line_number - 1] - issues.append(StyleIssue(line_number, line, msg)) - + line = content[line_number - 1] + pos = line.find('\t') + if pos != -1: + issues.append(StyleIssue(line_number, [pos, pos], line, + 'meson.build should use spaces for indentation')) return issues class ShellChecker(StyleChecker): + dependencies = ('shellcheck',) patterns = ('*.sh',) - results_line_regex = re.compile('In - line ([0-9]+):') - - def __init__(self, content): - super().__init__() - self.__content = content + results_line_regex = re.compile(r'In - line ([0-9]+):') - def check(self, line_numbers): + @classmethod + def check(cls, content, line_numbers): issues = [] - data = ''.join(self.__content).encode('utf-8') + data = ''.join(content).encode('utf-8') - try: - ret = subprocess.run(['shellcheck', '-Cnever', '-'], - input=data, stdout=subprocess.PIPE) - except FileNotFoundError: - issues.append(StyleIssue(0, None, "Please install shellcheck to validate shell script additions")) - return issues + ret = subprocess.run(['shellcheck', '-Cnever', '-'], + input=data, stdout=subprocess.PIPE) results = ret.stdout.decode('utf-8').splitlines() for nr, item in enumerate(results): @@ -370,11 +744,8 @@ class ShellChecker(StyleChecker): line = results[nr + 1] msg = results[nr + 2] - # Determined, but not yet used - position = msg.find('^') + 1 - if line_number in line_numbers: - issues.append(StyleIssue(line_number, line, msg)) + issues.append(StyleIssue(line_number, None, line, msg)) return issues @@ -383,67 +754,14 @@ class ShellChecker(StyleChecker): # Formatters # -_formatters = [] - -class FormatterRegistry(type): - def __new__(cls, clsname, bases, attrs): - newclass = super(FormatterRegistry, cls).__new__(cls, clsname, bases, attrs) - if clsname != 'Formatter': - _formatters.append(newclass) - return newclass - - -class Formatter(metaclass=FormatterRegistry): - enabled = True - - def __init__(self): - pass - - # - # Class methods - # - @classmethod - def formatters(cls, filename): - for formatter in _formatters: - if not cls.enabled: - continue - if formatter.supports(filename): - yield formatter - - @classmethod - def supports(cls, filename): - if not cls.enabled: - return False - for pattern in cls.patterns: - if fnmatch.fnmatch(os.path.basename(filename), pattern): - return True - return False - - @classmethod - def all_patterns(cls): - patterns = set() - for formatter in _formatters: - if not cls.enabled: - continue - patterns.update(formatter.patterns) - - return patterns - - -class AStyleFormatter(Formatter): - enabled = False - patterns = ('*.c', '*.cpp', '*.h') - - @classmethod - def format(cls, filename, data): - ret = subprocess.run(['astyle', *astyle_options], - input=data.encode('utf-8'), stdout=subprocess.PIPE) - return ret.stdout.decode('utf-8') +class Formatter(CheckerBase): + pass class CLangFormatter(Formatter): - enabled = False + dependencies = ('clang-format',) patterns = ('*.c', '*.cpp', '*.h') + priority = -1 @classmethod def format(cls, filename, data): @@ -456,7 +774,8 @@ class CLangFormatter(Formatter): class DoxygenFormatter(Formatter): patterns = ('*.c', '*.cpp') - return_regex = re.compile(' +\\* +\\\\return +[a-z]') + oneliner_regex = re.compile(r'^ +\* +\\(brief|param|return)\b.*\.$') + return_regex = re.compile(r' +\* +\\return +[a-z]') @classmethod def format(cls, filename, data): @@ -471,6 +790,7 @@ class DoxygenFormatter(Formatter): lines.append(line) continue + line = cls.oneliner_regex.sub(lambda m: m.group(0)[:-1], line) line = cls.return_regex.sub(lambda m: m.group(0)[:-1] + m.group(0)[-1].upper(), line) if line.find('*/') != -1: @@ -481,10 +801,42 @@ class DoxygenFormatter(Formatter): return '\n'.join(lines) +class DPointerFormatter(Formatter): + # Ensure consistent naming of variables related to the d-pointer design + # pattern. + patterns = ('*.cpp', '*.h') + + # The clang formatter runs first, we can thus rely on appropriate coding + # style. + declare_regex = re.compile(r'^(\t*)(const )?([a-zA-Z0-9_]+) \*( ?const )?([a-zA-Z0-9_]+) = (LIBCAMERA_[DO]_PTR)\(([a-zA-Z0-9_]+)\);$') + + @classmethod + def format(cls, filename, data): + lines = [] + + for line in data.split('\n'): + match = cls.declare_regex.match(line) + if match: + indent = match.group(1) or '' + const = match.group(2) or '' + macro = match.group(6) + klass = match.group(7) + if macro == 'LIBCAMERA_D_PTR': + var = 'Private *const d' + else: + var = f'{klass} *const o' + + line = f'{indent}{const}{var} = {macro}({klass});' + + lines.append(line) + + return '\n'.join(lines) + + class IncludeOrderFormatter(Formatter): patterns = ('*.cpp', '*.h') - include_regex = re.compile('^#include ["<]([^">]*)[">]') + include_regex = re.compile(r'^#include (["<])([^">]*)([">])') @classmethod def format(cls, filename, data): @@ -498,7 +850,21 @@ class IncludeOrderFormatter(Formatter): if match: # If the current line is an #include statement, add it to the # includes group and continue to the next line. - includes.append((line, match.group(1))) + open_token = match.group(1) + file_name = match.group(2) + close_token = match.group(3) + + # Ensure the "..." include style for internal headers and the + # <...> style for all other libcamera headers. + if (file_name.startswith('libcamera/internal')): + open_token = '"' + close_token = '"' + elif (file_name.startswith('libcamera/')): + open_token = '<' + close_token = '>' + + line = f'#include {open_token}{file_name}{close_token}' + includes.append((line, file_name)) continue # The current line is not an #include statement, output the sorted @@ -522,6 +888,17 @@ class IncludeOrderFormatter(Formatter): return '\n'.join(lines) +class Pep8Formatter(Formatter): + dependencies = ('autopep8',) + patterns = ('*.py',) + + @classmethod + def format(cls, filename, data): + ret = subprocess.run(['autopep8', '--ignore=E501', '-'], + input=data.encode('utf-8'), stdout=subprocess.PIPE) + return ret.stdout.decode('utf-8') + + class StripTrailingSpaceFormatter(Formatter): patterns = ('*.c', '*.cpp', '*.h', '*.py', 'meson.build') @@ -537,69 +914,27 @@ class StripTrailingSpaceFormatter(Formatter): # Style checking # -class Commit: - def __init__(self, commit): - self.commit = commit - - def get_info(self): - # Get the commit title and list of files. - ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', - self.commit], - stdout=subprocess.PIPE).stdout.decode('utf-8') - files = ret.splitlines() - # Returning title and files list as a tuple - return files[0], files[1:] - - def get_diff(self, top_level, filename): - return subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit), - '--', '%s/%s' % (top_level, filename)], - stdout=subprocess.PIPE).stdout.decode('utf-8') - - def get_file(self, filename): - return subprocess.run(['git', 'show', '%s:%s' % (self.commit, filename)], - stdout=subprocess.PIPE).stdout.decode('utf-8') - - -class StagedChanges(Commit): - def __init__(self): - Commit.__init__(self, '') - - def get_info(self): - ret = subprocess.run(['git', 'diff', '--staged', '--name-only'], - stdout=subprocess.PIPE).stdout.decode('utf-8') - return "Staged changes", ret.splitlines() - - def get_diff(self, top_level, filename): - return subprocess.run(['git', 'diff', '--staged', '--', - '%s/%s' % (top_level, filename)], - stdout=subprocess.PIPE).stdout.decode('utf-8') +def check_commit(top_level, commit, checkers): + issues = [] + # Apply the commit checkers first. + for checker in CommitChecker.instances(commit, checkers): + issues_ = checker.check_dependencies() + if issues_: + issues += issues_ + continue -class Amendment(StagedChanges): - def __init__(self): - StagedChanges.__init__(self) + issues += checker.check(commit, top_level) - def get_info(self): - # Create a title using HEAD commit - ret = subprocess.run(['git', 'show', '--pretty=oneline', '--no-patch'], - stdout=subprocess.PIPE).stdout.decode('utf-8') - title = 'Amendment of ' + ret.strip() - # Extract the list of modified files - ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'], - stdout=subprocess.PIPE).stdout.decode('utf-8') - return title, ret.splitlines() + for issue in issues: + print(issue) - def get_diff(self, top_level, filename): - return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--', - '%s/%s' % (top_level, filename)], - stdout=subprocess.PIPE).stdout.decode('utf-8') + return len(issues) -def check_file(top_level, commit, filename): +def check_file(top_level, commit, filename, checkers): # Extract the line numbers touched by the commit. - diff = commit.get_diff(top_level, filename) - diff = diff.splitlines(True) - commit_diff = parse_diff(diff) + commit_diff = commit.get_diff(top_level, filename) lines = [] for hunk in commit_diff: @@ -612,9 +947,15 @@ def check_file(top_level, commit, filename): # Format the file after the commit with all formatters and compute the diff # between the unformatted and formatted contents. after = commit.get_file(filename) + issues = [] formatted = after - for formatter in Formatter.formatters(filename): + for formatter in Formatter.instances(filename, checkers): + issues_ = formatter.check_dependencies() + if issues_: + issues += issues_ + continue + formatted = formatter.format(filename, formatted) after = after.splitlines(True) @@ -627,11 +968,14 @@ def check_file(top_level, commit, filename): formatted_diff = [hunk for hunk in formatted_diff if hunk.intersects(lines)] # Check for code issues not related to formatting. - issues = [] - for checker in StyleChecker.checkers(filename): - checker = checker(after) + for checker in StyleChecker.instances(filename, checkers): + issues_ = checker.check_dependencies() + if issues_: + issues += issues_ + continue + for hunk in commit_diff: - issues += checker.check(hunk.side('to').touched) + issues += checker.check(after, hunk.side('to').touched) # Print the detected issues. if len(issues) == 0 and len(formatted_diff) == 0: @@ -645,42 +989,38 @@ def check_file(top_level, commit, filename): print(hunk) if len(issues): - issues = sorted(issues, key=lambda i: i.line_number) + issues = sorted(issues, key=lambda i: getattr(i, 'line_number', -1)) for issue in issues: - print('%s#%u: %s' % (Colours.fg(Colours.Yellow), issue.line_number, issue.msg)) - if issue.line is not None: - print('+%s%s' % (issue.line.rstrip(), Colours.reset())) + print(issue) return len(formatted_diff) + len(issues) -def check_style(top_level, commit): - title, files = commit.get_info() - +def check_style(top_level, commit, checkers): + title = commit.commit + ' ' + commit.title separator = '-' * len(title) print(separator) print(title) print(separator) + # Apply the commit checkers first. + issues = check_commit(top_level, commit, checkers) + # Filter out files we have no checker for. patterns = set() patterns.update(StyleChecker.all_patterns()) patterns.update(Formatter.all_patterns()) - files = [f for f in files if len([p for p in patterns if fnmatch.fnmatch(os.path.basename(f), p)])] - if len(files) == 0: - print("Commit doesn't touch source files, skipping") - return 0 + files = [f for f in commit.files() if len([p for p in patterns if fnmatch.fnmatch(os.path.basename(f), p)])] - issues = 0 for f in files: - issues += check_file(top_level, commit, f) + issues += check_file(top_level, commit, f, checkers) if issues == 0: - print("No style issue detected") + print('No issue detected') else: print('---') - print("%u potential style %s detected, please review" % \ - (issues, 'issue' if issues == 1 else 'issues')) + print('%u potential %s detected, please review' % + (issues, 'issue' if issues == 1 else 'issues')) return issues @@ -723,8 +1063,8 @@ def main(argv): # Parse command line arguments parser = argparse.ArgumentParser() - parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'], - help='Code formatter. Default to clang-format if not specified.') + parser.add_argument('--checkers', '-c', type=str, + help='Specify which checkers to run as a comma-separated list. Defaults to all checkers') parser.add_argument('--staged', '-s', action='store_true', help='Include the changes in the index. Defaults to False') parser.add_argument('--amend', '-a', action='store_true', @@ -733,30 +1073,18 @@ def main(argv): help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.') args = parser.parse_args(argv[1:]) - # Check for required dependencies. + if args.checkers: + args.checkers = args.checkers.split(',') + + # Check for required common dependencies. for command, mandatory in dependencies.items(): found = shutil.which(command) if mandatory and not found: - print("Executable %s not found" % command) + print('Executable %s not found' % command) return 1 dependencies[command] = found - if args.formatter: - if not args.formatter in dependencies or \ - not dependencies[args.formatter]: - print("Formatter %s not available" % args.formatter) - return 1 - formatter = args.formatter - else: - if dependencies['clang-format']: - CLangFormatter.enabled = True - elif dependencies['astyle']: - AStyleFormatter.enabled = True - else: - print("No formatter found, please install clang-format or astyle") - return 1 - # Get the top level directory to pass absolute file names to git diff # commands, in order to support execution from subdirectories of the git # tree. @@ -781,7 +1109,7 @@ def main(argv): issues = 0 for commit in commits: - issues += check_style(top_level, commit) + issues += check_style(top_level, commit, args.checkers) print('') if issues: |