Merge pull request #1710 from haytham918/clang-tidy-feature

Add and Test Clang-tidy
SFTtech · Feb 9, 2025 · 0ce621f · 0ce621f
2 parents fb36b9d + 8debe0c
commit 0ce621f
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 16 deletions.
diff --git a/Makefile b/Makefile
@@ -128,22 +128,26 @@ mrproperer: mrproper
 checkfast:
 	python3 -m buildsystem.codecompliance --fast
 
-.PHONY: checkall
-checkall:
-	python3 -m buildsystem.codecompliance --all
+.PHONY: checkmerge
+checkmerge:
+	python3 -m buildsystem.codecompliance --merge
 
 .PHONY: checkchanged
 checkchanged:
-	python3 -m buildsystem.codecompliance --all --only-changed-files=origin/master
+	python3 -m buildsystem.codecompliance --merge --only-changed-files=origin/master
 
 .PHONY: checkuncommited
 checkuncommited:
-	python3 -m buildsystem.codecompliance --all --only-changed-files=HEAD
+	python3 -m buildsystem.codecompliance --merge --only-changed-files=HEAD
 
 .PHONY: checkpy
 checkpy:
 	python3 -m buildsystem.codecompliance --pystyle --pylint
 
+.PHONY: checkall
+checkall:
+	python3 -m buildsystem.codecompliance --all
+
 .PHONY: help
 help: $(BUILDDIR)/Makefile
 	@echo "openage Makefile"

diff --git a/buildsystem/codecompliance/__main__.py b/buildsystem/codecompliance/__main__.py
@@ -1,4 +1,4 @@
-# Copyright 2014-2024 the openage authors. See copying.md for legal info.
+# Copyright 2014-2025 the openage authors. See copying.md for legal info.
 
 """
 Entry point for the code compliance checker.
@@ -18,16 +18,24 @@ def parse_args():
     """ Returns the raw argument namespace. """
 
     cli = argparse.ArgumentParser()
-    cli.add_argument("--fast", action="store_true",
-                     help="do all checks that can be performed quickly")
-    cli.add_argument("--all", action="store_true",
-                     help="do all checks, even the really slow ones")
+    check_types = cli.add_mutually_exclusive_group()
+    check_types.add_argument("--fast", action="store_true",
+                             help="do all checks that can be performed quickly")
+    check_types.add_argument("--merge", action="store_true",
+                             help="do all checks that are required before merges to master")
+    check_types.add_argument("--all", action="store_true",
+                             help="do all checks, even the really slow ones")
+
     cli.add_argument("--only-changed-files", metavar='GITREF',
                      help=("slow checks are only done on files that have "
                            "changed since GITREF."))
     cli.add_argument("--authors", action="store_true",
                      help=("check whether all git authors are in copying.md. "
                            "repo must be a git repository."))
+    cli.add_argument("--clang-tidy", action="store_true",
+                     help=("Check the C++ code with clang-tidy. Make sure you have build the "
+                           "project with ./configure --clang-tidy or have set "
+                           "CMAKE_CXX_CLANG_TIDY for your CMake build."))
     cli.add_argument("--cppstyle", action="store_true",
                      help="check the cpp code style")
     cli.add_argument("--cython", action="store_true",
@@ -76,7 +84,7 @@ def process_args(args, error):
     # set up log level
     log_setup(args.verbose - args.quiet)
 
-    if args.fast or args.all:
+    if args.fast or args.merge or args.all:
         # enable "fast" tests
         args.authors = True
         args.cppstyle = True
@@ -86,16 +94,19 @@ def process_args(args, error):
         args.filemodes = True
         args.textfiles = True
 
-    if args.all:
-        # enable tests that take a bit longer
-
+    if args.merge or args.all:
+        # enable tests that are required before merging to master
         args.pystyle = True
         args.pylint = True
         args.test_git_change_years = True
 
+    if args.all:
+        # enable tests that take a bit longer
+        args.clang_tidy = True
+
     if not any((args.headerguards, args.legal, args.authors, args.pystyle,
                 args.cppstyle, args.cython, args.test_git_change_years,
-                args.pylint, args.filemodes, args.textfiles)):
+                args.pylint, args.filemodes, args.textfiles, args.clang_tidy)):
         error("no checks were specified")
 
     has_git = bool(shutil.which('git'))
@@ -128,6 +139,10 @@ def process_args(args, error):
         if not importlib.util.find_spec('pylint'):
             error("pylint python module required for linting")
 
+    if args.clang_tidy:
+        if not shutil.which('clang-tidy'):
+            error("--clang-tidy requires clang-tidy to be installed")
+
 
 def get_changed_files(gitref):
     """
@@ -264,6 +279,9 @@ def find_all_issues(args, check_files=None):
         from .modes import find_issues
         yield from find_issues(check_files, ('openage', 'buildsystem',
                                              'libopenage', 'etc/gdb_pretty'))
+    if args.clang_tidy:
+        from .clangtidy import find_issues
+        yield from find_issues(check_files, ('libopenage', ))
 
 
 if __name__ == '__main__':

diff --git a/buildsystem/codecompliance/clangtidy.py b/buildsystem/codecompliance/clangtidy.py
@@ -0,0 +1,69 @@
+# Copyright 2024-2024 the openage authors. See copying.md for legal info.
+
+"""
+Checks clang-tidy errors on cpp files
+"""
+
+import subprocess
+from .cppstyle import filter_file_list
+from .util import findfiles
+
+
+def find_issues(check_files, dirnames):
+    """
+    Invoke clang-tidy to check C++ files for issues.
+    Yields issues found by clang-tidy in real-time.
+    """
+    # Specify the checks to include
+    # 4 checks we focus on
+    checks_to_include = [
+        'clang-analyzer-*',
+        'bugprone-*',
+        'concurrency-*',
+        'performance-*'
+    ]
+    # Create the checks string
+    checks = ', '.join(checks_to_include)
+
+    # Invocation command
+    invocation = ['clang-tidy', f'-checks=-*,{checks}']
+
+    # Use utility functions from util.py and cppstyle.py
+    if check_files is not None:
+        filenames = list(filter_file_list(check_files, dirnames))
+    else:
+        filenames = list(filter_file_list(findfiles(dirnames), dirnames))
+
+    if not filenames:
+        print("No files to check.")
+        return  # No files to check
+
+    for filename in filenames:
+        # Run clang-tidy for each file
+        print(f"Starting clang-tidy check on file: {filename}")
+        try:
+            with subprocess.Popen(
+                invocation + [filename],
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                text=True
+            ) as process:
+                # Stream output in real-time
+                while True:
+                    output = process.stdout.readline()
+                    if output:
+                        yield ("clang-tidy output", output.strip(), None)
+                    elif process.poll() is not None:
+                        break
+
+                # Capture remaining errors (if any)
+                for error_line in process.stderr:
+                    yield ("clang-tidy error", error_line.strip(), None)
+
+        # Handle exception
+        except subprocess.SubprocessError as exc:
+            yield (
+                "clang-tidy error",
+                f"An error occurred while running clang-tidy on {filename}: {str(exc)}",
+                None
+            )
diff --git a/copying.md b/copying.md
@@ -161,6 +161,7 @@ _the openage authors_ are:
 | David Wever                 | dmwever                     | dmwever à crimson dawt ua dawt edu                |
 | Michael Lynch               | mtlynch                     | git à mtlynch dawt io                             |
 | Ngô Xuân Minh               |                             | xminh dawt ngo dawt 00 à gmail dawt com           |
+| Haytham Tang                | haytham918                  | yunxuant à umich dawt edu                         |
 
 If you're a first-time committer, add yourself to the above list. This is not
 just for legal reasons, but also to keep an overview of all those nicknames.

diff --git a/doc/building.md b/doc/building.md
@@ -39,6 +39,7 @@ Dependency list:
     CR    opengl >=3.3
     CR    libepoxy
     CR    libpng
+       S  clang-tidy
      R    dejavu font
     CR    eigen >=3
     CR    freetype2

diff --git a/kevinfile b/kevinfile
@@ -6,7 +6,7 @@
 
 sanity_check:
 	- skip              (? if job != "debian" ?)
-	make checkall
+	make checkmerge
 
 # Various optimisation options can affect warnings compiler generates.
 # Make sure both release and debug are tested. Arch job has more checks,