Linting and Formatting

Series: Drupal Docker

tl;dr: PHP Linting and CS Testing in VS Code and as CI/CD jobs can help reinforced Drupal best practices.

This series has now covered all the core functionality of having a local development environment and deploying updates. The majority of what's left has to do with automated testing.

Within VS Code, there are a couple of useful features:

1. warnings and errors that show me when I have violated best practices or worse, introduced a serious problem
2. automatic formatting, to fix those violations which are easy to fix, like not having a new line where I should or the order of attributes in a CSS rule.

With Drupal, there are three main types of code: PHP, CSS, and JavaScript. Maybe there's also some useful tools for twig, but I haven't looked into that at this point. For each of those, styles can be encouraged in the development environment, and tests can also be added in the CI/CD pipeline to make sure there are no errors before merging.

Note that the CI/CD jobs here are assuming this order of operations:

1. Validate: tests that don't need a freshly-built complete image.
2. Build: build the updated image.
3. Test: tests that require the full image to be able to run.

PHP

Basic Linting

PHP linting checks for any fatal errors. These kinds of errors are a lot harder to keep in the code long enough to commit it into GitLab when I was using a full dedicated developer environments with things like syntax and error highlighting, because I'm probably going to notice it first. With that said, it is still a good failsafe to have, and it doesn't take that long to run.

Here's a GitLab CI/CD approach to scan any PHP changes in case of any fatal error, starting with a general extendable job which will scan all files recursively in any specified directories.

.php_lint:
  stage: validate
  image: drupal:php8.3-apache
  variables:
    DIRECTORIES: "./"
    EXTENSIONS: "php"
  script:
    # Recursively checks for files of specified extensions in specified directories and completes php lint on them. #
    - cwd="$(pwd)"
    - |
      for DIRECTORY in $DIRECTORIES
        do
          cd $DIRECTORY
          for EXT in $EXTENSIONS
            do
              files="$(find -name *.${EXT} -type f)"

              for file in ${files}
                do php -l ${file};
              done;
            done;
          cd $cwd;
        done;

Each project can then set up up a CI/CD job to implement it, like this one that runs on any custom code change as well as on dev and staging, checking each of those custom code directories:

include:
  - project: "ryan-l-robinson/gitlab-ci"
    ref: main
    file: test.yml

## Stages ##
stages:
  - test

php_lint:
  extends: .php_lint
  variables:
    DIRECTORIES: ./web/themes/custom ./web/modules/custom
    EXTENSIONS: php module theme inc
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
      when: never
    - if: '$CI_COMMIT_BRANCH == "dev" || $CI_COMMIT_BRANCH == "staging"'
    - changes:
        - web/modules/custom/**/*
        - web/themes/custom/**/*

PHP CodeSniffer

CodeSniffer helps enforce more strict coding standards. There are standards defined for both Drupal and DrupalPractice. Drupal covers a lot of the basic bare minimum compliance with Drupal standards: indentation, style, and file structure. DrupalPractice goes further to offer deeper best practices. These can help find things like type mismatches or using a \Drupal:: service call instead of dependency injection inside of a class.

PHP CS in Local Developer Environment

Drupal by default will include CS packages in the composer vendors, so there's nothing extra needed to get those packages in place. I could already manually run a command from the terminal in the container like this:

/opt/drupal/vendor/bin/phpcs --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,theme" web/themes/custom web/modules/custom --ignore=*/vendor/*

That checks both standards against all files in the custom folders of both modules and themes, and ignores any vendor files that get included.

What I want, though, is to tell my VS Code environment to be running those tests in place so that I see errors right away, without needing to run the command line check every time myself. To enable that, I added these lines to the .devcontainer.json:

			"settings": {
        "phpcs.standard": "Drupal,DrupalPractice",
        "phpcs.executablePath": "/opt/drupal/vendor/bin/phpcs",
			}

Now PHPCS in VS Code knows which standards to check and which executable to run to check them.

PHP CS in CI/CD

This one is a one-line script, running that line that I shared earlier, so I haven't bothered to split it into a dedicated separate template. This is just one job, in the project's .gitlab-ci.yml:

### CodeSniffer for coding style ###
php_cs:
  stage: test
  image: $CI_REGISTRY_IMAGE/web:$CI_COMMIT_REF_SLUG
  script:
    - /opt/drupal/vendor/bin/phpcs --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,theme" web/themes/custom web/modules/custom --ignore=*/vendor/*
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
      when: never
    - if: '$CI_COMMIT_BRANCH == "dev"'
    - if: '$CI_COMMIT_BRANCH == "staging" || $CI_COMMIT_BRANCH == "main"'
      when: never
    - changes:
        - web/modules/custom/**/*
        - web/themes/custom/**/*

The key thing to note here, unlike with PHP linting, is that this needs to run after an updated web image is built. PHP linting is looking for the kind of errors that are visible simply by reading the code, so it can run before the build and if something fails, I don't even need to waste the time on the build. PHP CodeSniffer, though, needs some vendor files which only happen when the composer packages are installed, which is normally part of the build process.

CSS

CSS stylesheets can have their stylesheets enforced using stylelint and there is specific Drupal recommendations available as stylelint-config-drupal.

CSS Stylelint in VS Code

The necessary VS Code extension to include is "stylelint.vscode-stylelint". In addition to having that in the .devcontainer, though, there are a few other settings to help:

      "settings": {
        "editor.defaultFormatter": "esbenp.prettier-vscode",
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
          "source.fixAll.stylelint": "explicit",
        },
        "css.validate": false,
        "[css]": {
          "editor.defaultFormatter": null,
          "editor.formatOnSave": false
        },
			}

Here the default formatter is set to Prettier. That is useful for the JavaScript which I'll get into next. CSS is the exception to wanting Prettier styles, though, because I am going to want the specific Drupal recommended styles instead. Here in devcontainer that means I set the defaultFormatter to Prettier, but then disable that on CSS. I also disable the default CSS valdiation. Instead, I add a codeActionsOnSave that will automatically fix any issues found by stylelint.

The other component required is to provide the specific Drupal styles and tell VS Code to use it. In the postCreateCommand, I add all the necessary packages, which in this case is a simple one, adding the general stylelint package and the Drupal specific rules.

# Add CSS style linting
npm install --save-dev stylelint stylelint-config-drupal

Finally, the last essential file is to tell the VS Code extension to enforce specifically the Drupal style. This is done with a .stylelintrc.json file at the project root, with this:

{
    "extends": "stylelint-config-drupal"
}

CSS Stylelint in CI/CD

Running this test in CI/CD is going to rely on having some of the same packages and will also require that .stylelintrc.json file. With those in place, I can run a CI/CD job like this:

css_lint:
  extends: .css_lint
  script:
    - npm install -save-dev stylelint stylelint-config-drupal
    - npx stylelint --allow-empty-input web/themes/custom/**/*.css web/modules/custom/**/*.css
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
      when: never
    - if: "$CI_COMMIT_BRANCH =~ /^202/ || $CI_COMMIT_BRANCH =~ /^(dev|staging|main)$/"
    - changes:
        - web/modules/custom/**/*.css
        - web/themes/custom/**/*.css

JavaScript with ESLint and Prettier

Finally, we have JavaScript, which was a little more complicated than CSS, but in a few ways is a similar concept: we need a formatter, we need errors/warnings, we need a configuration file

ESLint in VS Code

The extension I needed was "dbaeumer.vscode-eslint".

The other devcontainer settings are a bit more complicated than with CSS:

      "settings": {
        "editor.defaultFormatter": "esbenp.prettier-vscode",
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
          "source.fixAll.eslint": "explicit"
        },
        "eslint.nodePath": "/opt/drupal/node_modules",
        "eslint.options": {
          "overrideConfigFile": "/opt/drupal/eslint.config.mjs"
        },
        "eslint.run": "onSave",
        "eslint.useFlatConfig": true,
        "eslint.validate": [
          "javascript",
          "yaml"
        ],
        "eslint.workingDirectories": [
          {
            "directory": "/opt/drupal",
            "!cwd": true
          }
        ],
			}

These settings were arrived to after a lot of trial and error, so there may be a cleaner version that works perfectly fine, but at least for now I'm not going to mess with something that is currently working.

The specific packages are also a bit more complicated because there are some conflicts in versions required. This combination worked for me to put in the postCreateCommand:

# Add JS style linting
npm install \
  eslint@^8 \
  @eslint/eslintrc \
  eslint-config-drupal@5.0.2 \
  eslint-config-prettier@latest \
  globals \
  prettier@^3

Finally, as with stylelint, we need a configuration file in the project. That is named eslint.config.mjs (or something else and set the other name in the devcontainer setting above), and looks something like this:

import path from "node:path";
import { fileURLToPath } from "node:url";
import { FlatCompat } from "@eslint/eslintrc";
import eslintConfigPrettier from "eslint-config-prettier/flat";
import globals from "globals";

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

const compat = new FlatCompat({
    baseDirectory: __dirname,
});

export default [
    // Don’t traverse core/contrib/etc.
    {
        ignores: [
            "**/node_modules/**",
            "**/vendor/**",
            "web/core/**",
            "web/modules/contrib/**",
            "web/themes/contrib/**",
        ],
    },

    // Use Drupal’s shareable config under flat-config.
    ...compat.extends("drupal"),

    // Project-wide tweaks.
    {
        files: ["web/themes/custom/**/*.js", "web/modules/custom/**/*.js"],
        languageOptions: {
            globals: {
                ...globals.browser,
                window: "readonly",
                document: "readonly",
                Drupal: "readonly",
                once: "readonly",
                tabbable: "readonly",
            },
            ecmaVersion: "latest",
            sourceType: "module",
        },
        // Stops the "must be semver or detect" warnings from eslint-plugin-react.
        settings: { react: { version: "9999" } },
        // If a shareable config enabled eslint-plugin-prettier, neutralize the rule here.
        rules: { "prettier/prettier": "off" },
    },

    // Keep ESLint style rules from fighting Prettier (we run Prettier separately).
    eslintConfigPrettier,
];

Again, this had some complications, but the main things it does:

1. Specifies which files to check and which to ignore.
2. Includes some global JavaScript that is available from Drupal.
3. Stop an error about React version numbers, since these sites don't even use React.
4. Stop it from conflicting with prettier standards.

The one other caveat for this one: the necessary packages only install in the postCreateCommand. The first time creating a new devcontainer environment, the eslint extension will have already started before the required packages are all there, and that means it will fail to be running in the background checking the files already. So, on first time after each rebuild, I have to use the command palette to start the ESLint server again. Otherwise, it will be working after closing VS Code and starting it up again, so this is only a nuisance if I immediately rebuilt the devcontainer and wanted to check JavaScript right away. That's not true for the PHP and CSS improvements; those ones will work immediately even without closing and reopening Code.

ESLint in CI/CD

As with CSS Stylelint, the last step is to add a CI/CD job that runs these tests on commits that change any JS file:

.js_lint:
  image: node:18
  stage: validate
  variables:
    # Adjust per project; newline-separated directories to include
    ESLINT_INCLUDE: web/themes/custom/**/*.css web/modules/custom/**/*.css
    # Include copying git submodules. #
    GIT_STRATEGY: clone
    GIT_SUBMODULE_STRATEGY: recursive
    GIT_SUBMODULE_UPDATE_FLAGS: --init
    GIT_SUBMODULE_FORCE_HTTPS: "true"
  script:
    - npm install --save-dev eslint@^8 @eslint/eslintrc eslint-config-drupal@5.0.2 eslint-config-prettier@latest prettier@^3
    - npx eslint --no-error-on-unmatched-pattern $ESLINT_INCLUDE
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
      when: never
    - if: "$CI_COMMIT_BRANCH =~ /^202/ || $CI_COMMIT_BRANCH =~ /^(dev|staging|main)$/"
    - changes:
        - web/modules/custom/**/*.js
        - web/themes/custom/**/*.js