mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

Build clang-tidy support into node-cpp-skel #63

Closed springmeyer closed 7 years ago

springmeyer commented 7 years ago

The clang-tidy command is a powerful tool from the llvm suite with the ability to:

Details at https://clang.llvm.org/extra/clang-tidy/

clang-tidy uses static analysis

We should integrate an easy clang-tidy workflow into node-cpp-skel by following the lead of both nodejs core, mapbox-gl-native, and OSRM:

Note: I presume clang-tidy will find very few issues with the current node-cpp-skel code, since it is fairly simple. The main idea here is to put this in place so that modules based on node-cpp-skel will more readily catch bugs before they hit production.

springmeyer commented 7 years ago

One challenge here is that clang-tidy needs to compile the code to do its checks. And to compile node-cpp-skel code requires a variety of unique include paths to node-gyp bundled headers and nan.h in node_modules, and any mason deps.

To avoid needing to write a second build system to collect and pass all these paths, it is best to have the build system generate a compile_commands.json that clang-tidy can consume. The format is basically an JSON array of which files are compiled, with what compiler args should be used. It is described in detail at https://clang.llvm.org/docs/JSONCompilationDatabase.html.

The cmake build system has support for outputting compile commands this with cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON. But we don't use cmake here because the nodejs standard is gyp. So the question becomes how to generate the compile_commands.json ourselves.

I've found https://github.com/rizsotto/Bear, but it looks like overkill. It would need to be compiled itself, packaged in mason, and who knows how well it works.

So, instead I think we should write a command that captures the output of the existing build, parses it, and rewrites the compile_commands.json from it. We can pipe the output of the existing build into this tool in order to capture it.

springmeyer commented 7 years ago

Done in #64