From f9fceb7ffce6994f5a7a4b9e4a18c54044bbce90 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Wed, 29 Jul 2020 18:19:48 +0200 Subject: [PATCH] Fix ci-custom.py const.py ordered check and improve code (#1222) --- script/ci-custom.py | 77 +++++++++++++++++++++++++++++++++++---------- script/quicklint | 2 +- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/script/ci-custom.py b/script/ci-custom.py index 3954eea0de..ab2beadf85 100755 --- a/script/ci-custom.py +++ b/script/ci-custom.py @@ -7,9 +7,18 @@ import os.path import re import subprocess import sys +import time +import functools +import argparse +sys.path.append(os.path.dirname(__file__)) +from helpers import git_ls_files, filter_changed def find_all(a_str, sub): + if not a_str.find(sub): + # Optimization: If str is not in whole text, then do not try + # on each line + return for i, line in enumerate(a_str.splitlines()): column = 0 while True: @@ -20,15 +29,24 @@ def find_all(a_str, sub): column += len(sub) -command = ['git', 'ls-files', '-s'] -proc = subprocess.Popen(command, stdout=subprocess.PIPE) -output, err = proc.communicate() -lines = [x.split() for x in output.decode('utf-8').splitlines()] -EXECUTABLE_BIT = { - s[3].strip(): int(s[0]) for s in lines -} -files = [s[3].strip() for s in lines] -files = list(filter(os.path.exists, files)) +parser = argparse.ArgumentParser() +parser.add_argument('files', nargs='*', default=[], + help='files to be processed (regex on path)') +parser.add_argument('-c', '--changed', action='store_true', + help='Only run on changed files') +parser.add_argument('--print-slowest', action='store_true', + help='Print the slowest checks') +args = parser.parse_args() + +EXECUTABLE_BIT = git_ls_files() +files = list(EXECUTABLE_BIT.keys()) +# Match against re +file_name_re = re.compile('|'.join(args.files)) +files = [p for p in files if file_name_re.search(p)] + +if args.changed: + files = filter_changed(files) + files.sort() file_types = ('.h', '.c', '.cpp', '.tcc', '.yaml', '.yml', '.ini', '.txt', '.ico', '.svg', @@ -60,7 +78,14 @@ def run_check(lint_obj, fname, *args): def run_checks(lints, fname, *args): for lint in lints: - add_errors(fname, run_check(lint, fname, *args)) + start = time.process_time() + try: + add_errors(fname, run_check(lint, fname, *args)) + except Exception: + print(f"Check {lint['func'].__name__} on file {fname} failed:") + raise + duration = time.process_time() - start + lint.setdefault('durations', []).append(duration) def _add_check(checks, func, include=None, exclude=None): @@ -96,6 +121,7 @@ def lint_re_check(regex, **kwargs): decor = lint_content_check(**kwargs) def decorator(func): + @functools.wraps(func) def new_func(fname, content): errors = [] for match in prog.finditer(content): @@ -109,6 +135,7 @@ def lint_re_check(regex, **kwargs): continue errors.append((lineno, col+1, err)) return errors + return decor(new_func) return decorator @@ -117,6 +144,7 @@ def lint_content_find_check(find, **kwargs): decor = lint_content_check(**kwargs) def decorator(func): + @functools.wraps(func) def new_func(fname, content): find_ = find if callable(find): @@ -206,6 +234,10 @@ def lint_no_long_delays(fname, match): @lint_content_check(include=['esphome/const.py']) def lint_const_ordered(fname, content): + """Lint that value in const.py are ordered. + + Reason: Otherwise people add it to the end, and then that results in merge conflicts. + """ lines = content.splitlines() errors = [] for start in ['CONF_', 'ICON_', 'UNIT_']: @@ -217,10 +249,10 @@ def lint_const_ordered(fname, content): continue target = next(i for i, l in ordered if l == ml) target_text = next(l for i, l in matching if target == i) - errors.append((ml, None, - "Constant {} is not ordered, please make sure all constants are ordered. " - "See line {} (should go to line {}, {})" - "".format(highlight(ml), mi, target, target_text))) + errors.append((mi, 1, + f"Constant {highlight(ml)} is not ordered, please make sure all " + f"constants are ordered. See line {mi} (should go to line {target}, " + f"{target_text})")) return errors @@ -302,7 +334,7 @@ def lint_no_arduino_framework_functions(fname, match): ) -@lint_re_check(r'[^\w\d]byte\s+[\w\d]+\s*=.*', include=cpp_include, exclude={ +@lint_re_check(r'[^\w\d]byte\s+[\w\d]+\s*=', include=cpp_include, exclude={ 'esphome/components/tuya/tuya.h', }) def lint_no_byte_datatype(fname, match): @@ -385,8 +417,8 @@ def lint_pragma_once(fname, content): return None -@lint_re_check(r'(whitelist|blacklist|slave)', exclude=['script/ci-custom.py'], - flags=re.IGNORECASE | re.MULTILINE) +@lint_re_check(r'(whitelist|blacklist|slave)', + exclude=['script/ci-custom.py'], flags=re.IGNORECASE | re.MULTILINE) def lint_inclusive_language(fname, match): # From https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49decddd39e5f6132ccd7d9fdc3d7c470b0061bb return ("Avoid the use of whitelist/blacklist/slave.\n" @@ -471,4 +503,15 @@ for f, errs in sorted(errors.items()): print(f"ERROR {f}:{lineno}:{col} - {msg}") print() +if args.print_slowest: + lint_times = [] + for lint in LINT_FILE_CHECKS + LINT_CONTENT_CHECKS + LINT_POST_CHECKS: + durations = lint.get('durations', []) + lint_times.append((sum(durations), len(durations), lint['func'].__name__)) + lint_times.sort(key=lambda x: -x[0]) + for i in range(min(len(lint_times), 10)): + dur, invocations, name = lint_times[i] + print(f" - '{name}' took {dur:.2f}s total (ran on {invocations} files)") + print(f"Total time measured: {sum(x[0] for x in lint_times):.2f}s") + sys.exit(len(errors)) diff --git a/script/quicklint b/script/quicklint index e391ca3276..a4fae98195 100755 --- a/script/quicklint +++ b/script/quicklint @@ -6,6 +6,6 @@ cd "$(dirname "$0")/.." set -x -script/ci-custom.py +script/ci-custom.py -c script/lint-python -c script/lint-cpp -c