From 6ca1d93473a1c4633fd2dce4e2df0bb604e198d4 Mon Sep 17 00:00:00 2001 From: Michael Suo Date: Tue, 18 Dec 2018 22:31:51 -0800 Subject: [PATCH] add whitelisted clang-format checks (#15254) Summary: This PR adds clang-format automation: - It only checks on whitelisted files, so we can enable incrementally without noise - There is a pre-commit hook provided that will do the same check, plus prompt users to apply the clang-format changes (no change is made without the user agreeing). My plan is to migrate over whole files at a time, clang-formatting them and then adding them to the whitelist. Doing it this way should avoid too many merge pains (the most you'll have to is run clang-format on the affected file before rebasing). Pull Request resolved: https://github.com/pytorch/pytorch/pull/15254 Differential Revision: D13515888 Pulled By: suo fbshipit-source-id: d098eabcc97aa228c4dfce8fc096c3b5a45b591f --- .travis.yml | 3 +- tools/clang_format.py | 146 ++++++++++++++++++++++++++++++++++++++++++ tools/git-pre-commit | 2 + 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 tools/clang_format.py diff --git a/.travis.yml b/.travis.yml index e4c9bc575d1..ab07565d232 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,7 +28,7 @@ matrix: script: mypy @mypy-files.txt - env: CPP_DOC_CHECK python: "3.6" - install: + install: - sudo apt-get install -y doxygen - pip install -r requirements.txt script: cd docs/cpp/source && ./check-doxygen.sh @@ -41,3 +41,4 @@ matrix: - llvm-toolchain-trusty packages: clang-tidy script: tools/run-clang-tidy-in-ci.sh + diff --git a/tools/clang_format.py b/tools/clang_format.py new file mode 100644 index 00000000000..8b5669c4961 --- /dev/null +++ b/tools/clang_format.py @@ -0,0 +1,146 @@ +#!/usr/bin/env python +""" +A script that runs clang-format on changes detected via git. It will +report if running clang-format generated any changes. + +In CI, the script considers it a failure if running clang-format makes a change. +In the pre-commit hook, the user is prompted to apply any clang-format changes. +Running tools/clang_format.py manually with no arguments should replicate the pre-commit hook behavior. + +Only files that are in CLANG_FORMAT_WHITELIST are checked. +""" +import subprocess +import glob +import itertools +import os +import argparse +import difflib +import sys + + +# for python2 compatability +PY2 = sys.version_info[0] == 2 +if PY2: + + def input(foo): + return raw_input(foo) + + +# Whitelist of files to check. Takes a glob syntax. Does not support +# recursive globs ("**") because I am lazy and don't want to make that +# work with Python 2. +CLANG_FORMAT_WHITELIST = ["torch/csrc/jit/passes/alias_analysis*"] + + +def parse_args(): + parser = argparse.ArgumentParser( + description="Execute clang-format on your working copy changes." + ) + parser.add_argument( + "-d", + "--diff", + default="HEAD", + help="Git revision to diff against to get changes", + ) + parser.add_argument( + "--non-interactive", + action="store_true", + default=False, + help="Don't prompt the user to apply clang-format changes. If there are any changes, just exit non-zero", + ) + parser.add_argument("--verbose", "-v", action="store_true", default=False) + return parser.parse_args() + + +def get_whitelisted_files(): + """ + Parse CLANG_FORMAT_WHITELIST and resolve all globs. + Returns the set of all whitelisted filenames. + """ + paths = [glob.glob(entry) for entry in CLANG_FORMAT_WHITELIST] + # flatten the files list + paths = itertools.chain(*paths) + # filter out directories + filenames = filter(lambda path: os.path.isfile(path), paths) + return set(filenames) + + +def get_changed_files(rev): + """ + Get all changed files between the working tree and `rev` + """ + changed_files = ( + subprocess.check_output( + ["git", "diff-index", "--diff-filter=AMU", "--name-only", rev] + ) + .decode() + .split("\n") + ) + return set(changed_files) + + +def get_diffs(files): + """ + Run clang-format on all `files` and report if it changed anything. + Returns a mapping of filename => diff generator + """ + name_to_diffs = {} + for f in files: + formatted_text = subprocess.check_output(["clang-format", f]).decode() + with open(f) as orig: + orig_text = orig.read() + if formatted_text != orig_text: + orig_lines = orig_text.split("\n") + formatted_lines = formatted_text.split("\n") + diff = difflib.unified_diff( + orig_lines, formatted_lines, "original", "formatted" + ) + name_to_diffs[f] = diff + + return name_to_diffs + + +def main(): + args = parse_args() + + changed_files = get_changed_files(args.diff) + whitelisted_files = get_whitelisted_files() + + files_to_check = changed_files & whitelisted_files + + if args.verbose: + print("Running clang-format on whitelisted files: ") + for f in files_to_check: + print(f) + + name_to_diffs = get_diffs(files_to_check) + + if len(name_to_diffs) != 0: + print("ERROR: Running clang-format created changes: ") + for name, diff in name_to_diffs.items(): + print("In ", name) + for line in diff: + print(line) + print("\n") + + if args.non_interactive: + exit(1) + else: + choice = None + # Loop until we choose y or n + while choice is None: + choice = input("Accept these changes? [Y/n] ").lower() + if choice != "" and choice[0] != "y" and choice[0] != "n": + choice = None + + if choice == "" or choice[0] == "y": + # run clang-format on the necessary files + args = ["clang-format", "-i"] + args.extend(name_to_diffs.keys()) + subprocess.check_output(args) + else: + exit(1) + + +if __name__ == "__main__": + main() diff --git a/tools/git-pre-commit b/tools/git-pre-commit index c3ff3c82fc6..81983a4f773 100755 --- a/tools/git-pre-commit +++ b/tools/git-pre-commit @@ -10,3 +10,5 @@ python tools/clang_tidy.py \ -g"-torch/csrc/jit/export.cpp" \ -g"-torch/csrc/jit/import.cpp" \ -j +echo "Running pre-commit clang-format" +python tools/clang_format.py