spotfiresoftware / spotfire-python

Package for Building Python Extensions to Spotfire®
Other
18 stars 6 forks source link

Add 'cppcheck'/'clang-tidy' to workflows for analysis of C code #50

Closed bbassett-tibco closed 10 months ago

bbassett-tibco commented 1 year ago

To help keep code quality high, we should run a linter on the C code in addition to running pylint on the Python code. While I would prefer to use cppcheck for linting, it appears that clang-tidy is installed on the ubuntu-latest GitHub Actions runner (and thus is available for use without building it ourselves).

bbassett-tibco commented 10 months ago

So, I had a look at what to use for linting C code. Boy, did I fall down a rabbit hole. :)

We're limiting the scope of linting C code to stuff authored in this repository. Generated C code (from the cythonization process of *.pyx files) and vendored C code (from the SBDF C library) will not be processed. This limits us to just the *_helpers.c files and *_helpers.h headers.

cppcheck

cppcheck doesn't really identify any issues; when turning up the analysis and explicitly setting the language (using --enable=all -x c), the only report is for unused struct members (which would be expected, since the uses are in the cythonized code we're excluding from the analysis).

clang-tidy

clang-tidy does not provide any feedback, since it reports "Compile command not found" for all source files. It looks like a compile_commands.json compilation database is required to provide that information (which makes sense, since the compilation database is originally a Clang concept), but it's not currently possible to use setuptools to generate one (pypa/setuptools#1975).

splint

splint was unable to parse the C source files due to preprocessor issues with both the Python and Windows API headers included.

oclint

oclint reported the same "Compile command not found" errors as clang-tidy, for the same reasons (missing compilation database).

cpplint

cpplint seems to be the best overall option for this repository. It's written in Python, and is available from PyPI (like our other linters). It's also the only tool that caught a usage of strcpy. It provides a good balance between ease of use and finding valid issues, and has reasonably good bones (since it is based on the Google style guide).

bbassett-tibco commented 10 months ago

Added cpplint to the build workflows.