Infrastructure/GitLab/CI/Static Code Analysis
Motivation
Static code analysis is a helpful tool to improve and keep high level of code quality. Static code analyzer inspects the source code and looks for issues like wrong usage of some API, bugprone statements, potential performance issues and many others. The static analyzers available on our KDE instance of GitLab are clazy and clang-tidy. Since both of those analyzers are based on the clang C++ compiler, the issues the analyzers find are reported as regular compiler warnings.
How It Works
The chapters below describe how to enable and configure static analysis for your project. Here we will describe some terminology and the overall design of the solution.
Introduction to GitLab CI
Whenever a trigger event happens on GitLab (e.g. a commit is pushed to GitLab, a new merge request is created etc.), GitLab CI will create a new pipeline. A pipeline consist of one or more stages, each stage containing one or more jobs. Which jobs are included in the pipeline may depend on the event that has triggered the pipeline - e.g. you can have different set of jobs executed for each push into the master branch and different set of jobs executed for merge requests.
Jobs within the same stage may be executed in parallel, but jobs in the next stage will always be executed after all jobs in the previous stage have finished. Artifacts of jobs from previous stage are automatically available to jobs in the next stage.
Static Code Analysis Intermezzo
Static code analysis is performed, no surprise, by static code analyzers. Analyzers currently supported by our solution are clang-tidy and clazy. Both of them are based on the clang compiler. To perform static analysis, the tools are passed the same arguments as if they were invoked during a regular build, they attempt to compile the given file and as a side-effect of that they produce additional compiler warnings with results of the static analysis check.
Since the static analyzers are basically C++ compilers, one major prerequisite for them to be able to analyze the source code successfuly is that the code must compile. In case of Qt-based applications that means that all the generated header files that might be included from C++ code (e.g. MOC headers, UI headers, DBus adaptor and interface headers) must exist. Since those are generated at build time, it is necessary to run a regular build first to ensure all the files are generated and each file compiles just fine.
Static Analysis Jobs on invent.kde.org
Enabling static analysis jobs on KDE's instance of GitLab effectively create two new jobs. We'll explain why there are two jobs later on. The jobs are placed in the "Test" stage, which follows after the "Build" stage. This way the project is first compiled as part of a regular CI build (same as what we have per-commit on build.kde.org. At the end the "build" directory is captured as a job artifact and is passed over to the "Test" stage. This way the static analysis jobs have access to all the generated files without having to compile the project again, which speeds up the pipeline.
To explain why there are two jobs: due to limitation in GitLab CI, it's not possible to define a single job that would create a pipeline either for a merge request OR for push to a git branch. This means that pushing to a branch that has an existing merge request would create a duplicate pipeline - one due for the merge_requests
trigger, one for the pushes
trigger. The two jobs are called static-analysis-linux-merge-request
and static-analysis-linux-commit
static-analysis-linux-merge-request
This job is executed for merge requested - when merge request is created and whenever a new commit is pushed there, this job will be included in the pipeline.
The job doesn't perform static analysis of the entire code base, but it instead detects files that have been changed compared to the merge request target branch and only perform static analysis on those files, again considerably speeding up the pipeline.
static-analysis-linux-commit
This job is executed automatically for push to the master branch. It is possible to customize the trigger branches per project, this will be shown below. The job doesn't perform static analysis of the entire project, but only of files that have changed since the last push to the branch.
Alternatively this job is also used in manually triggered pipelines. This is the only case when the entire code base will be analyzed.
Merge Requests
When the pipeline succeeds (both the build and the static analysis job pass successfully) you should see no information about code quality report, or possibly that the codequality has even improved.
When the static analysis job finds some issues in the changed files, you will see a quality report section in the merge request preview on GitLab:
Setting up
The GitLab CI pipeline for each project is defined in project's .gitlab-ci.yml
file, which must be placed in the top-level directory of the project's git repository.
Below are examples of default configuration that you need to add to the .gitlab-ci.yml
file in order to get the static analysis jobs working. The configuration is slightly different based on whether your project is part of Frameworks, Applications or Extragear "groups", or whether it's a completely standalone project (e.g. a personal project).
Frameworks, Applications and Extragear projects
include: - https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-before.yml - https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-<PRODUCT>-linux.yml - https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-static-analysis.yml static-analysis-linux-merge-request: extends: .static-analysis-linux-merge-request static-analysis-linux-commit: extends: .static-analysis-linux-commit
Replace <PRODUCT>
with the lower-case name of the product your application belongs to, i.e. frameworks
, applications
or extragear
.
Playground/Miscellaneous projects
include: - https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-before.yml - https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-playground-linux.yml - https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-static-analysis.yml static-analysis-linux-merge-request: extends: .static-analysis-linux-merge-request static-analysis-linux-commit: extends: .static-analysis-linux-commit
Customizing the static analyzer jobs
The jobs as we defined them above inherit (extends
) some pre-defined templates. It is possible to override any of the options defined in the template, see the full Gitlab CI pipeline configuration reference.
There are some options especially worth mentioning here explicitly, though.
The following is same for both jobs:
static-analysis-commit: extends: .static-analysis-commit variables: ANALYZERS: clazy,clang-tidy CXXFLAGS: -Werror -Wno-deprecated-declarations -Wno-deprecated-copy
ANALYZERS
- comma-separated list of static analyzers to execute. Currently onlyclazy
andclang-tidy
are supportedCXXFLAGS
- customCXXFLAGS
that should be passed to the static analyzer. The default is to disable deprecation warnings as often nothing can be done about those.
For the static-analysis-commit
job you may also be interested in triggering it for pushes on different branches than just `master` (say, for Applications it could also be the release/*
branches), in that case, add the following to the job definition:
static-analysis-commit: extends: .static-analysis-commit only: refs: - master - /^release/.*$/ - schedules - web
Remember to leave the `schedules` and `web` triggers in there, otherwise it won't be possible to launch the pipeline from the GitLab web interface.
Clazy
Clazy is a Qt-oriented static code analyzer based on the Clang framework.
Configuring Clazy
Clazy is configured through environment variable.
Checks which Clazy should perform are configured through CLAZY_CHECKS
environment variable. Clazy can be also asked to ignore warnings coming from files in particular directories (e.g. system includes or some 3rdparty code in your codebase) through CLAZY_IGNORE_DIRS
environment variable.
Example:
static-analysis-linux-merge-request: extends: .static-analysis-linux-merge-request variables: CLAZY_CHECKS: level0,level1,level2 CLAZY_IGNORE_DIRS: /usr/include/|myproject/3rdparty/brokenlib
Supressing warnings
If Clazy reports false-positives or is warning about code that simply cannot be change (e.g. due to preserving API/ABI), you can tell Clazy to not perform any checks on the entire file or the particular offending line. See the Clazy documentation for detailed explanation.
Clang-tidy
clang-tidy is a clang-based C++ “linter” tool. Its purpose is to provide an extensible framework for diagnosing and fixing typical programming errors, like style violations, interface misuse, or bugs that can be deduced via static analysis. clang-tidy is modular and provides a convenient interface for writing new checks.[1]
Configuring Clang-tidy
Clang-tidy is best configured through .clang-tidy
file in the root of the git repository. Below is an example of how the configuration file make look like:
Checks: -*, bugprone-*, clang-analyzer-*, -clang-analyzer-osx, -clang-analyzer-cplusplus.NewDeleteLeaks, google-*, -google-build-using-namespace, -google-readability-todo, -google-runtime-references, -google-readability-function-size, -google-default-arguments, misc-*, -misc-definitions-in-headers, -*-non-private-member-variables-in-classes, performance-*, -performance-no-automatic-move, readability-*, -readability-avoid-const-params-in-decls, -readability-convert-member-functions-to-static, -readability-else-after-return, -readability-redundant-access-specifiers, -readability-implicit-bool-conversion, -readability-static-accessed-through-instance, -readability-inconsistent-declaration-parameter-name, -readability-magic-numbers, -readability-make-member-function-const, -readability-function-size AnalyzeTemporaryDtors: true CheckOptions: - key: bugprone-assert-side-effects.AssertMacros value: 'Q_ASSERT;QVERIFY;QCOMPARE;AKVERIFY' - key: CheckFunctionCalls value: true - key: StringCompareLikeFuctions value: 'QString::compare;QString::localeAwareCompare' - key: WarnOnSizeOfIntegerExpression value: 1 - key: bugprone-dangling-handle.HandleClasses value: 'std::string_view;QStringView' - key: IgnoreClassesWithAllMemberVariablesBeingPublic value: true - key: VectorLikeClasses value: 'std::vector;QVector' WarningsAsErrors: bugprone-*, clang-*, google-*, misc-*, performance-*, readability-*, -readability-magic-numbers, -readability-make-member-function-const
The -*
statement in the `Checks` value disables all checks by default. The next line enables all checks starting with clang-analyzer-
. The two line below disable two specific clang-analyzer
checks (clang-analyzer-osx
and <clang-analyzer-cplusplus.NewDeleteLeaks).
The CheckOptions
provide configuration for specific checkers, in this case it explicitly adds some Qt equivalents for std library classes which the checkers are otherwise unaware of.
See the Clang-tidy checks page on clang-tidy home-page with an exhaustive list of checks that clang-tidy supports together with their description and options.
Supressing warnings
Similar to Clazy, if clang-tidy wrongly warns about some code or possibly it's not possible to address the issue in the code for various reasons, the line (or the entire file) can be marked as NOLINT by adding a comment. See the Supressing Undesired Diagnostics chapter in clang-tidy documentation.