utils: checkstyle: Add support for clang-format

clang-format produces better results than astyle as it can better match
the libcamera coding style. Default to clang-format over astyle, fall
back to astyle if clang-format isn't found, and add a --formatter
command line option to select a formatter manually.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Documentation/coding-style.rst

@@ -170,8 +170,11 @@ These rules match the `object ownership rules from the Chromium C++ Style Guide`
 Tools
 -----
 
-The 'astyle' code formatting tool can be used to reformat source files with the
+The 'clang-format' code formatting tool can be used to reformat source files
-libcamera coding style, defined by the following arguments.
+with the libcamera coding style, defined in the .clang-format file at the root
+of the source tree.
+
+Alternatively the 'astyle' tool can also be used, with the following arguments.
 
 ::
 
@@ -184,11 +187,15 @@ libcamera coding style, defined by the following arguments.
   --align-reference=name
   --max-code-length=120
 
-As astyle is a code formatter, it operates on full files outputs reformatted
+Use of astyle is discouraged as clang-format better matches the libcamera coding
-source code. While it can be used to reformat code before sending patches, it
+style.
-may generate unrelated changes. To avoid this, libcamera provides a
+
-'checkstyle.py' script wrapping astyle to only retain related changes. This
+As both astyle and clang-format are code formatters, they operate on full files
-should be used to validate modifications before submitting them for review.
+and output reformatted source code. While they can be used to reformat code
+before sending patches, it may generate unrelated changes. To avoid this,
+libcamera provides a 'checkstyle.py' script wrapping the formatting tools to
+only retain related changes. This should be used to validate modifications
+before submitting them for review.
 
 The script operates on one or multiple git commits specified on the command
 line. It does not modify the git tree, the index or the working directory and
@@ -277,4 +284,8 @@ diff that fixes the issues, on top of the corresponding commit. As the script is
 in early development false positive are expected. The flagged issues should be
 reviewed, but the diff doesn't need to be applied blindly.
 
+The checkstyle.py script uses clang-format by default if found, and otherwise
+falls back to astyle. The formatter can be manually selected with the
+'--formatter' argument.
+
 Happy hacking, libcamera awaits your patches!

utils/checkstyle.py

@@ -4,11 +4,11 @@
 #
 # Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 #
-# checkstyle.py - A patch style checker script based on astyle
+# checkstyle.py - A patch style checker script based on astyle or clang-format
 #
 # TODO:
 #
-# - Support other formatting tools (clang-format, ...)
+# - Support other formatting tools and checkers (cppcheck, cpplint, kwstyle, ...)
 # - Split large hunks to minimize context noise
 # - Improve style issues counting
 #
@@ -33,6 +33,12 @@ astyle_options = (
     '--max-code-length=120'
 )
 
+dependencies = {
+    'astyle': False,
+    'clang-format': False,
+    'git': True,
+}
+
 source_extensions = (
     '.c',
     '.cpp',
@@ -192,30 +198,38 @@ def parse_diff(diff):
 # Code reformatting
 #
 
-def formatter_astyle(data):
+def formatter_astyle(filename, data):
     ret = subprocess.run(['astyle', *astyle_options],
                          input=data.encode('utf-8'), stdout=subprocess.PIPE)
     return ret.stdout.decode('utf-8')
 
 
-def formatter_strip_trailing_space(data):
+def formatter_clang_format(filename, data):
+    ret = subprocess.run(['clang-format', '-style=file',
+                          '-assume-filename=' + filename],
+                         input=data.encode('utf-8'), stdout=subprocess.PIPE)
+    return ret.stdout.decode('utf-8')
+
+
+def formatter_strip_trailing_space(filename, data):
     lines = data.split('\n')
     for i in range(len(lines)):
         lines[i] = lines[i].rstrip() + '\n'
     return ''.join(lines)
 
 
-formatters = [
+available_formatters = {
-    formatter_astyle,
+    'astyle': formatter_astyle,
-    formatter_strip_trailing_space,
+    'clang-format': formatter_clang_format,
-]
+    'strip-trailing-spaces': formatter_strip_trailing_space,
+}
 
 
 # ------------------------------------------------------------------------------
 # Style checking
 #
 
-def check_file(top_level, commit, filename):
+def check_file(top_level, commit, filename, formatters):
     # Extract the line numbers touched by the commit.
     diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--',
                            '%s/%s' % (top_level, filename)],
@@ -239,7 +253,8 @@ def check_file(top_level, commit, filename):
 
     formatted = after
     for formatter in formatters:
-        formatted = formatter(formatted)
+        formatter = available_formatters[formatter]
+        formatted = formatter(filename, formatted)
 
     after = after.splitlines(True)
     formatted = formatted.splitlines(True)
@@ -261,7 +276,7 @@ def check_file(top_level, commit, filename):
     return len(formatted_diff)
 
 
-def check_style(top_level, commit):
+def check_style(top_level, commit, formatters):
     # Get the commit title and list of files.
     ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit],
                          stdout=subprocess.PIPE)
@@ -282,7 +297,7 @@ def check_style(top_level, commit):
 
     issues = 0
     for f in files:
-        issues += check_file(top_level, commit, f)
+        issues += check_file(top_level, commit, f, formatters)
 
     if issues == 0:
         print("No style issue detected")
@@ -330,18 +345,39 @@ def main(argv):
 
     # Parse command line arguments
     parser = argparse.ArgumentParser()
+    parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'],
+                        help='Code formatter. Default to clang-format if not specified.')
     parser.add_argument('revision_range', type=str, default='HEAD', nargs='?',
                         help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')
     args = parser.parse_args(argv[1:])
 
     # Check for required dependencies.
-    dependencies = ('astyle', 'git')
+    for command, mandatory in dependencies.items():
-
+        found = shutil.which(command)
-    for dependency in dependencies:
+        if mandatory and not found:
-        if not shutil.which(dependency):
+            print("Executable %s not found" % command)
-            print("Executable %s not found" % dependency)
             return 1
 
+        dependencies[command] = found
+
+    if args.formatter:
+        if not args.formatter in dependencies or \
+           not dependencies[args.formatter]:
+            print("Formatter %s not available" % args.formatter)
+            return 1
+        formatter = args.formatter
+    else:
+        if dependencies['clang-format']:
+            formatter = 'clang-format'
+        elif dependencies['astyle']:
+            formatter = 'astyle'
+        else:
+            print("No formatter found, please install clang-format or astyle")
+            return 1
+
+    # Create the list of formatters to be applied.
+    formatters = [formatter, 'strip-trailing-spaces']
+
     # Get the top level directory to pass absolute file names to git diff
     # commands, in order to support execution from subdirectories of the git
     # tree.
@@ -352,7 +388,7 @@ def main(argv):
     revlist = extract_revlist(args.revision_range)
 
     for commit in revlist:
-        check_style(top_level, commit)
+        check_style(top_level, commit, formatters)
         print('')
 
     return 0