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.

Screenshot of GitLab CI pipeline with two stages (Build and Test), each containing a single successfuly finished job.

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 only clazy and clang-tidy are supported
• CXXFLAGS - custom CXXFLAGS 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.

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.