Enable local clang-tidy lint (#61121)

Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/61121

This change enables the make target to run clang-tidy locally

Test Plan:
Run this command
```
make clang-tidy
```
This should run `clang-tidy` on the paths and filters specified in `tools/linter/clang_tidy/__main__.py`

Quicklint
```
make quicklint
```
This should report "No files detected" if no c/cpp files are altered.

Reviewed By: soulitzer

Differential Revision: D29598927

Pulled By: 1ntEgr8

fbshipit-source-id: aa443030494fed92c313da4b203a5450be09fa38

Makefile

@@ -71,6 +71,7 @@ setup_lint:
 	fi
 	pip install jinja2
 	pip install -r tools/linter/clang_tidy/requirements.txt
+	$(PYTHON) -m tools.linter.install.clang_tidy
 
 quick_checks:
 # TODO: This is broken when 'git config submodule.recurse' is 'true' since the
@@ -104,9 +105,10 @@ cmakelint:
 		--job 'cmakelint' \
 		--step 'Run cmakelint'
 
-clang_tidy:
+clang-tidy:
-	echo "clang-tidy local lint is not yet implemented"
+	@$(PYTHON) tools/actions_local_runner.py \
-	exit 1
+		$(CHANGED_ONLY) \
+		--job 'clang-tidy'
 
 toc:
 	@$(PYTHON) tools/actions_local_runner.py \
@@ -117,4 +119,4 @@ toc:
 lint: flake8 mypy quick_checks cmakelint shellcheck
 
 quicklint: CHANGED_ONLY=--changed-only
-quicklint: mypy flake8 quick_checks cmakelint shellcheck
+quicklint: mypy flake8 quick_checks cmakelint shellcheck clang-tidy

tools/actions_local_runner.py

@@ -270,7 +270,8 @@ class ShellCheck(Check):
 
     async def quick(self, files: List[str]) -> CommandResult:
         return await shell_cmd(
-            ["tools/linter/run_shellcheck.sh"] + [os.path.join(REPO_ROOT, f) for f in files],
+            ["tools/linter/run_shellcheck.sh"]
+            + [os.path.join(REPO_ROOT, f) for f in files],
         )
 
     async def full(self) -> None:
@@ -289,6 +290,30 @@ class ShellCheck(Check):
         )
 
 
+class ClangTidy(Check):
+    name = "clang-tidy: Run clang-tidy"
+    common_options = [
+        "--clang-tidy-exe",
+        ".clang-tidy-bin/clang-tidy",
+        "--parallel",
+    ]
+
+    def filter_files(self, files: List[str]) -> List[str]:
+        return self.filter_ext(files, {".c", ".cc", ".cpp"})
+
+    async def quick(self, files: List[str]) -> CommandResult:
+        return await shell_cmd(
+            [sys.executable, "tools/linter/clang_tidy", "--paths"]
+            + [os.path.join(REPO_ROOT, f) for f in files]
+            + self.common_options,
+        )
+
+    async def full(self) -> None:
+        await shell_cmd(
+            [sys.executable, "tools/linter/clang_tidy"] + self.common_options
+        )
+
+
 class YamlStep(Check):
     def __init__(self, step: Dict[str, Any], job_name: str, quiet: bool):
         super().__init__(files=None, quiet=quiet)
@@ -405,6 +430,7 @@ ad_hoc_steps = {
     "mypy": Mypy,
     "flake8-py3": Flake8,
     "shellcheck": ShellCheck,
+    "clang-tidy": ClangTidy,
 }
 
 if __name__ == "__main__":

tools/linter/clang_tidy/__main__.py

@@ -1,10 +1,52 @@
 import argparse
 import pathlib
+import os
+import shutil
+import subprocess
+import re
+from typing import List
+
 
 from run import run
 from generate_build_files import generate_build_files
 
 
+def clang_search_dirs() -> List[str]:
+    # Compilers are ordered based on fallback preference
+    # We pick the first one that is available on the system
+    compilers = ["clang", "gcc", "cpp", "cc"]
+    compilers = [c for c in compilers if shutil.which(c) is not None]
+    if len(compilers) == 0:
+        raise RuntimeError(f"None of {compilers} were found")
+    compiler = compilers[0]
+
+    result = subprocess.run(
+        [compiler, "-E", "-x", "c++", "-", "-v"],
+        stdin=subprocess.DEVNULL,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        check=True,
+    )
+    stderr = result.stderr.decode().strip().split("\n")
+    search_start = r"#include.*search starts here:"
+    search_end = r"End of search list."
+
+    append_path = False
+    search_paths = []
+    for line in stderr:
+        if re.match(search_start, line):
+            if append_path:
+                continue
+            else:
+                append_path = True
+        elif re.match(search_end, line):
+            break
+        elif append_path:
+            search_paths.append(line.strip())
+
+    return search_paths
+
+
 DEFAULTS = {
     "glob": [
         # The negative filters below are to exclude files that include onnx_pb.h or
@@ -32,7 +74,7 @@ DEFAULTS = {
         "-torch/csrc/deploy/interpreter/test_main.cpp",
     ],
     "paths": ["torch/csrc/"],
-    "include-dir": ["/usr/lib/llvm-11/include/openmp"],
+    "include-dir": ["/usr/lib/llvm-11/include/openmp"] + clang_search_dirs(),
 }
 
 
@@ -68,7 +110,8 @@ def parse_args() -> argparse.Namespace:
         help="Path to the folder containing compile_commands.json",
     )
     parser.add_argument(
-        "--diff-file", help="File containing diff to use for determining files to lint and line filters"
+        "--diff-file",
+        help="File containing diff to use for determining files to lint and line filters",
     )
     parser.add_argument(
         "-p",
@@ -103,7 +146,7 @@ def parse_args() -> argparse.Namespace:
     parser.add_argument(
         "--print-include-paths",
         action="store_true",
-        help="Print the search paths used for include directives"
+        help="Print the search paths used for include directives",
     )
     parser.add_argument(
         "-I",
@@ -112,8 +155,12 @@ def parse_args() -> argparse.Namespace:
         default=DEFAULTS["include-dir"],
         help="Add the specified directory to the search path for include files",
     )
-    parser.add_argument("-s", "--suppress-diagnostics", action="store_true",
+    parser.add_argument(
-                        help="Add NOLINT to suppress clang-tidy violations")
+        "-s",
+        "--suppress-diagnostics",
+        action="store_true",
+        help="Add NOLINT to suppress clang-tidy violations",
+    )
     parser.add_argument(
         "extra_args", nargs="*", help="Extra arguments to forward to clang-tidy"
     )
@@ -124,7 +171,23 @@ def main() -> None:
     if not pathlib.Path("build").exists():
         generate_build_files()
     options = parse_args()
-    run(options)
+
+    # Check if clang-tidy executable exists
+    exists = os.access(options.clang_tidy_exe, os.X_OK) or shutil.which(
+        options.clang_tidy_exe
+    )
+    if not exists:
+        msg = (
+            "Could not find 'clang-tidy' binary\n"
+            "You can install it by running:\n"
+            "   python3 tools/linter/install/clang_tidy.py"
+        )
+        print(msg)
+        exit(1)
+
+    return_code = run(options)
+    if return_code != 0:
+        raise RuntimeError("Warnings found in clang-tidy output!")
 
 
 main()

tools/linter/clang_tidy/run.py

@@ -13,7 +13,6 @@ glob or regular expressions.
 """
 
 
-
 import collections
 import fnmatch
 import json
@@ -52,7 +51,9 @@ def map_filename(build_folder: str, fname: str) -> str:
     build_cpu_prefix = os.path.join(build_folder, native_cpu_prefix, "")
     default_arch_suffix = ".DEFAULT.cpp"
     if fname.startswith(native_cpu_prefix) and fname.endswith(".cpp"):
-        return f"{build_cpu_prefix}{fname[len(native_cpu_prefix):]}{default_arch_suffix}"
+        return (
+            f"{build_cpu_prefix}{fname[len(native_cpu_prefix):]}{default_arch_suffix}"
+        )
     if fname.startswith(build_cpu_prefix) and fname.endswith(default_arch_suffix):
         return f"{native_cpu_prefix}{fname[len(build_cpu_prefix):-len(default_arch_suffix)]}"
     return fname
@@ -165,11 +166,12 @@ build {i}: do_cmd
 
 def run_shell_commands_in_parallel(commands: Iterable[List[str]]) -> str:
     """runs all the commands in parallel with ninja, commands is a List[List[str]]"""
+
     async def run_command(cmd: List[str]) -> str:
         proc = await asyncio.create_subprocess_shell(
-            ' '.join(shlex.quote(x) for x in cmd),  # type: ignore[attr-defined]
+            " ".join(shlex.quote(x) for x in cmd),  # type: ignore[attr-defined]
             stdout=asyncio.subprocess.PIPE,
-            stderr=asyncio.subprocess.PIPE
+            stderr=asyncio.subprocess.PIPE,
         )
         stdout, stderr = await proc.communicate()
         return f">>>\nstdout:\n{stdout.decode()}\nstderr:\n{stderr.decode()}\n<<<"
@@ -181,7 +183,9 @@ def run_shell_commands_in_parallel(commands: Iterable[List[str]]) -> str:
             async with semaphore:
                 return await task
 
-        return await asyncio.gather(*(sem_task(task) for task in tasks), return_exceptions=True)
+        return await asyncio.gather(
+            *(sem_task(task) for task in tasks), return_exceptions=True
+        )
 
     async def helper() -> Any:
         coros = [run_command(cmd) for cmd in commands]
@@ -192,7 +196,9 @@ def run_shell_commands_in_parallel(commands: Iterable[List[str]]) -> str:
     return "\n".join(results)
 
 
-def run_clang_tidy(options: Any, line_filters: List[Dict[str, Any]], files: Iterable[str]) -> str:
+def run_clang_tidy(
+    options: Any, line_filters: List[Dict[str, Any]], files: Iterable[str]
+) -> str:
     """Executes the actual clang-tidy command in the shell."""
     command = [options.clang_tidy_exe, "-p", options.compile_commands_dir]
     if not options.config_file and os.path.exists(".clang-tidy"):
@@ -202,7 +208,10 @@ def run_clang_tidy(options: Any, line_filters: List[Dict[str, Any]], files: Iter
 
         with open(options.config_file) as config:
             # Here we convert the YAML config file to a JSON blob.
-            command += ["-config", json.dumps(yaml.load(config, Loader=yaml.SafeLoader))]
+            command += [
+                "-config",
+                json.dumps(yaml.load(config, Loader=yaml.SafeLoader)),
+            ]
     if options.print_include_paths:
         command += ["--extra-arg", "-v"]
     if options.include_dir:
@@ -215,7 +224,10 @@ def run_clang_tidy(options: Any, line_filters: List[Dict[str, Any]], files: Iter
         command += ["-line-filter", json.dumps(line_filters)]
 
     if options.parallel:
-        commands = [list(command) + [map_filename(options.compile_commands_dir, f)] for f in files]
+        commands = [
+            list(command) + [map_filename(options.compile_commands_dir, f)]
+            for f in files
+        ]
         output = run_shell_commands_in_parallel(commands)
     else:
         command += map_filenames(options.compile_commands_dir, files)
@@ -232,7 +244,9 @@ def run_clang_tidy(options: Any, line_filters: List[Dict[str, Any]], files: Iter
     return output
 
 
-def extract_warnings(output: str, base_dir: str = ".") -> Dict[str, Dict[int, Set[str]]]:
+def extract_warnings(
+    output: str, base_dir: str = "."
+) -> Dict[str, Dict[int, Set[str]]]:
     rc: Dict[str, Dict[int, Set[str]]] = {}
     for line in output.split("\n"):
         p = CLANG_WARNING_PATTERN.match(line)
@@ -258,17 +272,50 @@ def apply_nolint(fname: str, warnings: Dict[int, Set[str]]) -> None:
 
     line_offset = -1  # As in .cpp files lines are numbered starting from 1
     for line_no in sorted(warnings.keys()):
-        nolint_diagnostics = ','.join(warnings[line_no])
+        nolint_diagnostics = ",".join(warnings[line_no])
         line_no += line_offset
-        indent = ' ' * (len(lines[line_no]) - len(lines[line_no].lstrip(' ')))
+        indent = " " * (len(lines[line_no]) - len(lines[line_no].lstrip(" ")))
-        lines.insert(line_no, f'{indent}// NOLINTNEXTLINE({nolint_diagnostics})\n')
+        lines.insert(line_no, f"{indent}// NOLINTNEXTLINE({nolint_diagnostics})\n")
         line_offset += 1
 
     with open(fname, mode="w") as f:
         f.write("".join(lines))
 
 
-def run(options: Any) -> None:
+def filter_from_diff(
+    paths: List[str], diffs: List[str]
+) -> Tuple[List[str], List[Dict[Any, Any]]]:
+    files = []
+    line_filters = []
+
+    for diff in diffs:
+        changed_files = find_changed_lines(diff)
+        changed_files = {
+            filename: v
+            for filename, v in changed_files.items()
+            if any(filename.startswith(path) for path in paths)
+        }
+        line_filters += [
+            {"name": name, "lines": lines} for name, lines, in changed_files.items()
+        ]
+        files += list(changed_files.keys())
+
+    return files, line_filters
+
+
+def filter_from_diff_file(
+    paths: List[str], filename: str
+) -> Tuple[List[str], List[Dict[Any, Any]]]:
+    with open(filename, "r") as f:
+        diff = f.read()
+    return filter_from_diff(paths, [diff])
+
+
+def filter_default(paths: List[str]) -> Tuple[List[str], List[Dict[Any, Any]]]:
+    return get_all_files(paths), []
+
+
+def run(options: Any) -> int:
     # This flag is pervasive enough to set it globally. It makes the code
     # cleaner compared to threading it through every single function.
     global VERBOSE
@@ -276,25 +323,12 @@ def run(options: Any) -> None:
 
     # Normalize the paths first.
     paths = [path.rstrip("/") for path in options.paths]
+
     if options.diff_file:
-        with open(options.diff_file, "r") as f:
+        files, line_filters = filter_from_diff_file(options.paths, options.diff_file)
-            changed_files = find_changed_lines(f.read())
-            changed_files = {
-                filename: v
-                for filename, v in changed_files.items()
-                if any(filename.startswith(path) for path in options.paths)
-            }
-            line_filters = [
-                {"name": name, "lines": lines} for name, lines, in changed_files.items()
-            ]
-            files = list(changed_files.keys())
-            # Since header files are excluded, add .cpp file if it exists in the same folder
-            cpp_files = [f[:-1] + "cpp" for f in files if f.endswith(".h")]
-            cpp_files = [f for f in cpp_files if os.path.exists(f)]
-            files = list(set(files + cpp_files))
     else:
-        line_filters = []
+        files, line_filters = filter_default(options.paths)
-        files = get_all_files(paths)
+
     file_patterns = get_file_patterns(options.glob, options.regex)
     files = list(filter_files(files, file_patterns))
 
@@ -304,8 +338,13 @@ def run(options: Any) -> None:
         sys.exit()
 
     clang_tidy_output = run_clang_tidy(options, line_filters, files)
+    warnings = extract_warnings(
+        clang_tidy_output, base_dir=options.compile_commands_dir
+    )
     if options.suppress_diagnostics:
-        warnings = extract_warnings(clang_tidy_output, base_dir=options.compile_commands_dir)
+        warnings = extract_warnings(
+            clang_tidy_output, base_dir=options.compile_commands_dir
+        )
         for fname in warnings.keys():
             mapped_fname = map_filename(options.compile_commands_dir, fname)
             print(f"Applying fixes to {mapped_fname}")
@@ -318,4 +357,6 @@ def run(options: Any) -> None:
         print(clang_tidy_output)
     for line in clang_tidy_output.splitlines():
         if line.startswith(pwd):
-            print(line[len(pwd):])
+            print(line[len(pwd) :])
+
+    return len(warnings.keys())