Merge pull request #30886 from jbytheway/custom_clang_tidy

Custom clang tidy plugin

.clang-tidy

@@ -6,6 +6,7 @@
 # fix their errors or recategorise them as checks we don't care about.
 Checks: "\
 bugprone-*,\
+cata-*,\
 cert-*,\
 -cert-err58-cpp,\
 clang-diagnostic-*,\

.travis.yml

@@ -105,12 +105,12 @@ jobs:
       osx_image: xcode10.1
       compiler: clang
 
-    - env: CLANG=clang++-8 TILES=1 SOUND=1 CXXFLAGS=-Wno-unused-command-line-argument CMAKE=1 CATA_CLANG_TIDY=clang-tidy-8
+    - env: CLANG=clang++-8 TILES=1 SOUND=1 CXXFLAGS=-Wno-unused-command-line-argument CMAKE=1 CATA_CLANG_TIDY=plugin
       name: "Clang-tidy CMake build with Tiles and Sound"
       compiler: clang
       addons: &clang8
         apt:
-          packages: ["clang-8", "clang-tidy-8", "libsdl2-dev", "libsdl2-ttf-dev", "libsdl2-image-dev", "libsdl2-mixer-dev"]
+          packages: ["clang-8", "libclang-8-dev", "llvm-8-dev", "llvm-8-tools", "libsdl2-dev", "libsdl2-ttf-dev", "libsdl2-image-dev", "libsdl2-mixer-dev"]
           sources: [*apt_sources, llvm-toolchain-xenial-8]
 
     # Finally check the compiler variants

CMakeLists.txt

@@ -8,6 +8,8 @@ SET(CMAKE_MODULE_PATH
     ${CMAKE_SOURCE_DIR}/CMakeModules
 )
 
+SET(CMAKE_TLS_VERIFY ON)
+
 # Build options
 option(TILES        "Build graphical tileset version."						"OFF")
 option(CURSES       "Build curses version."							"ON" )
@@ -17,8 +19,11 @@ option(USE_HOME_DIR "Use user's home directory for save files."					"ON" )
 option(LOCALIZE     "Support for language localizations. Also enable UTF support."		"ON" )
 option(LANGUAGES    "Compile localization files for specified languages."			""   )
 option(DYNAMIC_LINKING "Use dynamic linking. Or use static to remove MinGW dependency instead."	"ON")
-option(GIT_BINARY   "Git binary name or path."							"")
-OPTION(PREFIX       "Location of Data & GFX directories"                                   "")
+option(CATA_CLANG_TIDY_PLUGIN "Build Cata's custom clang-tidy plugin" "OFF")
+set(CATA_CLANG_TIDY_INCLUDE_DIR "" CACHE STRING "Path to internal clang-tidy headers required for plugin (e.g. ClangTidy.h)")
+set(CATA_CHECK_CLANG_TIDY "" CACHE STRING "Path to check_clang_tidy.py for plugin tests")
+set(GIT_BINARY       "" CACHE STRING "Git binary name or path.")
+set(PREFIX           "" CACHE STRING "Location of Data & GFX directories")
 
 include(CTest)
 
@@ -341,6 +346,9 @@ if (NOT MSVC)
 	add_subdirectory(src/chkjson)
 endif()
 add_subdirectory(tests)
+if (CATA_CLANG_TIDY_PLUGIN)
+	add_subdirectory(tools/clang-tidy-plugin)
+endif()
 
 CONFIGURE_FILE(
 	"${CMAKE_CURRENT_SOURCE_DIR}/cmake_uninstall.cmake.in"

Makefile

@@ -685,6 +685,8 @@ TESTSRC := $(wildcard tests/*.cpp)
 TESTHDR := $(wildcard tests/*.h)
 JSON_FORMATTER_SOURCES := tools/format/format.cpp src/json.cpp
 CHKJSON_SOURCES := src/chkjson/chkjson.cpp src/json.cpp
+CLANG_TIDY_PLUGIN_SOURCES := \
+  $(wildcard tools/clang-tidy-plugin/*.cpp tools/clang-tidy-plugin/*/*.cpp)
 TOOLHDR := $(wildcard tools/*/*.h)
 # Using sort here because it has the side-effect of deduplicating the list
 ASTYLE_SOURCES := $(sort \
@@ -694,6 +696,7 @@ ASTYLE_SOURCES := $(sort \
   $(TESTHDR) \
   $(JSON_FORMATTER_SOURCES) \
   $(CHKJSON_SOURCES) \
+  $(CLANG_TIDY_PLUGIN_SOURCES) \
   $(TOOLHDR))
 
 _OBJS = $(SOURCES:$(SRC_DIR)/%.cpp=%.o)

build-scripts/build.sh

@@ -41,6 +41,17 @@ then
         build_type=Debug
     fi
 
+    cmake_extra_opts=
+
+    if [ "$CATA_CLANG_TIDY" = "plugin" ]
+    then
+        cmake_extra_opts="$cmake_extra_opts -DCATA_CLANG_TIDY_PLUGIN=ON"
+        # Need to specify the particular LLVM / Clang versions to use, lest it
+        # use the llvm-7 that comes by default on the Travis Xenial image.
+        cmake_extra_opts="$cmake_extra_opts -DLLVM_DIR=/usr/lib/llvm-8/lib/cmake/llvm"
+        cmake_extra_opts="$cmake_extra_opts -DClang_DIR=/usr/lib/llvm-8/lib/cmake/clang"
+    fi
+
     mkdir build
     cd build
     cmake \
@@ -49,9 +60,25 @@ then
         -DCMAKE_BUILD_TYPE="$build_type" \
         -DTILES=${TILES:-0} \
         -DSOUND=${SOUND:-0} \
+        $cmake_extra_opts \
         ..
     if [ -n "$CATA_CLANG_TIDY" ]
     then
+        if [ "$CATA_CLANG_TIDY" = "plugin" ]
+        then
+            make -j$num_jobs CataAnalyzerPlugin
+            export PATH=$PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin:$PATH
+            if ! which FileCheck
+            then
+                ls -l tools/clang-tidy-plugin/clang-tidy-plugin-support/bin
+                ls -l /usr/bin
+                echo "Missing FileCheck"
+                exit 1
+            fi
+            CATA_CLANG_TIDY=clang-tidy
+            lit -v tools/clang-tidy-plugin/test
+        fi
+
         "$CATA_CLANG_TIDY" --version
 
         # Run clang-tidy analysis instead of regular build & test
@@ -95,7 +122,7 @@ then
         analyze_files_in_random_order "$remaining_cpp_files"
     else
         # Regular build
-        make -j3
+        make -j$num_jobs
         cd ..
         # Run regular tests
         [ -f "${bin_path}cata_test" ] && run_tests "${bin_path}cata_test"

build-scripts/clang-tidy-wrapper.sh

@@ -15,5 +15,12 @@ then
     exit 0
 fi
 
+plugin=build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so
+
 set -x
-"$CATA_CLANG_TIDY" "$@"
+if [ -f "$plugin" ]
+then
+    LD_PRELOAD=$plugin "$CATA_CLANG_TIDY" "$@"
+else
+    "$CATA_CLANG_TIDY" "$@"
+fi

build-scripts/requirements.sh

@@ -30,7 +30,7 @@ if [ -n "${CODE_COVERAGE}" ]; then
 fi
 
 if [ -n "$CATA_CLANG_TIDY" ]; then
-    travis_retry pip install --user compiledb
+    travis_retry pip install --user compiledb lit
 fi
 
 # Influenced by https://github.com/zer0main/battleship/blob/master/build/windows/requirements.sh

doc/DEVELOPER_TOOLING.md

@@ -9,6 +9,56 @@ On Windows, there is an astyle extension for Visual Studio.
 
 See the [JSON style guide](JSON_STYLE.md).
 
+## clang-tidy
+
+Cataclysm has a [clang-tidy configuration file](../.clang-tidy) and if you have
+`clang-tidy` available you can run it to perform static analysis of the
+codebase.  We test with `clang-tidy` from LLVM 8.0.1 on Travis, so for the most
+consistent results, you might want to use that version.
+
+### Custom clang-tidy plugin
+
+We have written our own clang-tidy checks in a custom plugin.  Unfortunately,
+`clang-tidy` as distributed by LLVM doesn't support plugins, so making this
+work requires some extra steps.
+
+If you are on Ubuntu Xenial then you might be able to get it working the same
+way Travis does.  Add the LLVM 8 Xenial source [listed
+here](https://apt.llvm.org/) to your `sources.list`, install the `clang-8
+libclang-8-dev llvm-8-dev llvm-8-tools` packages and build Cataclysm with CMake
+adding `-DCATA_CLANG_TIDY_PLUGIN=ON`.
+
+On other distributions you will probably need to build `clang-tidy` yourself.
+* Check out the `llvm`, `clang`, and `clang-tools-extra` repositories in the
+  required layout (as described for example
+  [here](https://quuxplusone.github.io/blog/2018/04/16/building-llvm-from-source/).
+* Patch in plugin support for `clang-tidy` using [this
+  patch](https://github.com/jbytheway/clang-tidy-plugin-support/blob/master/plugin-support.patch).
+* Configure LLVM using CMake, including the
+  `-DCMAKE_EXE_LINKER_FLAGS="-rdynamic"` option.
+* Add the `build/bin` directory to your path so that `clang-tidy` and
+  `FileCheck` are found from there.
+
+Then you can use your locally build `clang-tidy` to compile Cataclysm.  You'll
+need to use the CMake version of the Cataclysm build rather than the `Makefile`
+build.  Add the following CMake options:
+```sh
+-DCATA_CLANG_TIDY_PLUGIN=ON
+-DCATA_CLANG_TIDY_INCLUDE_DIR="$extra_dir/clang-tidy"
+-DCATA_CHECK_CLANG_TIDY="$extra_dir/test/clang-tidy/check_clang_tidy.py"
+```
+where `$extra_dir` is the location of your `clang-tools-extra` checkout.
+
+If you wish to run the tests for the custom clang-tidy plugin you will also
+need `lit`.  This will be built as part of `llvm`, or you can install it via
+`pip` or your local package manager if you prefer.
+
+Then, assuming `build` is your Cataclysm build directory, you can run the tests
+with
+```sh
+lit -v build/tools/clang-tidy-plugin/test
+```
+
 ## include-what-you-use
 
 [include-what-you-use](https://github.com/include-what-you-use/include-what-you-use)

tests/colony_test.cpp

@@ -332,7 +332,7 @@ TEST_CASE( "insert and erase", "[colony]" )
     } while( count < 15000 );
 
     // Erase randomly till half-empty
-    CHECK( test_colony.size() == static_cast<unsigned long>( 30000 - count ) );
+    CHECK( test_colony.size() == static_cast<size_t>( 30000 - count ) );
 
     for( int i = 0; i < count; ++i ) {
         test_colony.insert( 1 );
@@ -501,7 +501,7 @@ TEST_CASE( "insert and erase", "[colony]" )
     }
 
     // Multiple sequential small insert/erase commands
-    CHECK( test_colony.size() == static_cast<unsigned long>( count ) );
+    CHECK( test_colony.size() == static_cast<size_t>( count ) );
 }
 
 TEST_CASE( "range erase", "[colony]" )

tools/clang-tidy-plugin/CMakeLists.txt

@@ -0,0 +1,54 @@
+include(ExternalProject)
+
+find_package(LLVM REQUIRED CONFIG)
+find_package(Clang REQUIRED CONFIG)
+
+add_library(
+    CataAnalyzerPlugin MODULE
+    CataTidyModule.cpp NoLongCheck.cpp)
+
+target_include_directories(
+    CataAnalyzerPlugin SYSTEM PRIVATE
+    ${LLVM_INCLUDE_DIRS} ${CLANG_INCLUDE_DIRS})
+
+if ("${CATA_CLANG_TIDY_INCLUDE_DIR}" STREQUAL "")
+    SET(ctps_releases
+        https://github.com/jbytheway/clang-tidy-plugin-support/releases/download)
+    SET(ctps_version llvm-8.0.1-r12)
+    SET(ctps_src
+        ${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-plugin-support)
+
+    ExternalProject_Add(
+        clang-tidy-plugin-support
+        URL ${ctps_releases}/${ctps_version}/clang-tidy-plugin-support-${ctps_version}.tar.xz
+        URL_HASH SHA256=00ffab0df11250f394830735514c62ae787bd2eb6eb9d5e97471206d270c54e2
+        SOURCE_DIR ${ctps_src}
+        CONFIGURE_COMMAND ""
+        BUILD_COMMAND ""
+        INSTALL_COMMAND ""
+        TEST_COMMAND ""
+    )
+
+    add_dependencies(CataAnalyzerPlugin clang-tidy-plugin-support)
+    target_include_directories(
+        CataAnalyzerPlugin SYSTEM PRIVATE ${ctps_src}/include)
+else()
+    target_include_directories(
+        CataAnalyzerPlugin SYSTEM PRIVATE ${CATA_CLANG_TIDY_INCLUDE_DIR})
+endif()
+
+target_compile_definitions(
+    CataAnalyzerPlugin PRIVATE ${LLVM_DEFINITIONS})
+
+# We need to turn off exceptions and RTTI to match the LLVM build.
+# I feel there ought to be a way to extract these flags from the
+# LLVMConfig.cmake as we have done for e.g. LLVM_INCLUDE_DIRS above, but I
+# haven't found one.
+if(MSVC)
+else()
+    target_compile_options(
+        CataAnalyzerPlugin PRIVATE -fno-exceptions -fno-rtti)
+endif()
+
+configure_file(test/lit.site.cfg.in test/lit.site.cfg @ONLY)
+configure_file(test/.clang-tidy test/.clang-tidy COPYONLY)

tools/clang-tidy-plugin/CataTidyModule.cpp

@@ -0,0 +1,28 @@
+#include "ClangTidy.h"
+#include "ClangTidyModule.h"
+#include "ClangTidyModuleRegistry.h"
+#include "NoLongCheck.h"
+
+namespace clang
+{
+namespace tidy
+{
+namespace cata
+{
+
+class CataModule : public ClangTidyModule
+{
+    public:
+        void addCheckFactories( ClangTidyCheckFactories &CheckFactories ) override {
+            CheckFactories.registerCheck<NoLongCheck>( "cata-no-long" );
+        }
+};
+
+} // namespace cata
+
+// Register the MiscTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<cata::CataModule>
+X( "cata-module", "Adds Cataclysm-DDA checks." );
+
+} // namespace tidy
+} // namespace clang