Code Quality and Tooling

WissKI project makes use of the Gitlab Templates for linting. In particular, we make use of:

• parallel-lint
• cspell
• eslint
• phpcs (PHP_CodeSniffer)
• stylelint
• phpstan
• phpunit

These tools vaguely fall into three categories:

• Linters and Code Style checkers: These check that code is properly formatted.
• Static Analysers: These check types of specific variables and sections of the code without running it.
• Test Runners: These run automated test code that checks if code works as expected.

All configuration files for these tools are committed. All tools exclude the legacy/ directory as modules inside it are no longer being maintained. The tools all run:

• Manually inside the Dev Container.
• Automatically inside GitLab CI, whenever a pull request is made, or a commit is pushed.

For details on what these tools do, see the sections below.

Utility Scripts in the devcontainer

There are several scripts used to support running these tools locally. For example:

bash misc/scripts/lint.sh

In total there are five scripts. They are expected to be run inside the devcontainer, but may also work in other setups. They are:

• misc/scripts/lint.sh
  Lints all files exactly as the CI would. Aborts after the first failing test.
• misc/scripts/fmt.sh
  Runs automatic fixers on all source code.
• misc/scripts/quicklint.sh
  An alternative to lint.sh that runs much quicker. It does the same thing as the CI would, but runs on a subset of all source files. These are picked as follows:
  1. If there are any paths provided as an argument, lint only those.
  2. Otherwise, lint the files currently shown as "dirty" by git.
  3. Fall back to the files changed in the last git commit.
• misc/scripts/quickfmt.sh
  An alternative to fmt.sh that only runs as a subset of files. Files to format are picked the same way as quicklint.sh does.
• misc/scripts/_dev.sh
  Intended to be sourced in bash. It updates the current shell to allow for running all development tools individually in the current shell.

Before committing one should usually run either misc/scripts/quicklint.sh or misc/scripts/lint.sh. This ensures that all code beautifiers, linters and static analysis tools run as expected.

parallel-lint - PHP Syntax Checker

parallel-lint checks that all php files are valid php source code and no syntax errors occur.

There is no configuration file for this tool. There should never be any reason to ignore a failing parallel-lint; there are no false positives.

To run parallel-lint in the devcontainer, run:

parallel-lint .

The CI runs parallel-lint inside the composer-lint job.

cspell - spell checker

cspell performs spell-checking on all source files. In particular, it checks that no mis-spelled words occur inside any of the source files. It is configured using the .cspell.json configuration file.

It can be run in the devcontainer with cspell .. It is run on CI using the CSpell job.

All source files should pass cspell checks. Words can be added to cspell using one of two ways:

1. Using a dictionary file
2. Using a <!-- spellchecker:words some-word-here --> comment

Dictionaries are plain-text files with one word per line. The dictionaries being used are configured in .cspell.json. Usually words should only added to a dictionary if they occur in more than a single file.

By convention, dictionaries are placed in the misc/cspell directory. It currently contains the following:

• misc/cspell/wisski.txt
  Words being used inside the WissKI source code.
• misc/cspell/fixme.txt
  Words that are obviously mis-spelled but cannot be fixed for various reasons, This might be because they are being used in the machine name of a plugin.
• misc/cspell/names.txt
  Contributor names being used inside the WissKI code.
• misc/cspell/drupal/
  Dictionaries copied over from drupal core. An appropriate bash script exists to automatically update them.
• misc/cspell/unknown.txt
  Words that have not been otherwise classified, and are likely typos to be fixed. This is usually generated automatically, using the command found in the dictionary file itself.
• misc/cspell/docs.txt
  Words being used only inside the documentation. This dictionary is special, as it is only applied to the docs directory.

eslint and prettier - JavaScript linters

eslint checks that JavaScript source files are properly formatted. It is configured using .eslintrc.json.

It can be run in the devcontainer with node_modules/.bin/eslint .. It is run on CI using the Eslint job.

Eslint can be configured using so-called rules. As suggested by the drupal community, we make use of two standards of rules:

• prettier/recommended
  Performs basic style checking of JavaScript source files.
• yml/recommended
  Checks that all yml files are valid YAML and formatted nicely.

When there are code style warnings, it is also possible to automatically re-format the files:

node_modules/.bin/eslint --fix .

stylelint and prettier

stylelint checks that CSS source files are properly formatted. It is configured using .stylelintrc.json.

It can be run in the devcontainer with node_modules/.bin/stylelint .. It is run on CI using the Stylelint job.

When there are code style warnings, it is also possible to automatically re-format the files:

node_modules/.bin/stylelint --fix .

phpcs - static code analysis

phpcs (PHP_CodeSniffer) checks that php code is nicely formatted. It is configured using the phpcs.xml.dist file.

It can be run in the devcontainer using phpcs .. When running phpcs repeatedly it can be very slow. To speed up performance, it is recommended to use the cache argument with something like:

phpcs --cache=/tmp/phpcs.cache .

This speeds up repeated runs significantly.

It is run on CI using the PHP Coding Standards job.

Most code style issues can be fixed automatically using phpcbf. Inside the dev container use:

phpcbf .

In very limited cases, it is worthwhile to ignore specific coding issues. In this case an ignore comment can be placed inside the code. For example:

// phpcs:ignore Drupal.Semantics.FunctionT.ConcatString -- otherwise translation adds <em> for placeholders

It is important to not over-use these comments to maintain code quality. In particular one should never place a blanket phpcs:ignore comment without any rule names. It is also important to place a reason (separated with --) on why the rule is ignored.

phpstan - static analysis

phpstan checks types of variables and certain classes inside the code. It is configured using phpstan.neon.

It can be run inside the devcontainer using phpstan analyze -v .. It is run on CI using the PHPStan job.

PHPStan can generally be configured using specific levels going from 0 to 10. PHPStan is currently on level 6, which requires that all parts of the code have appropriate type annotations.

An additional set of files and folders are already checked on phpstan level 10. These are listed in phpstan-level10.txt. This check can be run locally using bash misc/scripts/lint_l10.txt. On CI this is run as a custom job phpstan_level10.

There are places inside the code where it is needed to ignore specific error messages. This can be documented using @phpstan-ignore comments. For example:

/** @phpstan-ignore property.notFound (we just checked above it exists) */

To maintain code quality, it is important that specific rule names are passed to the ignore comment. For these, PHPStan will check that the ignore is actually needed - if no such error exists, it will produce an error.

In case a rule triggers multiple times, the names can be separated with a comma:

/** @phpstan-ignore method.childParameterType, method.childParameterType */

The most common ignore inside the code base is the method.childReturnType rule. This is because Drupal Core is only on phpstan level 1 and return types are frequently mis-documented.

phpunit -- Unit tests

This section has moved to the Testing Documentation.