pw_presubmit: clang-format wrapper; misc
- Add format_cc.py, which simplifies using clang-format in presubmits by
checking for changes and printing a colorized diff.
- Add --continue option to presubmit_tools.py to continue running checks
after a failure.
- Add exclude option to filter_paths.
Change-Id: If17cf84b3e33550054a10f36df6403dada9005b4
diff --git a/pw_presubmit/format_cc.py b/pw_presubmit/format_cc.py
new file mode 100755
index 0000000..625a2c6
--- /dev/null
+++ b/pw_presubmit/format_cc.py
@@ -0,0 +1,151 @@
+#!/usr/bin/env python3
+
+# Copyright 2019 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+"""Checks that C and C++ source files match clang-format's formatting."""
+
+import argparse
+import difflib
+import os
+import subprocess
+import sys
+from typing import Container, Iterable, List, Optional
+
+SOURCE_EXTENSIONS = frozenset(['.h', '.hh', '.hpp', '.c', '.cc', '.cpp'])
+DEFAULT_FORMATTER = 'clang-format'
+
+
+def _make_color(*codes: int):
+ start = ''.join(f'\033[{code}m' for code in codes)
+ return f'{start}{{}}\033[0m'.format if os.name == 'posix' else str
+
+
+color_green = _make_color(32)
+color_red = _make_color(31)
+
+
+def _find_extensions(directory, extensions) -> Iterable[str]:
+ for root, _, files in os.walk(directory):
+ for file in files:
+ if os.path.splitext(file)[1] in extensions:
+ yield os.path.join(root, file)
+
+
+def list_files(paths: Iterable[str], extensions: Container[str]) -> List[str]:
+ """Lists files with C or C++ extensions."""
+ files = set()
+
+ for path in paths:
+ if os.path.isfile(path):
+ files.add(path)
+ else:
+ files.update(_find_extensions(path, extensions))
+
+ return sorted(files)
+
+
+def clang_format(*args: str, formatter='clang-format') -> bytes:
+ """Returns the output of clang-format with the provided arguments."""
+ return subprocess.run([formatter, *args],
+ stdout=subprocess.PIPE,
+ check=True).stdout
+
+
+def _colorize_diff_line(line: str) -> str:
+ if line.startswith('-') and not line.startswith('--- '):
+ return color_red(line)
+ if line.startswith('+') and not line.startswith('+++ '):
+ return color_green(line)
+ return line
+
+
+def colorize_diff(lines: Iterable[str]) -> str:
+ """Takes a diff str or list of str lines and returns a colorized version."""
+ if isinstance(lines, str):
+ lines = lines.splitlines(True)
+
+ return ''.join(_colorize_diff_line(line) for line in lines)
+
+
+def clang_format_diff(path, formatter=DEFAULT_FORMATTER) -> Optional[str]:
+ """Returns a diff comparing clang-format's output to the path's contents."""
+ with open(path, 'rb') as fd:
+ current = fd.read()
+
+ formatted = clang_format(path, formatter=formatter)
+
+ if formatted != current:
+ diff = difflib.unified_diff(
+ current.decode(errors='replace').splitlines(True),
+ formatted.decode(errors='replace').splitlines(True),
+ f'{path} (original)', f'{path} (reformatted)')
+
+ return colorize_diff(diff)
+
+ return None
+
+
+def check_format(files, formatter=DEFAULT_FORMATTER) -> List[str]:
+ """Diffs files against clang-format; returns paths that did not match."""
+ errors = []
+
+ for path in files:
+ difference = clang_format_diff(path, formatter)
+ if difference:
+ errors.append(path)
+ print(difference)
+
+ if errors:
+ print(f'--> Files with formatting errors: {len(errors)}',
+ file=sys.stderr)
+ print(' ', '\n '.join(errors), file=sys.stderr)
+
+ return errors
+
+
+def _main(paths, fix, formatter) -> int:
+ """Checks or fixes formatting."""
+ files = list_files(paths, SOURCE_EXTENSIONS)
+
+ if fix:
+ for path in files:
+ clang_format(path, '-i', formatter=formatter)
+ return 0
+
+ errors = check_format(files, formatter)
+ return 1 if errors else 0
+
+
+def _parse_args() -> argparse.Namespace:
+ """Parse and return command line arguments."""
+ parser = argparse.ArgumentParser(description=__doc__)
+
+ parser.add_argument(
+ 'paths',
+ default=[os.getcwd()],
+ nargs='*',
+ help=('Files or directories to check. '
+ 'Within a directory, only C or C++ files are checked.'))
+ parser.add_argument('--fix',
+ action='store_true',
+ help='Apply clang-format fixes in place.')
+ parser.add_argument('--formatter',
+ default=DEFAULT_FORMATTER,
+ help='The clang-format binary to use.')
+
+ return parser.parse_args()
+
+
+if __name__ == '__main__':
+ sys.exit(_main(**vars(_parse_args())))
diff --git a/pw_presubmit/presubmit_tools.py b/pw_presubmit/presubmit_tools.py
index a5380e0..9cb56e3 100755
--- a/pw_presubmit/presubmit_tools.py
+++ b/pw_presubmit/presubmit_tools.py
@@ -1,3 +1,16 @@
+# Copyright 2019 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
"""Tools for running presubmit checks in a Git repository.
A presubmit checks are defined as a function or other callable. The function may
@@ -37,7 +50,7 @@
import subprocess
import sys
import time
-from typing import Callable, Iterable, Optional, Sequence
+from typing import Callable, Iterable, List, Optional, Sequence
from inspect import signature
@@ -93,11 +106,12 @@
return f'{start}{{}}\033[0m'.format if os.name == 'posix' else str
-color_red = _make_color(31, 1)
-color_bold_red = _make_color(30, 41)
+color_red = _make_color(31)
+color_bold_red = _make_color(31, 1)
+color_black_on_red = _make_color(30, 41)
color_yellow = _make_color(33, 1)
color_green = _make_color(32)
-color_bold_green = _make_color(30, 42)
+color_black_on_green = _make_color(30, 42)
def _make_box(section_alignments: Sequence[str]) -> str:
@@ -139,7 +153,7 @@
def _result_box(style, result, color, title, time_s, box=_make_box('^<^')):
minutes, seconds = divmod(time_s, 60)
- time_str = ' ' * 8 if time_s is None else f'{int(minutes)}:{seconds:04.1f}'
+ time_str = f' {int(minutes)}:{seconds:04.1f} '
info = f' {title} '.ljust(WIDTH - 14 - len(time_str))
return box.format(*style,
@@ -148,7 +162,7 @@
section2=info,
width2=len(info),
section3=time_str,
- width3=len(time_str) + 2)
+ width3=len(time_str))
class PresubmitFailure(Exception):
@@ -167,7 +181,7 @@
if self is _Result.PASS:
return color_green
if self is _Result.FAIL:
- return color_red
+ return color_bold_red
if self is _Result.CANCEL:
return color_yellow
@@ -179,13 +193,16 @@
self.paths = paths
self.log = print
- def run(self, program: Sequence[Callable]) -> bool:
+ def run(self, program: Sequence[Callable],
+ continue_on_error: bool = False) -> bool:
"""Executes a series of presubmit checks on the paths."""
self.log(_title(f'Presubmit checks for {os.path.basename(self.root)}'))
self._log_presubmit_start(program)
- start_time = time.time()
+ passed, failed = [], []
+
+ start_time: float = time.time()
for i, check in enumerate(program):
self.log(_start_box(_TOP, f'{i+1}/{len(program)}', check.__name__))
@@ -198,11 +215,13 @@
_result_box(_BOTTOM, result.value, result.color(),
check.__name__, time_s))
- if result is not _Result.PASS:
- passed, failed = program[:i], program[i:]
+ if result is _Result.PASS:
+ passed.append(check)
+ elif continue_on_error:
+ failed.append(check)
+ else:
+ failed = list(program[i:])
break
- else:
- passed, failed = program, []
self._log_summary(time.time() - start_time, len(passed), len(failed))
return not failed
@@ -212,7 +231,8 @@
check(*([self.paths] if signature(check).parameters else []))
return _Result.PASS
except Exception as failure:
- self.log(failure)
+ if str(failure):
+ self.log(failure)
return _Result.FAIL
except KeyboardInterrupt:
self.log()
@@ -236,10 +256,10 @@
def _log_summary(self, time_s: float, passed: int, failed: int) -> None:
text = 'FAILED' if failed else 'PASSED'
- color = color_bold_red if failed else color_bold_green
+ color = color_black_on_red if failed else color_black_on_green
self.log(
_result_box(_DOUBLE, text, color,
- f'Finished {passed} of {passed + failed} checks',
+ f'Passed {passed} of {passed + failed} checks',
time_s))
@@ -271,18 +291,26 @@
action='append',
type=re.compile,
help='Exclude paths matching any of these regular expressions.')
+ parser.add_argument('--continue',
+ dest='continue_on_error',
+ action='store_true',
+ help='Continue instead of aborting when errors occur.')
-def run_presubmit(program: Sequence[Callable],
- base: Optional[str] = None,
- paths: Sequence[str] = (),
- exclude: Sequence = (),
- repository: str = '.') -> bool:
+def run_presubmit(
+ program: Sequence[Callable],
+ base: Optional[str] = None,
+ paths: Sequence[str] = (),
+ exclude: Sequence = (),
+ repository: str = '.',
+ continue_on_error: bool = False,
+) -> bool:
"""Lists files in the current Git repo and runs a Presubmit with them."""
root = git_repo_root(repository)
os.chdir(root)
- return Presubmit(root, list_files(base, paths, exclude)).run(program)
+ return Presubmit(root, list_files(base, paths,
+ exclude)).run(program, continue_on_error)
def parse_args_and_run_presubmit(
@@ -299,16 +327,29 @@
return run_presubmit(program, **vars(arg_parser.parse_args()))
-def filter_paths(endswith: Iterable[str] = (), skip_if_empty: bool = True):
+def _wrap_if_str(value: Iterable[str]) -> List[str]:
+ return [value] if isinstance(value, str) else value
+
+
+def filter_paths(endswith: Iterable[str] = (''),
+ exclude: Iterable = (),
+ skip_if_empty: bool = True):
"""Decorator for filtering the files list for a presubmit check function."""
- endswith = frozenset([endswith] if isinstance(endswith, str) else endswith)
+ endswith = frozenset(_wrap_if_str(endswith))
+ exclude = [re.compile(exp) for exp in _wrap_if_str(exclude)]
def filter_paths_for_function(function: Callable):
+ if len(signature(function).parameters) != 1:
+ raise TypeError('Functions wrapped with @filter_paths must take '
+ f'exactly one argument: {function.__name__} takes '
+ f'{len(signature(function).parameters)}.')
+
@functools.wraps(function)
def wrapped_function(paths: Iterable[str]):
paths = [
- path for path in paths if any(
- path.endswith(end) for end in endswith)
+ path for path in paths
+ if any(path.endswith(end) for end in endswith) and not any(
+ exp.fullmatch(path) for exp in exclude)
]
if not paths and skip_if_empty:
@@ -334,7 +375,7 @@
if process.returncode:
print(f'[RESULT] Failed with return code {process.returncode}.')
print('[OUTPUT]')
- print(process.stdout.decode(errors="backslashreplace"))
+ print(process.stdout.decode(errors='backslashreplace'))
raise PresubmitFailure
