Partial commit towards linediff #38

Makefile

@@ -5,7 +5,7 @@ clean:
 	rm -rf dist *.egg-info
 
 install:
-	rm ~/.local/bin/*-hook && pip3 install . --user
+	rm ~/.local/bin/*-hook || true && pip3 install . --user
 
 test: install
 	pytest -x -vvv --pdb

README.md

@@ -218,15 +218,28 @@ pre-commit run
 
 ### Special flags in this repo
 
-There are 2 flags, `--version` and `--no-diff` that can be added to `args:` for a pre-commit hook.
-They will be removed and not be passed on to the command.
+There are 3 flags, `--version`,  `--no-diff`, and `--line-diff` that can be
+added to `args:` for a pre-commit hook. They will be removed and not be passed
+on to the command.
 
-Some linters change behavior between versions. To enforce a linter version
+`--version`: Some linters change behavior between versions. To enforce a linter version
 8.0.0, for example, add `--version=8.0.0` to `args:` for that linter. Note that
 this is a pre-commit hook arg and will be filtered before args are passed to the linter.
 
-You can add `--no-diff` to the `args:` for clang-format and uncrustify
-if you would like there to be no diff output for these commands.
+`--no-diff`: You can add `--no-diff` to the `args:` for clang-format and uncrustify
+if you would like there to be no diff output for these commands. The maintainer of pre-commit
+is mostly against [having a quiet option](https://github.com/pre-commit/pre-commit/issues/823).
+
+`--line-diff`: Use this flag if you have a large codebase and only want to apply
+clang-format or clang-tidy to lines changed in this commit. It is adapted from
+and has the same functionality as [clang-format-diff.py](https://github.com/llvm/llvm-project/blob/main/clang/tools/clang-format/clang-format-diff.py)
+and [clang-tidy-diff.py](https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py).
+It currently only works with clang-format and clang-tidy because only those tools have a flag to specify line numbers to analyze.
+
+Quoting u/Supadoplex from reddit:
+
+> Those are great since they can reasonably be retrofitted to codebases that used to not have formatters or analysers,
+> while allowing their use without a single massive commit that changes formatting of entire files and fixes a billion warnings.
 
 ### Default Options
 
@@ -305,7 +318,6 @@ You can build all of these from source.
 | [cpplint](https://github.com/cpplint/cpplint)                            |  | `--verbose=0` |  |
 | [include-what-you-use](https://github.com/include-what-you-use/include-what-you-use) | | `--verbose=3` | |
 
-
 [1]: `-fix` will fail if there are compiler errors. `-fix-errors` will `-fix`
 and fix compiler errors if it can, like missing semicolons.
 

hooks/clang_format.py

@@ -16,21 +16,25 @@ def __init__(self, args: List[str]):
         super().__init__(self.command, self.lookbehind, args)
         self.check_installed()
         self.parse_args(args)
-        self.set_diff_flag()
+        self.set_line_diff_flag()
+        self.set_no_diff_flag()
+        self.set_no_diff_flag()
         self.edit_in_place = "-i" in self.args
 
     def run(self):
         """Run clang-format. Error if diff is incorrect."""
         for filename in self.files:
-            self.compare_to_formatted(filename)
-        if self.returncode != 0:
-            sys.stdout.buffer.write(self.stderr)
-            sys.exit(self.returncode)
+            options: List[str] = []
+            if self.line_diff_flag and filename in self.changed_lines:
+                for range in self.changed_lines[filename]:
+                    options += ["-lines", f"{range[0]}:{range[1]}"]
+            self.compare_to_formatted(filename, options)
 
 
 def main(argv: List[str] = sys.argv):
     cmd = ClangFormatCmd(argv)
     cmd.run()
+    cmd.exit_on_error()
 
 
 if __name__ == "__main__":

hooks/clang_tidy.py

@@ -2,6 +2,7 @@
 """Wrapper script for clang-tidy."""
 import re
 import sys
+import json
 from typing import List
 
 from hooks.utils import StaticAnalyzerCmd
@@ -16,23 +17,30 @@ class ClangTidyCmd(StaticAnalyzerCmd):
     def __init__(self, args: List[str]):
         super().__init__(self.command, self.lookbehind, args)
         self.parse_args(args)
+        self.set_line_diff_flag()
         self.edit_in_place = "-fix" in self.args or "--fix-errors" in self.args
 
     def run(self):
         """Run clang-tidy. If --fix-errors is passed in, then return code will be 0, even if there are errors."""
         for filename in self.files:
-            self.run_command([filename] + self.args)
+            line_diff_arg = []
+            if self.line_diff_flag:
+                # clang-tidy help gives this JSON as reference: [{"name":"file1.cpp","lines":[[1,3],[5,7]]},{"name":"file2.h"}]
+                line_diff_prefix = "-line-filter="
+                ct_json_changed_lines = [{"name": name, "lines": self.changed_lines[name]} for name in self.changed_lines]
+                line_diff_arg = [line_diff_prefix + json.dumps(ct_json_changed_lines)]
+
+            self.run_command([filename] + self.args + line_diff_arg)
             # Warnings generated aren't important.
             self.stderr = re.sub(rb"[\d,]+ warning \S+\s+", b"", self.stderr)
             if len(self.stderr) > 0 and "--fix-errors" in self.args:
                 self.returncode = 1
-            self.exit_on_error()
 
 
 def main(argv: List[str] = sys.argv):
     cmd = ClangTidyCmd(argv)
     cmd.run()
-
+    cmd.exit_on_error()
 
 if __name__ == "__main__":
     main()

hooks/cppcheck.py

@@ -30,12 +30,12 @@ def run(self):
         """Run cppcheck"""
         for filename in self.files:
             self.run_command([filename] + self.args)
-            self.exit_on_error()
 
 
 def main(argv: List[str] = sys.argv):
     cmd = CppcheckCmd(argv)
     cmd.run()
+    cmd.exit_on_error()
 
 
 if __name__ == "__main__":

hooks/cpplint.py

@@ -21,13 +21,12 @@ def run(self):
         """Run cpplint"""
         for filename in self.files:
             self.run_command(self.args + [filename])  # cpplint is unique in requiring args before filename
-            self.exit_on_error()
 
 
 def main(argv: List[str] = sys.argv):
     cmd = CpplintCmd(argv)
     cmd.run()
-
+    cmd.exit_on_error()
 
 if __name__ == "__main__":
     main()

hooks/include_what_you_use.py

@@ -24,19 +24,15 @@ def run(self):
         for filename in self.files:
             self.run_command([filename] + self.args)
             is_correct = b"has correct #includes/fwd-decls" in self.stderr
-            if is_correct:
-                self.returncode = 0
-                self.stdout = b""
-                self.stderr = b""
-            else:
-                sys.stderr.buffer.write(self.stdout + self.stderr)
-                sys.exit(self.returncode)
+            if not is_correct:
+                self.returncode = 1
+                self.stderr = self.stdout + self.stderr
 
 
 def main(argv: List[str] = sys.argv):
     cmd = IncludeWhatYouUseCmd(argv)
     cmd.run()
-
+    cmd.exit_on_error()
 
 if __name__ == "__main__":
     main()

hooks/line_diff.py

@@ -0,0 +1,62 @@
+#!/usr/bin/env python3
+"""Adapted for from LLVM's clang-format-diff.py found here:
+
+https://github.com/llvm/llvm-project/blob/main/clang/tools/clang-format/clang-format-diff.py
+
+Logic behind `--line-diff` flag: Gets git diff of most recent commit to get the
+lines that have changed and the files they were changed in.
+"""
+
+import re
+import subprocess as sp
+from typing import Any, Dict, List
+
+TypeLineNumsToChange = List[List[int]]
+TypeFilesToChange = Dict[str, TypeLineNumsToChange]
+
+
+def get_changed_lines():
+    """Extract changed lines for each file."""
+    diff_lines_str = get_added_diff_text()
+    diff_lines_json = parse_diff_lines(diff_lines_str)
+    return diff_lines_json
+
+def check_errors(program: List[str], child: Any):
+    if child.stderr or child.returncode != 0:
+        raise RuntimeError(f"""{program} failed due to a nonzero return code or stderr.
+returncode: {child.returncode}\nstdout: `{child.stdout}`\nstderr: `{child.stderr}`""")
+
+def get_added_diff_text() -> str:
+    """Return a git diff as a list of lines altered since last commit.
+    Compare to clang-format-diff, which does a post-commit comparison"""
+    command = ["git", "diff", "HEAD"]
+    child = sp.run(command, stdout=sp.PIPE, stderr=sp.PIPE)
+    check_errors(command, child)
+    return child.stdout.decode()
+
+def parse_diff_lines(diff_lines_str: str) -> TypeFilesToChange:
+    changed_lines: TypeFilesToChange = {}  # {filename: [[start_line, end_line], ...], ...}
+    filename = ''
+    diff_lines = [line + '\n' for line in diff_lines_str.split('\n')[:-1]]
+
+    # Keeping regex logic intact as the LLVM has already done testing with it
+    for line in diff_lines:
+        match = re.search(r'^\+\+\+\ b/(.+)', line)
+        if match:
+            filename = match.group(1)
+        match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
+        if match:
+            start_line = int(match.group(1))
+            line_count = 1
+            if match.group(3):
+                line_count = int(match.group(3))
+            # Also format lines range if line_count is 0 in case of deleting
+            # surrounding statements.
+            end_line = start_line
+            if line_count != 0:
+                end_line += line_count - 1
+            if filename not in changed_lines:
+                changed_lines[filename] = []
+            changed_lines[filename].append([start_line, end_line])
+
+    return changed_lines

hooks/oclint.py

@@ -42,7 +42,6 @@ def run(self):
                 self.stderr = self.stdout
             # If errors have been captured, stdout is unexpected
             self.stdout = b""
-            self.exit_on_error()
             self.cleanup_files(current_files)
 
     @staticmethod
@@ -57,7 +56,7 @@ def cleanup_files(existing_files: List[str]):
 def main(argv: List[str] = sys.argv):
     cmd = OCLintCmd(argv)
     cmd.run()
-
+    cmd.exit_on_error()
 
 if __name__ == "__main__":
     main()

hooks/uncrustify.py

@@ -19,7 +19,8 @@ def __init__(self, args: List[str]):
         super().__init__(self.command, self.lookbehind, args)
         self.check_installed()
         self.parse_args(args)
-        self.set_diff_flag()
+        self.set_no_diff_flag()
+        self.set_no_diff_flag()
         self.add_if_missing(["-q"])  # Remove stderr, which causes issues
         self.file_flag = "-f"
         self.edit_in_place = "--replace" in self.args
@@ -44,15 +45,13 @@ def fix_defaults():
     def run(self):
         """Run uncrustify with the arguments provided."""
         for filename in self.files:
-            self.compare_to_formatted(filename)
-        if self.returncode != 0:
-            sys.stdout.buffer.write(self.stderr)
-            sys.exit(self.returncode)
-
+            options: List[str] = []  # For line diffs at some point
+            self.compare_to_formatted(filename, options)
 
 def main(argv: List[str] = sys.argv):
     cmd = UncrustifyCmd(argv)
     cmd.run()
+    cmd.exit_on_error()
 
 
 if __name__ == "__main__":

hooks/utils.py

@@ -8,6 +8,8 @@
 import sys
 from typing import List
 
+from hooks.line_diff import get_changed_lines
+
 
 class Command:
     """Super class that all commands inherit"""
@@ -24,6 +26,12 @@ def __init__(self, command: str, look_behind: str, args: List[str]):
         self.stderr = b""
         self.returncode = 0
 
+    def set_line_diff_flag(self):
+        self.line_diff_flag = "--line-diff" in self.args
+        if self.line_diff_flag:
+            self.changed_lines = get_changed_lines()
+            self.args.remove("--line-diff")
+
     def check_installed(self):
         """Check if command is installed and fail exit if not."""
         path = shutil.which(self.command)
@@ -115,18 +123,23 @@ def raise_error(self, problem: str, details: str):
 
     def get_version_str(self):
         """Get the version string like 8.0.0 for a given command."""
+        def version_has_changed_error():
+            details = """The version format for this command has changed.
+Create an issue at github.com/pocc/pre-commit-hooks."""
+            self.raise_error("getting version", details)
+
         args = [self.command, "--version"]
         sp_child = sp.run(args, stdout=sp.PIPE, stderr=sp.PIPE)
         version_str = str(sp_child.stdout, encoding="utf-8")
         # After version like `8.0.0` is expected to be '\n' or ' '
+        if not re.search(self.look_behind, version_str):
+            version_has_changed_error()
         regex = self.look_behind + r"((?:\d+\.)+[\d+_\+\-a-z]+)"
-        search = re.search(regex, version_str)
-        if not search:
-            details = """The version format for this command has changed.
-Create an issue at github.com/pocc/pre-commit-hooks."""
-            self.raise_error("getting version", details)
-        version = search.group(1)
-        return version
+        version_matches = re.search(regex, version_str)
+        if version_matches:
+            return version_matches.group(1)
+        else:
+            version_has_changed_error()
 
 
 class StaticAnalyzerCmd(Command):
@@ -156,17 +169,17 @@ def __init__(self, command: str, look_behind: str, args: List[str]):
         super().__init__(command, look_behind, args)
         self.file_flag = None
 
-    def set_diff_flag(self):
+    def set_no_diff_flag(self):
         self.no_diff_flag = "--no-diff" in self.args
         if self.no_diff_flag:
             self.args.remove("--no-diff")
 
-    def compare_to_formatted(self, filename_str: str) -> None:
+    def compare_to_formatted(self, filename_str: str, options: List[str]) -> None:
         """Compare the expected formatted output to file contents."""
         # This string encode is from argparse, so we should be able to trust it.
         filename = filename_str.encode()
         actual = self.get_filelines(filename_str)
-        expected = self.get_formatted_lines(filename_str)
+        expected = self.get_formatted_lines(filename_str, options)
         if self.edit_in_place:
             # If edit in place is used, the formatter will fix in place with
             # no stdout. So compare the before/after file for hook pass/fail
@@ -186,10 +199,10 @@ def get_filename_opts(self, filename: str):
             return [self.file_flag, filename]
         return [filename]
 
-    def get_formatted_lines(self, filename: str) -> List[bytes]:
+    def get_formatted_lines(self, filename: str, options: List[str]) -> List[bytes]:
         """Get the expected output for a command applied to a file."""
         filename_opts = self.get_filename_opts(filename)
-        args = [self.command, *self.args, *filename_opts]
+        args = [self.command, *self.args, *options, *filename_opts]
         child = sp.run(args, stdout=sp.PIPE, stderr=sp.PIPE)
         if len(child.stderr) > 0 or child.returncode != 0:
             problem = f"Unexpected Stderr/return code received when analyzing {filename}.\nArgs: {args}"
@@ -205,3 +218,10 @@ def get_filelines(self, filename: str):
         with open(filename, "rb") as f:
             filetext = f.read()
         return filetext.split(b"\x0a")
+
+    def exit_on_error(self):
+        """Exit on error. Note that this is different from Static Analyzer exit_on_erro.
+        This one only writes stderr to stdout whereas other fn writes stdout+stderr => stdout"""
+        if self.returncode != 0:
+            sys.stdout.buffer.write(self.stderr)
+            sys.exit(self.returncode)