Add stylelint CSS linting

[afercia]

Trac ticket: https://core.trac.wordpress.org/ticket/29792

Work In Progress (WIP) to add Stylelint CSS coding standards rules enforcement to Core.

Documenting the work done so far:

Core

Two new npm scripts are added together with a Grunt task that can be run individually and is also part of grunt precommit:css. Under the hood, they use the lint-style wp-script:

"lint:css": "wp-scripts lint-style \"src/**/*.{css,scss}\""
"lint:css:fix": "npm run lint:css -- --fix"
grunt lint:css

The linting rules are defined in the .stylelintrc.js file in the root of the project.
Paths and files to be excluded are defined in .stylelintignore.
The themes directory is excluded.

Themes

Some bundled themes already have their own Stylelint scripts:

• Twenty Twenty: lint:css as a standalone script that is not part of the build process. Only for .css files. Uses lint-style wp-script.
• Twenty Twenty-One: lint:scss and lint-fix:scss for the .scss files. Plus, build:stylelint, which is part of the build process, for the .css files. All the thre scripts use stylelint directly.

These existing scripts use their own Stylelint configuration. It made sense when these two themes were under development. I think these should be removed in favor of a centralized Stylelint configuration in core that lints also the themes. This PR adds:

"lint:css:themes": "wp-scripts lint-style \"src/wp-content/themes/**/*.{css,scss}\" --ignore-path .stylelintignore-themes",
"lint:css:themes:fix": "npm run lint:css:themes -- --fix",

They use the same rules defined in the .stylelintrc.js file in the root of the project.
Paths to be excluded are defined in .stylelintignore-themes instead.

Note on the existing config in Twenty Twenty:
The lint-style script from wp-scripts doesn't walk the directory tree upwards to auto-discover a Stylelint configuration. It expects a configuration in the theme's root. Instead, when used directly, Stylelint does. This doesn't allow to use the Core configuration from the theme.
Also, Twenty Twenty uses the stylelint-a11y Stylelint 'plugin' to add two lint rules:

"a11y/no-outline-none": true,
"a11y/selector-pseudo-class-focus": true

If the 'ad-hoc' configurations for the two themes get removed in favor of a centralized configuration, these two a11y ruels would be lost. A decision should be made on whether to add them to the centralized configuration.

Rules

The WordPress CSS Coding Standards (more readable in their GitHub page version) are described in a conversational language style and are difficult to summarize point by point. Extracting some of the most important ones to document the related rules.

The rules configuration extends the rules from the @wordpress/stylelint-config/scss-stylistic one and add or change rules to cover the following points:

One blank line between blocks in a section

• rule-empty-line-before
• at-rule-empty-line-before
• @stylistic/max-empty-lines set to 1 instead of 2

Each selector should be on its own line

This was tricky to address as some rules conflict. To make sure indentation linting is performed correctly, a specific order of some rules needs to be preserver.

'@stylistic/block-closing-brace-newline-before': 'always',
'@stylistic/block-opening-brace-newline-after': 'always',
'@stylistic/declaration-block-semicolon-newline-after': 'always',
'@stylistic/selector-list-comma-space-after': 'always-single-line',
'@stylistic/selector-list-comma-newline-after': 'always',
'@stylistic/declaration-colon-newline-after': 'always-multi-line',
'@stylistic/indentation': 'tab',

All properties and values should be lowercase, except for font names and vendor-specific properties

• already inherits @stylistic/property-case
• added: value-keyword-case with exceptions for currentColor and optimizeLegibility

Avoid RGB format

• added 'function-disallowed-list': ['rgb']

Line height should also be unit-less, unless necessary to be defined as a specific pixel value.

• added declaration-property-unit-allowed-list with 'line-height': []

Font weights should be defined using numeric values

• added 'font-weight-notation': 'numeric'

Refrain from using over-qualified selectors, div.container can simply be stated as .container

Right now, there are 1815 violations of this rule in Core. It's a lot. Things like input[type="text"] or simply a.current are considered invalid. As such, I added the rule selector-no-qualifying-type and changed the severity type to warning. A decision on how to handle these warnings can be made later.

Remove multiple spaces between selector combinators

This is not mentioned in the coding standards but I addressed it anyways to cover cases like
.myclass1 .myclass2 {}
The added rule is: '@stylistic/selector-descendant-combinator-no-non-space': true.

Recommended changes to the CSS Coding Standards

Similar to the WordPress PHP Coding Standards for file names, use lowercase and separate words with hyphens when naming selectors. Avoid camelcase and underscores.

Gutenberg already uses a BEM-like naming convention that uses underscores for class selectors. Some new selectors recently introduced in core already follow that convention e.g.: .wp-tooltip__toggle.
On the other hand, there are several violations already in core for both class and ID selectors. A few examples:

• .ac_match
• .privacy_requests
• #login_error
• #dashboard_right_now
• #TB_window

All of these aren't allowed by the current CSS Coding Standarrds. They can't be changed though, because of backward compatibility concerns.

As such, the current recommendation to use hyphens and avoid underscores isn't applicable.

Add two blank lines between sections

This is not doable with Stylelint. The rule '@stylistic/max-empty-lines': 1 cannot distinguish between nprmal comments and section comments. Two blank lines between sections add little value anyways. Suggest to remove this point from the Coding Standards.

Long comments should manually break the line length at 80 characters.

@stylistic/max-line-length can be used to set a global maximum line length. It cannot distinguish between lines of code and lines of comments. Either we set a global 80 characters limit or we should use another tool e.g. a custom postcss script to apply the line length limit only to comments. A little overkill to me. We could just decide to not lint it or change this point in the Coding Standards.

Property Ordering

All the points in this sections are not enforceable with Stylelint. They can be kept as genereal recommendation but they will always be subject to personal preferences.

Testing

Don't forget to run npm install before testing.

For now, it is important to check the following:

• Whether the excluded paths make sense.
• Whether there is the need to add more files to scan.
• Whether there is the need to add more rules or adjusts the existing ones.
• Make sure that the results are consistent when running npm run lint:css and grunt lint:css.

Rather than running the linting on the whole codebase with npm run lint:css or npm run lint:css:fix I suggest to also start with lintin single files so to get a clearer view of how it works. Start with a small file, for example:

npx stylelint src/wp-admin/css/color-picker.css

Then, run the inter with the auto-fix flag:

npx stylelint src/wp-admin/css/color-picker.css --fix

Then, try larger files, for example:

npx stylelint src/wp-includes/css/media-views.css
npx stylelint src/wp-admin/css/common.css

As of Core revision 63292, the Stylelint scan reports the following:

✖ 4988 problems (3173 errors, 1815 warnings)
    2527 errors potentially fixable with the "--fix" option.

Most of these errors are trivial fixes. Running the auto-fix will reduce the errors to 646. They will need to either be manually fixed or have a stylelint-disable comment when appropriate.

Use of AI Tools

None.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[afercia]

Nice, given the new lint:css grunt task is added to grunt precommit:css, it already runs on the GitHub Actions and the Run SASS precommit tasks step of the update-built-files job fails.

[afercia]

The first run of the css linter job on this PR reports 2746 errors while locally I get 2823 errors. Will look into it. At a first check it appears some files aren't scanned in the job:

• All files within src/wp-includes/js/ are missing.
• All admin color schemes files except one are missing. For example:
  • src/wp-admin/css/colors/midnight/colors.scss is missing
  • src/wp-admin/css/colors/sunrise/colors.scss is there instead
  • I guess all the admin scheme css files should be excluded as they are auto-generated.

[afercia]

The bundled themes twentytwenty and twentytwentyone have their own Stylelint configuration.

The one for twentytwenty is broken:

• It uses the stylelint-config-wordpress package, which is now deprecated.
• It uses the stylelint-a11y plugin, which is unmaintained and imcompatible with latest versions of Stylelint.

Although there are working forks of the stylelint-a11y plugin, I'd rather entirely remove the Stylelint configuration for both themes.

Even after making them work, the linting for both themes reports several errors, mostly because of outdated configuration:

• twentytwenty: 372 errors
• twentytwentyone: 135 errors

It appears the linting scripts of these two themes haven't been used for a long time. Likely, they have been intensively used during the themes development but now they are way behind.

Also, bundled themes are part of Core. To me, it makes sense to have a ceentralized tool to lint everything.

In the latest commit I added a npm run lint:css:themes script that only scans the themes directory, which is excluded from the main npm run lint:css. It reports 15570 errors :)

As said earlier, for now it's best to focus on the functionality. The set of rules can be refined later. It will need some adjustments as some rules aren't applicable in Core, for example selector-class-pattern and selector-id-pattern are specific to Gutenberg.

[afercia]

Regarding the a11y rules that were used in twentytwenty, I tried to re-add them to the Core Stylelint configuration by using the compatible stylelint-a11y plugin fork. It's a useful experiment to check a few CSS patterns that are potentially harmful for accessibility:

a11y/no-outline-none
Only 5 occurrences detected. This rule attempts to detext the removal of the outline for a cofus style without an alternative e.g. a border or box-shadow change, While it can't be fully trusted, I'd find useful adding in to the configuration.

a11y/selector-pseudo-class-focus
92 occurrences. This one is a little trickier. It considers invalid any 'hover' style that is not accompanined by a :focusstyle. For example, it expects that.nav-tab:hoveris used together with:focus. I tend to think this rule would produce too many false positives. In core, the focus style is often provided by styles that are not paired with the :hover` style.

Among other rules provided by the plugin, this one might be useful:

media-prefers-reduced-motion
It tries to detect when an animation is used outside of a media-prefers-reduced-motion media query, which is something we observed it is sometimes missed during development.

I would suggest to experiment these rules at a later stage, after the initial configuration is proved to be stable and reliable,