feat(tidy): Add new FairMQTidy.cmake module

FairRootGroup · Sep 7, 2021 · dff2b4b · dff2b4b
1 parent 9cbaf7e
commit dff2b4b
Show file tree

Hide file tree

Showing 7 changed files with 181 additions and 7 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -135,6 +135,11 @@ if(BUILD_DOCS)
     DESTINATION ${PROJECT_INSTALL_DATADIR}/docs
   )
 endif()
+if(BUILD_TIDY_TOOL)
+  install(FILES cmake/FairMQTidy.cmake
+    DESTINATION ${PROJECT_INSTALL_CMAKEMODDIR}
+  )
+endif()
 
 include(FairMQPackage)
 install_cmake_package()

diff --git a/README.md b/README.md
@@ -147,6 +147,9 @@ After the `find_package(FairMQ)` call the following CMake variables are defined:
     3. [Introspection](docs/Configuration.md#33-introspection)
 4. [Development](docs/Development.md#4-development)
    1. [Testing](docs/Development.md#41-testing)
+   2. [Static Analysis](docs/Development.md#42-static-analysis)
+      1. [CMake Integration](docs/Development.md#421-cmake-integration)
+      2. [Extra Compiler Arguments](docs/Development.md#422-extra-compiler-arguments)
 5. [Logging](docs/Logging.md#5-logging)
    1. [Log severity](docs/Logging.md#51-log-severity)
    2. [Log verbosity](docs/Logging.md#52-log-verbosity)

diff --git a/cmake/FairMQSummary.cmake b/cmake/FairMQSummary.cmake
@@ -97,9 +97,17 @@ macro(fairmq_summary_static_analysis)
     endforeach()
     set(static_ana_summary "${BWhite}(${CR}${analyser_list}${BWhite})${CR}  (disable with ${BMagenta}-DRUN_STATIC_ANALYSIS=OFF${CR})")
   else()
-    set(static_ana_summary "${BRed}OFF${CR}  (default, enable with ${BMagenta}-DRUN_STATIC_ANALYSIS=ON${CR})")
+    set(static_ana_summary "${BRed} NO${CR}  (default, enable with ${BMagenta}-DRUN_STATIC_ANALYSIS=ON${CR})")
   endif()
   message(STATUS "  ${Cyan}RUN STATIC ANALYSIS  ${static_ana_summary}")
+  if(BUILD_TIDY_TOOL)
+    if(RUN_FAIRMQ_TIDY)
+      set(tidy_summary "${BGreen}YES${CR}  (disable with ${BMagenta}-DRUN_FAIRMQ_TIDY=OFF${CR})")
+    else()
+      set(tidy_summary "${BRed} NO${CR}  (default, enable with ${BMagenta}-DRUN_FAIRMQ_TIDY=ON${CR})")
+    endif()
+    message(STATUS "  ${Cyan}RUN fairmq-tidy      ${tidy_summary}")
+  endif()
 endmacro()
 
 macro(fairmq_summary_install_prefix)

diff --git a/cmake/FairMQTidy.cmake b/cmake/FairMQTidy.cmake
@@ -0,0 +1,103 @@
+################################################################################
+#    Copyright (C) 2021 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH    #
+#                                                                              #
+#              This software is distributed under the terms of the             #
+#              GNU Lesser General Public Licence (LGPL) version 3,             #
+#                  copied verbatim in the file "LICENSE"                       #
+################################################################################
+
+include_guard(GLOBAL)
+
+#[=======================================================================[.rst:
+
+``fairmq_target_tidy()``
+========================
+
+Runs the FairMQ static analyzer ``fairmq-tidy`` on source files in given
+target.
+
+.. code-block:: cmake
+
+  fairmq_target_tidy(TARGET <target> [EXTRA_ARGS <args>]
+                    [CLANG_EXECUTABLE <clang>])
+
+Registers a custom command that depends on ``<target>`` which runs ``fairmq-tidy``
+on the source files that belong to ``<target>``. Optional extra arguments
+``<args>`` are passed to the ``fairmq-tidy`` command-line.
+
+Requires ``CMAKE_EXPORT_COMPILE_COMMANDS`` to be enabled.
+
+#]=======================================================================]
+
+function(fairmq_target_tidy)
+  cmake_parse_arguments(PARSE_ARGV 0 ARG "" "TARGET;EXTRA_ARGS;CLANG_EXECUTABLE" "")
+
+  if(NOT CMAKE_EXPORT_COMPILE_COMMANDS)
+    message(AUTHOR_WARNING "CMAKE_EXPORT_COMPILE_COMMANDS is not enabled. Skipping.")
+    return()
+  endif()
+
+  if(NOT ARG_TARGET)
+    message(AUTHOR_WARNING "TARGET argument is required. Skipping.")
+    return()
+  endif()
+
+  if(NOT TARGET ${ARG_TARGET})
+    message(AUTHOR_WARNING "Given TARGET argument `${ARG_TARGET}` is not a target. Skipping.")
+    return()
+  endif()
+
+  if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+    if(NOT fairmq_tidy_clang_resource_dir)
+      if(ARG_CLANG_EXECUTABLE)
+        set(clang_exe ${ARG_CLANG_EXECUTABLE})
+      else()
+        if(TARGET clang)
+          get_property(clang_exe TARGET clang PROPERTY LOCATION)
+        else()
+          set(clang_exe "clang")
+        endif()
+      endif()
+      execute_process(
+        COMMAND ${clang_exe} -print-resource-dir
+        OUTPUT_VARIABLE clang_resource_dir
+        OUTPUT_STRIP_TRAILING_WHITESPACE
+      )
+      set(fairmq_tidy_clang_resource_dir ${clang_resource_dir}
+        CACHE PATH "fairmq_target_tidy() internal state" FORCE)
+    endif()
+    list(APPEND ARG_EXTRA_ARGS
+      "--extra-arg-before=-I${fairmq_tidy_clang_resource_dir}/include")
+  endif()
+
+  if(ARG_EXTRA_ARGS)
+    set(extra 1)
+  else()
+    set(extra 0)
+  endif()
+
+  get_target_property(sources ${ARG_TARGET} SOURCES)
+  list(FILTER sources INCLUDE REGEX "\.(cpp|cxx)$")
+  string(REPLACE ":" "-" target_nocolon "${ARG_TARGET}")
+
+  set(src_deps)
+  foreach(source IN LISTS sources)
+    string(REPLACE "\/" "-" source_noslash "${source}")
+    set(src_stamp "${CMAKE_CURRENT_BINARY_DIR}/${target_nocolon}-${source_noslash}.fairmq-tidy")
+    add_custom_command(
+      OUTPUT ${src_stamp}
+      COMMAND $<TARGET_FILE:FairMQ::fairmq-tidy> -p=${CMAKE_BINARY_DIR}
+              $<${extra}:${ARG_EXTRA_ARGS}> ${source}
+      COMMAND ${CMAKE_COMMAND} -E touch ${src_stamp}
+      COMMENT "fairmq-tidy: Analyzing source file '${source}' of target '${ARG_TARGET}'"
+      DEPENDS ${ARG_TARGET}
+      WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+      COMMAND_EXPAND_LISTS VERBATIM
+    )
+    list(APPEND src_deps ${src_stamp})
+  endforeach()
+
+  if(src_deps)
+    add_custom_target("fairmq-tidy-${target_nocolon}" ALL DEPENDS ${src_deps})
+  endif()
+endfunction()
diff --git a/docs/Development.md b/docs/Development.md
@@ -8,6 +8,51 @@ For unit testing it is often not feasible to boot up a full-blown distributed sy
 
 In some scenarios it is useful to not even instantiate a `FairMQDevice` at all. Please see [this example](../test/protocols/_push_pull_multipart.cxx) for single and multi threaded unit test without a device instance. If you store your transport factories and channels on the heap, pls make sure, you destroy the channels before you destroy the related transport factory for proper shutdown. Channels provide all the `Send/Receive` and `New*Message/New*Poller` APIs provided by the device too.
 
-TODO Multiple devices in one process.
+## 4.2 Static Analysis
+
+With `-DBUILD_TIDY_TOOL=ON` you can build the `fairmq-tidy` tool that implements static checks on your source code. To use it, enable the generation of a [compilation database](https://clang.llvm.org/docs/JSONCompilationDatabase.html) in your project via `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON` (generates a file `<builddir>/compile_commands.json`):
+
+```
+fairmq-tidy -p <builddir> mysourcefile.cpp
+```
+
+If you find any issue (e.g. false positives) with this tool, please tell us by opening an issue on github.
+
+### 4.2.1 CMake Integration
+
+When discovering a FairMQ installation in your project, explicitely state, that you want one with the `fairmq-tidy` tool included:
+
+```
+find_package(FairMQ COMPONENTS tidy_tool)
+```
+
+Now the CMake module [`FairMQTidy.cmake`](../cmake/FairMQTidy.cmake) is available for inclusion:
+
+```
+include(FairMQTidy)
+```
+
+It provides the CMake function `fairmq_target_tidy()` which attaches appropriate `fairmq-tidy` build rules to each source file contained in the passed library or executable target, e.g. if you have an executable that uses FairMQ:
+
+```
+add_executable(myexe mysource1.cpp mysource2.cpp)
+target_link_libraries(myexe PRIVATE FairMQ::FairMQ)
+fairmq_target_tidy(TARGET myexe)
+```
+
+### 4.2.2 Extra Compiler Arguments
+
+On most Linux distros you are likely to use GCC to compile your projects, so the resulting `compile_commands.json` contains the command-line tuned for GCC which might be missing options needed to successfully invoke the Clang frontend which is used internally by `fairmq-tidy`. In general, you can pass extra `clang` options via the following options:
+
+```
+  --extra-arg=<string>        - Additional argument to append to the compiler command line
+  --extra-arg-before=<string> - Additional argument to prepend to the compiler command line
+```
+
+E.g. if standard headers are not found, you can hint the location like this:
+
+```
+fairmq-top -p <builddir> --extra-arg-before=-I$(clang -print-resource-dir)/include mysourcefile.cpp
+```
 
 ← [Back](../README.md)
diff --git a/fairmq/tidy/Tool.h b/fairmq/tidy/Tool.h
@@ -9,6 +9,7 @@
 #ifndef FAIR_MQ_TIDY_TOOL
 #define FAIR_MQ_TIDY_TOOL
 
+#include <algorithm>
 #include <clang/ASTMatchers/ASTMatchFinder.h>
 #include <clang/Basic/FileManager.h>
 #include <clang/Tooling/CompilationDatabase.h>
@@ -58,11 +59,20 @@ struct Tool
         tool.appendArgumentsAdjuster(
             [](tooling::CommandLineArguments const &_args, StringRef /*file*/) {
                 tooling::CommandLineArguments args(_args);
-                // TODO add only if cdb was generated with GCC
-                args.emplace(args.begin() + 1, "-I/usr/lib64/clang/12.0.0/include");
-                // TODO add only if missing
-                args.emplace(args.begin() + 1, "-std=c++17");
+
+                auto const no_std_arg_present =
+                    std::find_if(cbegin(args),
+                                 cend(args),
+                                 [](std::string const &arg) {
+                                     return arg.find("-std=") != std::string::npos;
+                                 })
+                    == std::cend(args);
+                if (no_std_arg_present) {
+                    args.emplace(std::cbegin(args) + 1, "-std=c++17");
+                }
+
                 args.emplace_back("-Wno-everything");
+
                 return args;
             });
 

diff --git a/fairmq/tidy/runTool.cpp b/fairmq/tidy/runTool.cpp
@@ -19,7 +19,7 @@ int main(int argc, const char** argv)
 {
     // TODO Replace command line parser with CLI11
     auto parser(clang::tooling::CommonOptionsParser::create(
-        argc, argv, ToolCategory, llvm::cl::NumOccurrencesFlag::Optional, ""));
+        argc, argv, ToolCategory, llvm::cl::NumOccurrencesFlag::ZeroOrMore, ""));
     if (!parser) {
         llvm::errs() << parser.takeError();
         return EXIT_FAILURE;