From patchwork Tue Jun 25 17:09:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1519 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 464C860103 for ; Tue, 25 Jun 2019 19:09:40 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CA867510 for ; Tue, 25 Jun 2019 19:09:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1561482579; bh=ttD+kglOiWrxthWdMaNZVnR8m68E9Q2ym9CgYwPNKRY=; h=From:To:Subject:Date:From; b=KuD5/98/erNukXOgSCJcO9mTmEOcrPTfC12T0g3CuXpDAuMkJiiORNrwOKqyPW0x4 z3Eotc/tZL0G4rEI64jS7zxI6LXqJ9lGHqtCKhyaRCQnERjSB3tqPpt9akcvM1f/Y9 O7dUL0nLPIeNbiGo8saG4zWB+Y85XhNl6yaeWiqw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 25 Jun 2019 20:09:16 +0300 Message-Id: <20190625170917.17818-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] utils: checkstyle.py: Refactor formatters and checkers support X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Jun 2019 17:09:40 -0000 Introduce two new base classes for the code formatters and style checkers, with an auto-registration mechanism that automatically uses all derived classes. This will allow easier addition of new formatters and checkers. Signed-off-by: Laurent Pinchart --- utils/checkstyle.py | 202 ++++++++++++++++++++++++++++++++------------ 1 file changed, 149 insertions(+), 53 deletions(-) diff --git a/utils/checkstyle.py b/utils/checkstyle.py index 9abd2687b1f9..5c8bde6e1f46 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -15,6 +15,8 @@ import argparse import difflib +import fnmatch +import os.path import re import shutil import subprocess @@ -39,12 +41,6 @@ dependencies = { 'git': True, } -source_extensions = ( - '.c', - '.cpp', - '.h' -) - # ------------------------------------------------------------------------------ # Colour terminal handling # @@ -195,44 +191,61 @@ def parse_diff(diff): # ------------------------------------------------------------------------------ -# Code reformatting +# Style Checkers # -def formatter_astyle(filename, data): - ret = subprocess.run(['astyle', *astyle_options], - input=data.encode('utf-8'), stdout=subprocess.PIPE) - return ret.stdout.decode('utf-8') +_style_checkers = [] +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 -def formatter_clang_format(filename, data): - ret = subprocess.run(['clang-format', '-style=file', - '-assume-filename=' + filename], - input=data.encode('utf-8'), stdout=subprocess.PIPE) - return ret.stdout.decode('utf-8') +class StyleChecker(metaclass=StyleCheckerRegistry): + def __init__(self): + pass -def formatter_strip_trailing_space(filename, data): - lines = data.split('\n') - for i in range(len(lines)): - lines[i] = lines[i].rstrip() + '\n' - return ''.join(lines) + # + # Class methods + # + @classmethod + def checkers(cls, filename): + for checker in _style_checkers: + if checker.supports(filename): + yield checker + @classmethod + def supports(cls, filename): + for pattern in cls.patterns: + if fnmatch.fnmatch(os.path.basename(filename), pattern): + return True + return False -available_formatters = { - 'astyle': formatter_astyle, - 'clang-format': formatter_clang_format, - 'strip-trailing-spaces': formatter_strip_trailing_space, -} + @classmethod + def all_patterns(cls): + patterns = set() + for checker in _style_checkers: + patterns.update(checker.patterns) + return patterns + + +class StyleIssue(object): + def __init__(self, line_number, line, msg): + self.line_number = line_number + self.line = line + self.msg = msg -# ------------------------------------------------------------------------------ -# Style checking -# -class LogCategoryChecker(object): +class LogCategoryChecker(StyleChecker): log_regex = re.compile('\\bLOG\((Debug|Info|Warning|Error|Fatal)\)') + patterns = ('*.cpp',) def __init__(self, content): + super().__init__() self.__content = content def check(self, line_numbers): @@ -242,17 +255,101 @@ class LogCategoryChecker(object): if not LogCategoryChecker.log_regex.search(line): continue - issues.append([line_number, line, 'LOG() should use categories']) + issues.append(StyleIssue(line_number, line, 'LOG() should use categories')) return issues -available_checkers = { - 'log_category': LogCategoryChecker, -} +# ------------------------------------------------------------------------------ +# Formatters +# +_formatters = [] -def check_file(top_level, commit, filename, 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 CLangFormatter(Formatter): + enabled = False + patterns = ('*.c', '*.cpp', '*.h') + + @classmethod + def format(cls, filename, data): + ret = subprocess.run(['clang-format', '-style=file', + '-assume-filename=' + filename], + input=data.encode('utf-8'), stdout=subprocess.PIPE) + return ret.stdout.decode('utf-8') + + +class StripTrailingSpaceFormatter(Formatter): + patterns = ('*.c', '*.cpp', '*.h', '*.py', 'meson.build') + + @classmethod + def format(cls, filename, data): + lines = data.split('\n') + for i in range(len(lines)): + lines[i] = lines[i].rstrip() + '\n' + return ''.join(lines) + + +# ------------------------------------------------------------------------------ +# Style checking +# + +def check_file(top_level, commit, filename): # Extract the line numbers touched by the commit. diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--', '%s/%s' % (top_level, filename)], @@ -275,9 +372,8 @@ def check_file(top_level, commit, filename, formatters): after = after.decode('utf-8') formatted = after - for formatter in formatters: - formatter = available_formatters[formatter] - formatted = formatter(filename, formatted) + for formatter in Formatter.formatters(filename): + formatted = formatter.format(filename, formatted) after = after.splitlines(True) formatted = formatted.splitlines(True) @@ -290,8 +386,8 @@ def check_file(top_level, commit, filename, formatters): # Check for code issues not related to formatting. issues = [] - for checker in available_checkers: - checker = available_checkers[checker](after) + for checker in StyleChecker.checkers(filename): + checker = checker(after) for hunk in commit_diff: issues += checker.check(hunk.side('to').touched) @@ -307,15 +403,15 @@ def check_file(top_level, commit, filename, formatters): print(hunk) if len(issues): - issues.sort() + issues = sorted(issues, key=lambda i: i.line_number) for issue in issues: - print('%s#%u: %s' % (Colours.fg(Colours.Yellow), issue[0], issue[2])) - print('+%s%s' % (issue[1].rstrip(), Colours.reset())) + print('%s#%u: %s' % (Colours.fg(Colours.Yellow), issue.line_number, issue.msg)) + print('+%s%s' % (issue.line.rstrip(), Colours.reset())) return len(formatted_diff) + len(issues) -def check_style(top_level, commit, formatters): +def check_style(top_level, commit): # Get the commit title and list of files. ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit], stdout=subprocess.PIPE) @@ -328,15 +424,18 @@ def check_style(top_level, commit, formatters): print(title) print(separator) - # Filter out non C/C++ files. - files = [f for f in files if f.endswith(source_extensions)] + # 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 issues = 0 for f in files: - issues += check_file(top_level, commit, f, formatters) + issues += check_file(top_level, commit, f) if issues == 0: print("No style issue detected") @@ -407,16 +506,13 @@ def main(argv): formatter = args.formatter else: if dependencies['clang-format']: - formatter = 'clang-format' + CLangFormatter.enabled = True elif dependencies['astyle']: - formatter = 'astyle' + AStyleFormatter.enabled = True else: print("No formatter found, please install clang-format or astyle") return 1 - # Create the list of formatters to be applied. - formatters = [formatter, 'strip-trailing-spaces'] - # 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. @@ -427,7 +523,7 @@ def main(argv): revlist = extract_revlist(args.revision_range) for commit in revlist: - check_style(top_level, commit, formatters) + check_style(top_level, commit) print('') return 0