Fix ci-custom.py const.py ordered check and improve code (#1222)

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:
+        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))

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