refactor the clang format example and add test

04-static-analysis/clang-format/CMakeLists.txt

@@ -11,4 +11,5 @@ add_subdirectory(subproject1)
 add_subdirectory(subproject2)
 
 set(CLANG_FORMAT_BIN_NAME clang-format-3.6)
+set(CLANG_FORMAT_EXCLUDE_PATTERNS  "build/" ${CMAKE_BINARY_DIR})
 find_package(ClangFormat)

04-static-analysis/clang-format/README.adoc

@@ -32,14 +32,15 @@ $ tree
 ```
 
   * link:CMakeLists.txt[] - Top level CMakeLists.txt
-  * link:.clang-format[] - The file describing the stype guide
+  * link:.clang-format[] - The file describing the style guide
   * link:cmake/modules/FindClangFormat.cmake[] - Script to find the clang-format binary
   * link:cmake/modules/clang-format.cmake[] - Script to setup the format targets
-  * link:cmake/scripts/clang-format-check-changed[] - A helper script to check against changed files in git
+  * link:cmake/scripts/clang-format-check-changed.py[] - A helper script to check against changed files in git
+  * link:cmake/scripts/clang-format-check-changed.py[] - An old simplified version of the above helper script
   * link:subproject1/CMakeLists.txt[] - CMake commands for subproject 1
-  * link:subproject1/main.cpp[] - source for a subproject with no errors
+  * link:subproject1/main.cpp[] - source for a subproject with no style errors
   * link:subproject2/CMakeLists.txt[] - CMake commands for subproject 2
-  * link:subproject2/main2.cpp[] - source for a subproject that includes errors
+  * link:subproject2/main2.cpp[] - source for a subproject that includes style errors
 
 # Requirements
 
@@ -47,16 +48,18 @@ To run this example you must have clang format tool installed. This can be insta
 
 [source,bash]
 ----
-$ sudo apt-get install clang-format
+$ sudo apt-get install clang-format-3.6
 ----
 
 It will result in the tool being available as:
 
 [source,bash]
 ----
-$ clang-format
+$ clang-format-3.6
 ----
 
+If you install a different version, you can edit the `CLANG_FORMAT_BIN_NAME` variable in the root CMakeLists.txt
+
 # Concepts
 
 ## clang-format
@@ -95,10 +98,10 @@ The format target will find any C++ source files and in place modify them to mat
 
 [source,cmake]
 ----
-file(GLOB_RECURSE ALL_SOURCE_FILES *.cpp *.h *.cxx *.hxx *.hpp *.cc)
+file(GLOB_RECURSE ALL_SOURCE_FILES *.cpp *.h *.cxx *.hxx *.hpp *.cc *.ipp)
 
 # Don't include some common build folders
-set(CLANG_FORMAT_EXCLUDE_PATTERNS ${CLANG_FORMAT_EXCLUDE_PATTERNS} "build/" "/CMakeFiles/")
+set(CLANG_FORMAT_EXCLUDE_PATTERNS ${CLANG_FORMAT_EXCLUDE_PATTERNS} "/CMakeFiles/" "cmake")
 
 # get all project files file
 foreach (SOURCE_FILE ${ALL_SOURCE_FILES}) 
@@ -112,7 +115,14 @@ endforeach ()
 ----
 
 This will find files matching the common C++ suffixes and then remove any that match some
-common CMake build directories.
+common CMake directories.
+
+In the root `CMakeList.txt` we also exclude the build directory by adding the line 
+
+[source,cmake]
+----
+set(CLANG_FORMAT_EXCLUDE_PATTERNS  "build/")
+----
 
 ## format-check 
 
@@ -123,13 +133,20 @@ if any files don't match the clang-format style
 
 This target will check the output of `git status` and scan the files to check if they match the style. This can be used by developers to make sure their changed files match the correct style.
 
-In this example the actual check is done with a helper script +clang-format-check-changed+. This calls the following command to check files:
+In this example the actual check is done with a helper script +clang-format-check-changed.py+. This script will run `git status --porcelain --ignore-submodules`
+to get a list of changed files, match them against the allowed extensions from the above list, and finally remove any
+that match the exclude pattern from +CLANG_FORMAT_EXCLUDE_PATTERNS+. It will then run these files through clang-format and 
+exit with an error if the files do not match the style.
+
+An example call to the +clang-format-check-changed.py+ script is:
 
 [source,bash]
 ----
-git status --porcelain \
+cmake/scripts/clang-format-check-changed.py --file-extensions ".cpp,*.cpp,*.h,*.cxx,*.hxx,*.hpp,*.cc,*.ipp" --exclude=build/ --exclude=/CMakeFiles/ --exclude=cmake --clang-format-bin /usr/bin/clang-format-3.6
-  | egrep '*\.cpp|*\.h|*\.cxx|*\.hxx|*\.hpp|*\.cc' \
-  | awk -F " " '{print $NF}' \
-  | xargs -r clang-format -style=file -output-replacements-xml \
-  | grep "replacement offset" 2>&1 > /dev/null
 ----
+
+[NOTE]
+====
+This will include all changed files in your git repository that match the patterns. In this example repository this can include files that 
+are part of different examples.
+====

04-static-analysis/clang-format/cmake/modules/FindClangFormat.cmake

@@ -6,7 +6,7 @@ if(NOT CLANG_FORMAT_BIN_NAME)
 endif()
 
 # if custom path check there first
-if(CPPCHECK_ROOT_DIR)
+if(CLANG_FORMAT_ROOT_DIR)
     find_program(CLANG_FORMAT_BIN 
         NAMES
         ${CLANG_FORMAT_BIN_NAME}

04-static-analysis/clang-format/cmake/modules/clang-format.cmake

@@ -1,10 +1,11 @@
 # A CMake script to find all source files and setup clang-format targets for them
 
 # Find all source files
-file(GLOB_RECURSE ALL_SOURCE_FILES *.cpp *.h *.cxx *.hxx *.hpp *.cc)
+set(CLANG_FORMAT_CXX_FILE_EXTENSIONS ${CLANG_FORMAT_CXX_FILE_EXTENSIONS} *.cpp *.h *.cxx *.hxx *.hpp *.cc *.ipp)
+file(GLOB_RECURSE ALL_SOURCE_FILES ${CLANG_FORMAT_CXX_FILE_EXTENSIONS})
 
 # Don't include some common build folders
-set(CLANG_FORMAT_EXCLUDE_PATTERNS ${CLANG_FORMAT_EXCLUDE_PATTERNS} "main2.cpp" "build/" "/CMakeFiles/")
+set(CLANG_FORMAT_EXCLUDE_PATTERNS ${CLANG_FORMAT_EXCLUDE_PATTERNS} "/CMakeFiles/" "cmake")
 
 # get all project files file
 foreach (SOURCE_FILE ${ALL_SOURCE_FILES}) 
@@ -24,21 +25,38 @@ add_custom_target(format
     ${ALL_SOURCE_FILES}
 )
 
+
 add_custom_target(format-check
     COMMENT "Checking clang-format changes"
-    COMMAND ${CLANG_FORMAT_BIN}
+    # Use ! to negate the result for correct output
+    COMMAND !
+    ${CLANG_FORMAT_BIN}
     -style=file
     -output-replacements-xml
     ${ALL_SOURCE_FILES}
-    | grep "replacement offset" 2>&1 > /dev/null
+    | grep -q "replacement offset" 
 )
 
 # Get the path to this file
 get_filename_component(_clangcheckpath ${CMAKE_CURRENT_LIST_FILE} PATH)
+# have at least one here by default
+set(CHANGED_FILE_EXTENSIONS ".cpp")
+foreach(EXTENSION ${CLANG_FORMAT_CXX_FILE_EXTENSIONS})
+    set(CHANGED_FILE_EXTENSIONS "${CHANGED_FILE_EXTENSIONS},${EXTENSION}" )
+endforeach()
+
+set(EXCLUDE_PATTERN_ARGS)
+foreach(EXCLUDE_PATTERN ${CLANG_FORMAT_EXCLUDE_PATTERNS})
+    list(APPEND EXCLUDE_PATTERN_ARGS "--exclude=${EXCLUDE_PATTERN}")
+endforeach()
+
 # call the script to chech changed files in git
 add_custom_target(format-check-changed
     COMMENT "Checking changed files in git"
     WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
-    COMMAND ${_clangcheckpath}/../scripts/clang-format-check-changed ${CLANG_FORMAT_BIN}
+    COMMAND ${_clangcheckpath}/../scripts/clang-format-check-changed.py 
+    --file-extensions \"${CHANGED_FILE_EXTENSIONS}\"
+    ${EXCLUDE_PATTERN_ARGS}
+    --clang-format-bin ${CLANG_FORMAT_BIN}
 )
 

04-static-analysis/clang-format/cmake/scripts/clang-format-check-changed.py

@@ -0,0 +1,163 @@
+#!/usr/bin/env python
+
+import argparse
+import os
+import sys
+import subprocess
+
+
+def check_file(filename, excludes, extensions):
+    """
+    Check if a file should be included in our check
+    """
+    name, ext = os.path.splitext(filename)
+
+    if len(ext) > 0 and ext in extensions:
+        if len(excludes) == 0:
+            return True
+
+        for exclude in excludes:
+            if exclude in filename:
+                return False
+
+        return True
+
+    return False
+
+
+def check_directory(directory, excludes, extensions):
+    output = []
+
+    if len(excludes) > 0:
+        for exclude in excludes:
+            if exclude in directory:
+                directory_excluded = False
+                return output
+
+    for root, _, files in os.walk(directory):
+        for file in files:
+            filename = os.path.join(root, file)
+            if check_file(filename, excludes, extensions):
+                print("Will check file [{}]".format(filename))
+                output.append(filename)
+    return output
+
+def get_git_root(git_bin):
+    cmd = [git_bin, "rev-parse", "--show-toplevel"]
+    try:
+        return subprocess.check_output(cmd).strip()
+    except subprocess.CalledProcessError, e:
+        print("Error calling git [{}]".format(e))
+        raise
+
+def clean_git_filename(line):
+    """
+    Takes a line from git status --porcelain and returns the filename
+    """
+    file = None
+    git_status = line[:2]
+    # Not an exhaustive list of git status output but should
+    # be enough for this case
+    # check if this is a delete
+    if 'D' in git_status:
+        return None
+    # ignored file
+    if '!' in git_status:
+        return None
+    # Covers renamed files
+    if '->' in line:
+        file = line[3:].split('->')[-1].strip()
+    else:
+        file = line[3:].strip()
+
+    return file
+
+
+def get_changed_files(git_bin, excludes, file_extensions):
+    """
+    Run git status and return the list of changed files
+    """
+    extensions = file_extensions.split(",")
+    # arguments coming from cmake will be *.xx. We want to remove the *
+    for i, extension in enumerate(extensions):
+        if extension[0] == '*':
+            extensions[i] = extension[1:]
+
+    git_root = get_git_root(git_bin)
+
+    cmd = [git_bin, "status", "--porcelain", "--ignore-submodules"]
+    print("git cmd = {}".format(cmd))
+    output = []
+    returncode = 0
+    try:
+        cmd_output = subprocess.check_output(cmd)
+        for line in cmd_output.split('\n'):
+            if len(line) > 0:
+                file = clean_git_filename(line)
+                if not file:
+                    continue
+                file = os.path.join(git_root, file)
+
+                if file[-1] == "/":
+                    directory_files = check_directory(
+                        file, excludes, file_extensions)
+                    output = output + directory_files
+                else:
+                    if check_file(file, excludes, file_extensions):
+                        print("Will check file [{}]".format(file))
+                        output.append(file)
+
+    except subprocess.CalledProcessError, e:
+        print("Error calling git [{}]".format(e))
+        returncode = e.returncode
+
+    return output, returncode
+
+
+def run_clang_format(clang_format_bin, changed_files):
+    """
+    Run clang format on a list of files 
+    @return 0 if formatted correctly.
+    """
+    if len(changed_files) == 0:
+        return 0
+    cmd = [clang_format_bin, "-style=file",
+           "-output-replacements-xml"] + changed_files
+    print("clang-format cmd = {}".format(cmd))
+    try:
+        cmd_output = subprocess.check_output(cmd)
+        if "replacement offset" in cmd_output:
+            print("ERROR: Changed files don't match format")
+            return 1
+    except subprocess.CalledProcessError, e:
+        print("Error calling clang-format [{}]".format(e))
+        return e.returncode
+
+    return 0
+
+
+def cli():
+    # global params
+    parser = argparse.ArgumentParser(prog='clang-format-check-changed',
+                                     description='Checks if files chagned in git match the .clang-format specification')
+    parser.add_argument("--file-extensions", type=str,
+                        default=".cpp,.h,.cxx,.hxx,.hpp,.cc,.ipp",
+                        help="Comma seperated list of file extensions to check")
+    parser.add_argument('--exclude', action='append', default=[],
+                        help='Will not match the files / directories with these in the name')
+    parser.add_argument('--clang-format-bin', type=str, default="clang-format",
+                        help="The clang format binary")
+    parser.add_argument('--git-bin', type=str, default="git",
+                        help="The git binary")
+    args = parser.parse_args()
+
+    # Run gcovr to get the .gcda files form .gcno
+    changed_files, returncode = get_changed_files(
+        args.git_bin, args.exclude, args.file_extensions)
+    if returncode != 0:
+        return returncode
+
+    return run_clang_format(args.clang_format_bin, changed_files)
+
+if __name__ == '__main__':
+    sys.exit(cli())

04-static-analysis/clang-format/run_test.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+mkdir -p build && cd build && cmake .. && make format-check
+RET=$?
+echo "return code was ${RET}"
+if [ ${RET} == "0" ]; then
+	echo "test failed. Expected format-check to fail"
+	exit 1
+else
+	echo "test success"
+	exit 0
+fi

dockerfiles/ubuntu14.04-cmake-3.4.3

@@ -8,6 +8,7 @@ RUN apt-get update && apt-get install -y build-essential \
     libprotobuf-dev \
     protobuf-compiler \
     clang-3.6 \
+    clang-format-3.6 \
     ninja-build \
     wget \
     git \

dockerfiles/ubuntu14.04-default-2.8.12.2

@@ -10,6 +10,7 @@ RUN apt-get update && apt-get install -y build-essential \
     protobuf-compiler \
     cppcheck \
     clang-3.6 \
+    clang-format-3.6 \
     ninja-build \
     wget \
     git \

dockerfiles/ubuntu16.04-default-cmake-3.5.1

@@ -9,6 +9,7 @@ RUN apt-get update && apt-get install -y build-essential \
     libprotobuf-dev \
     protobuf-compiler \
     clang-3.6 \
+    clang-format-3.6 \
     ninja-build \
     wget \
     git \

test.sh

@@ -31,6 +31,7 @@ dirs=(./01-basic/A-hello-cmake \
 ./04-static-analysis/cppcheck \
 ./04-static-analysis/cppcheck-compile-commands \
 ./04-static-analysis/clang-analyzer \
+./04-static-analysis/clang-format \
 ./05-unit-testing/boost \
 ./06-installer/deb \
 )