Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
springmeyer opened this issue Aug 17, 2017 · 2 comments
Closed

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

springmeyer opened this issue Aug 17, 2017 · 2 comments
Milestone

Comments

@springmeyer
Copy link
Contributor

springmeyer commented Aug 17, 2017

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

  • apply a large set of robustness checks
  • automatically modernize code

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
Copy link
Contributor Author

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 pushed a commit to mapbox/carmen-cache that referenced this issue Aug 18, 2017
 - avoid leaking memory when error is throw
 - incorrect usage of std::move which prevents copy elison

Detected using the setup from mapbox/node-cpp-skel#63
@springmeyer springmeyer added this to the 1.0.0 milestone Aug 25, 2017
@springmeyer
Copy link
Contributor Author

Done in #64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant