Bug 1508369 - add stylelint linter support to mach lint, r=Standard8,dao,linter-reviewers,sylvestre

Differential Revision: https://phabricator.services.mozilla.com/D177477
This commit is contained in:
Gijs Kruitbosch 2023-05-11 16:06:38 +00:00
parent 7ec07f0365
commit f238fa0035
18 changed files with 2937 additions and 264 deletions

View File

@ -1,9 +1,11 @@
# Please DO NOT add more third party files to this file.
# They should be added to tools/rewriting/ThirdPartyPaths.txt instead.
#
# Please also DO NOT add generated files that are for some reason checked
# into source - add them to tools/rewriting/Generated.txt instead.
# This file should only be used for exclusions where we have:
# - preprocessed files
# - generated files that are for some reason checked into source
# - intentionally invalid files
# - build directories and other items that we need to ignore

88
.stylelintignore Normal file
View File

@ -0,0 +1,88 @@
# Please DO NOT add more third party files to this file.
# They should be added to tools/rewriting/ThirdPartyPaths.txt instead.
#
# Please also DO NOT add generated files that are for some reason checked
# into source - add them to tools/rewriting/Generated.txt instead.
# This file should only be used for exclusions where we have:
# - preprocessed files
# - intentionally invalid files
# - build directories and other items that we need to ignore
# Always ignore node_modules.
**/node_modules/
# Always ignore crashtests - specially crafted files that originally caused a
# crash.
**/crashtests/
# Also ignore reftest - specially crafted to produce expected output.
**/reftest/
**/reftests/
# Exclude expected objdirs.
obj*/
# This file is generated in some way.
browser/components/pocket/content/panels/css/main.compiled.css
# Note that the debugger has its own stylelint setup, but that currently
# produces errors. Bug 1831302 tracks making this better
devtools/client/debugger/src/components/PrimaryPanes/Outline.css
devtools/client/debugger/src/components/PrimaryPanes/Sources.css
devtools/client/debugger/src/components/shared/AccessibleImage.css
devtools/client/debugger/src/utils/editor/source-editor.css
devtools/client/debugger/test/mochitest/examples/
# These get their sourcemap annotations autofixed, though they produce
# no errors at all.
devtools/client/inspector/rules/test/doc_sourcemaps.css
# Some of these produce parse errors, some have sourcemaps modified.
# They're tests, so let's just ignore all of them:
devtools/client/inspector/computed/test/doc_sourcemaps.css
devtools/client/inspector/rules/test/doc_invalid_sourcemap.css
devtools/client/shared/sourceeditor/test/css_statemachine_testcases.css
devtools/client/webconsole/test/browser/*.css
# Style editor tests check how it copes with invalid or "special" CSS,
# so don't try to "fix" those.
devtools/client/styleeditor/test/
# These are empty or have funky charsets
dom/base/test/bug466409-empty.css
dom/encoding/test/file_utf16_be_bom.css
dom/encoding/test/file_utf16_le_bom.css
dom/security/test/cors/file_cors_logging_test.html.css
dom/tests/mochitest/general/cssA.css
dom/tests/mochitest/general/cssC.css
# These are test-only and cause us to complain about font families or
# similar, but we don't want to touch these tests at this point.
dom/security/test/csp/file_CSP.css
dom/security/test/sri/style2.css
dom/xml/test/old/docbook.css
dom/xml/test/old/toc/book.css
dom/xml/test/old/toc/toc.css
# Tests we don't want to modify at this point:
layout/base/tests/bug839103.css
layout/inspector/tests/bug856317.css
layout/inspector/tests/chrome/test_bug467669.css
layout/inspector/tests/chrome/test_bug708874.css
layout/style/test/gtest/example.css
layout/style/test/mapped2.css
layout/style/test/unstyled-frame.css
# Empty test files:
netwerk/test/mochitests/test1.css
netwerk/test/mochitests/test2.css
# Has substitution gunk in it:
python/mozbuild/mozbuild/test/backend/data/build/foo.css
# This is third-party in a way:
toolkit/components/pdfjs/content/web/debugger.css
# Ignore web-platform tests as they are not necessarily under our control.
testing/web-platform/tests/

243
.stylelintrc.js Normal file
View File

@ -0,0 +1,243 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
/* eslint-env node */
"use strict";
const fs = require("fs");
const path = require("path");
function readFile(path) {
return fs
.readFileSync(path, { encoding: "utf-8" })
.split("\n")
.filter(p => p && !p.startsWith("#"));
}
const ignoreFiles = [
...readFile(
path.join(__dirname, "tools", "rewriting", "ThirdPartyPaths.txt")
),
...readFile(path.join(__dirname, "tools", "rewriting", "Generated.txt")),
...readFile(
path.join(__dirname, "devtools", "client", "debugger", ".stylelintignore")
).map(p => `devtools/client/debugger/${p}`),
];
module.exports = {
extends: ["stylelint-config-recommended"],
ignoreFiles,
rules: {
/* Disabled because of `-moz-element(#foo)` which gets misparsed. */
"color-no-invalid-hex": null,
"font-family-no-missing-generic-family-keyword": [
true,
{
ignoreFontFamilies: [
"-moz-button",
"-moz-field",
"-moz-fixed",
"-moz-list",
"caption",
],
},
],
"function-no-unknown": [
true,
{
ignoreFunctions: [
"-moz-image-rect" /* Used for cropping images */,
"add" /* Used in mathml.css */,
],
},
],
"no-descending-specificity": null,
"no-duplicate-selectors": null,
"property-no-unknown": [
true,
{
ignoreProperties: ["overflow-clip-box"],
},
],
/*
* XXXgijs: we would like to enable this, but we can't right now.
* This is because Gecko uses a number of custom pseudoclasses,
* and stylelint assumes that for `:unknown-pseudoclass(foo)`,
* `foo` should be a known type.
* This is tedious but workable for things like `-moz-locale-dir` where
* the set of acceptable values (ltr/rtl) is small.
* However, for tree cells, the set of values is unlimited (ie
* user-defined, based on atoms sent by the JS tree view APIs).
* There does not appear to be a way to exempt the contents of these
* unknown pseudoclasses, and as a result, this rule is not
* usable for us. The 'type' only includes the contents of the
* pseudoclass, not the pseudo itself, so we can't filter based on the
* pseudoclass either.
* Ideally, we would either create an option to the builtin rule
* in stylelint itself, or mimic the rule but exempt these, or
* add parser support for our custom pseudoclasses.
*
* For now, we just disable this rule.
*/
"selector-type-no-unknown": null,
/*
* See above - if we enabled this rule, we'd have to allow for a number
* of custom elements we use, which are listed here:
"selector-type-no-unknown": [
true,
{
ignore: ["custom-elements"],
ignoreTypes: [
// Modern custom element / storybooked components:
/^moz-/,
// moz-locale-dir trips this rule for some reason:
"rtl",
"ltr",
// Migrated XBL elements not part of core XUL that we use at the moment:
"findbar",
"panelmultiview",
"panelview",
"popupnotification",
"popupnotificationcontent",
// Legacy XUL elements:
// (the commented out ones used to be a thing and aren't used in-tree anymore)
"arrowscrollbox",
"box",
// "broadcaster",
// "broadcasterset",
"button",
"browser",
"checkbox",
"caption",
// clicktoscroll
// colorpicker
// column
// columns
"commandset",
"command",
// conditions
// content
// datepicker
"deck",
"description",
"dialog",
// dialogheader
"dropmarker",
"editor",
// grid
// grippy
"groupbox",
"hbox",
// iframe
// image
"key",
"keyset",
// label
"listbox",
// listcell
// listcol
// listcols
// listhead
// listheader
"listitem",
// member
"menu",
"menubar",
"menucaption",
"menuitem",
"menulist",
"menupopup",
"menuseparator",
"notification",
"notificationbox",
"observes",
// overlay
// page
"panel",
// param
"popupset",
// preference
// preferences
// prefpane
// prefwindow
// progressmeter
// query
// queryset
"radio",
"radiogroup",
// resizer
"richlistbox",
"richlistitem",
// row
// rows
// rule
// scale
// script
"scrollbar",
"scrollbox",
"scrollcorner",
"separator",
"spacer",
// spinbuttons
"splitter",
"stack",
// statusbar
// statusbarpanel
"stringbundle",
"stringbundleset",
"tab",
"tabbox",
"tabpanel",
"tabpanels",
"tabs",
// template
// textnode
"textbox",
// timepicker
"titlebar",
"toolbar",
"toolbarbutton",
// toolbargrippy
"toolbaritem",
"toolbarpalette",
"toolbarpaletteitem",
"toolbarseparator",
"toolbarset",
"toolbarspacer",
"toolbarspring",
"toolbartabstop",
"toolbox",
"tooltip",
"tree",
"treecell",
"treechildren",
"treecol",
"treecols",
"treeitem",
"treerow",
"treeseparator",
// triple
"vbox",
// where
"window",
"wizard",
"wizardpage",
],
},
],
*/
"selector-pseudo-class-no-unknown": [
true,
{
ignorePseudoClasses: ["popover-open"],
},
],
},
};

View File

@ -366,6 +366,7 @@ menupopup::part(drop-indicator) {
--arrowpanel-color: FieldText;
}
/* stylelint-disable-next-line media-feature-name-no-unknown */
@media (-moz-content-prefers-color-scheme: dark) and (not (prefers-contrast)) {
#DateTimePickerPanel {
color-scheme: dark;

View File

@ -54,6 +54,21 @@ In this document, we try to list these all tools.
- :ref:`Formatting C++ Code With clang-format`
- https://clang.llvm.org/docs/ClangFormat.html
.. list-table:: CSS
:widths: 20 20 20 20 20
:header-rows: 1
* - Tools
- Has autofixes
- Meta bug
- More info
- Upstream
* - Stylelint
- Yes
- `bug 1762027 <https://bugzilla.mozilla.org/show_bug.cgi?id=1762027>`__
- :ref:`Stylelint`
- https://stylelint.io/
.. list-table:: JavaScript
:widths: 20 20 20 20 20
:header-rows: 1
@ -79,8 +94,6 @@ In this document, we try to list these all tools.
- :ref:`JavaScript Coding style`
- https://prettier.io/
.. list-table:: Python
:widths: 20 20 20 20 20
:header-rows: 1

View File

@ -1,7 +1,7 @@
ESLint
======
`ESLint`_ is a popular linter for JavaScript. The ESLint integration also uses
`ESLint`__ is a popular linter for JavaScript. The ESLint integration also uses
`Prettier`_ to enforce code formatting.
Run Locally
@ -195,11 +195,11 @@ For test harness issues, file bugs in Developer Infrastructure :: Lint and Forma
eslint-plugin-mozilla
eslint-plugin-spidermonkey-js
.. _ESLint: http://eslint.org/
.. __: https://eslint.org/
.. _Prettier: https://prettier.io/
.. _Usage guide: ../usage.html
.. _ESLint's documentation: http://eslint.org/docs/user-guide/configuring
.. _eslint.org's rule list: http://eslint.org/docs/rules/
.. _ESLint's documentation: https://eslint.org/docs/user-guide/configuring
.. _eslint.org's rule list: https://eslint.org/docs/rules/
.. _eslint-plugin-mozilla: eslint-plugin-mozilla.html
.. _eslint-plugin-spidermonkey-js: eslint-plugin-spidermonkey-js.html
.. _informed that it is a module: https://searchfox.org/mozilla-central/rev/9399e5832979755cd340383f4ca4069dd5fc7774/browser/base/content/.eslintrc.js

View File

@ -0,0 +1,77 @@
Stylelint
=========
`Stylelint`__ is a popular linter for CSS.
Run Locally
-----------
The mozlint integration of Stylelint can be run using mach:
.. parsed-literal::
$ mach lint --linter stylelint <file paths>
Alternatively, omit the ``--linter stylelint`` and run all configured linters, which will include
Stylelint.
Stylelint also supports the ``--fix`` option to autofix most errors raised from most of the rules.
See the `Usage guide`_ for more options.
Understanding Rules and Errors
------------------------------
* Only some files are linted, see the :searchfox:`configuration <tools/lint/stylelint.yml>` for details.
* By design we do not lint/format reftests not crashtests as these are specially crafted tests.
* If you don't understand a rule, you can look it in `stylelint.io's rule list`_ for more
information about it.
Common Issues and How To Solve Them
-----------------------------------
This code should neither be linted nor formatted
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* If it is a third-party piece of code, please add it to :searchfox:`ThirdPartyPaths.txt <tools/rewriting/ThirdPartyPaths.txt>`.
* If it is a generated file, please add it to :searchfox:`Generated.txt <tools/rewriting/Generated.txt>`.
* If intentionally invalid, please add it to :searchfox:`.stylelintignore <.stylelintignore>`.
Configuration
-------------
The global configuration file lives in ``topsrcdir/.stylelintrc.js``.
For an overview of the supported configuration, see `Stylelint's documentation`_.
Please keep differences in rules across the tree to a minimum. We want to be consistent to
make it easier for developers.
Sources
-------
* :searchfox:`Configuration (YAML) <tools/lint/stylelint.yml>`
* :searchfox:`Source <tools/lint/stylelint/__init__.py>`
Builders
--------
`Gijs Kruitbosch (gijs) <https://people.mozilla.org/s?query=gijs>`__ owns
the builders. Questions can also be asked on #lint:mozilla.org on Matrix.
Stylelint task
^^^^^^^^^^^^^^
This is a tier-1 task. For test failures the patch causing the
issue should be backed out or the issue fixed.
Some failures can be fixed with ``./mach lint -l stylelint --fix path/to/file``.
For test harness issues, file bugs in Developer Infrastructure :: Lint and Formatting.
.. __: https://stylelint.io/
.. _Usage guide: ../usage.html
.. _Stylelint's documentation: https://stylelint.io/user-guide/configure/
.. _stylelint.io's rule list: https://stylelint.io/user-guide/rules/

2466
package-lock.json generated

File diff suppressed because it is too large Load Diff

View File

@ -26,6 +26,8 @@
"eslint-plugin-spidermonkey-js": "file:tools/lint/eslint/eslint-plugin-spidermonkey-js",
"jsdoc": "4.0.2",
"prettier": "1.19.1",
"stylelint": "^15.6.0",
"stylelint-config-recommended": "^12.0.0",
"yarn": "1.22.19"
},
"notes(private)": "We don't want to publish to npm, so this is marked as private",

View File

@ -152,6 +152,25 @@ eslint-build:
- artifact: target.xpt_artifacts.zip
dest: xpt_artifacts
stylelint:
description: CSS lint check
treeherder:
symbol: stylelint
run:
using: run-task
cwd: '{checkout}'
command: >
cp -r /build/node_modules_eslint node_modules &&
./mach lint -v -l stylelint -f treeherder -f json:/builds/worker/mozlint.json .
when:
files-changed:
# Files that are likely audited.
- '**/*.css'
- 'tools/lint/styleint.yml'
# Run when stylelint policies change.
- '**/.stylelintignore'
- '**/*stylelintrc*'
license:
description: Check for license blocks in source files.
treeherder:

View File

@ -48,6 +48,7 @@ analysing the code with a linter.
"``ES-build``", "`eslint-build </code-quality/lint/linters/eslint.html#eslint-build-es-b>`_", "All", "Extended javascript analysis that uses build artifacts."
"``mocha(EPM)``", "`ESLint-plugin-mozilla </code-quality/lint/linters/eslint-plugin-mozilla.html>`__", "Desktop", "The ESLint plugin rules."
"``f8``", "`flake8 </code-quality/lint/linters/flake8.html>`__", "All", "Python analyzed for style and correctness."
"``stylelint``", "`Stylelint </code-quality/lint/linters/stylelint.html>`__", "All", "CSS is analyzed for correctness."
"``W``", "`wpt lint </web-platform/index.html>`__", "Desktop", "web-platform-tests analyzed for style and manifest correctness"
"``WR(tidy)``", "`WebRender servo-tidy </testing/webrender/index.html>`__", "Desktop", "Code in gfx/wr is run through servo-tidy."
"``A``", "`Spotless </code-quality/lint/linters/android-format.html>`_", "Android", "Java is analyzed for style and correctness."

View File

@ -1,9 +1,9 @@
[
{
"filename": "eslint.tar.gz",
"size": 24096013,
"size": 25359680,
"algorithm": "sha512",
"digest": "dcf615e3d47219b6e6e7eeef9f72559bf0c59d0c503ec3d4e784cf0f2ad0178d6dac9341486ec6cb8dcfbaf02718e93470888ff7e8a3c16ca3171514f4b8136f",
"digest": "21e1bf49436755754014a0594c71cf7bbdabdbff4f504ff5c4881affa1b6f206b6a7ebbf68680badc42136245b57032cee5a968f8c5c6dac9b066facf23c1263",
"unpack": true,
"visibility": "public"
}

15
tools/lint/stylelint.yml Normal file
View File

@ -0,0 +1,15 @@
---
stylelint:
description: CSS linter
# Stylelint infra handles its own path filtering, so just include cwd
include: ['.']
exclude: []
extensions: ['css']
support-files:
- 'package.json'
- '**/.stylelintrc.json'
- '.stylelintignore'
- 'tools/lint/stylelint/**'
type: external
payload: stylelint:lint
setup: stylelint:setup

View File

@ -0,0 +1,184 @@
# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
# vim: set filetype=python:
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
import json
import os
import re
import signal
import subprocess
import sys
sys.path.append(os.path.join(os.path.dirname(__file__), "eslint"))
from eslint import setup_helper
from mozbuild.nodeutil import find_node_executable
from mozlint import result
STYLELINT_ERROR_MESSAGE = """
An error occurred running stylelint. Please check the following error messages:
{}
""".strip()
STYLELINT_NOT_FOUND_MESSAGE = """
Could not find stylelint! We looked at the --binary option, at the STYLELINT
environment variable, and then at your local node_modules path. Please install
eslint, stylelint and needed plugins with:
mach eslint --setup
and try again.
""".strip()
FILE_EXT_REGEX = re.compile(r"\.[a-z0-9_]{2,10}$", re.IGNORECASE)
def setup(root, **lintargs):
setup_helper.set_project_root(root)
if not setup_helper.check_node_executables_valid():
return 1
return setup_helper.eslint_maybe_setup()
def lint(paths, config, binary=None, fix=None, rules=[], setup=None, **lintargs):
"""Run stylelint."""
log = lintargs["log"]
setup_helper.set_project_root(lintargs["root"])
module_path = setup_helper.get_project_root()
modified_paths = []
exts = "*.(" + "|".join(config["extensions"]) + ")"
for path in paths:
filepath, fileext = os.path.splitext(path)
if fileext:
modified_paths += [path]
else:
modified_paths += [path + "**" + os.path.sep + exts]
# Valid binaries are:
# - Any provided by the binary argument.
# - Any pointed at by the STYLELINT environmental variable.
# - Those provided by |mach lint --setup|.
if not binary:
binary, _ = find_node_executable()
if not binary:
print(STYLELINT_NOT_FOUND_MESSAGE)
return 1
extra_args = lintargs.get("extra_args") or []
exclude_args = []
for path in config.get("exclude", []):
exclude_args.extend(
["--ignore-pattern", os.path.relpath(path, lintargs["root"])]
)
# First run Stylelint
cmd_args = (
[
binary,
os.path.join(
module_path, "node_modules", "stylelint", "bin", "stylelint.js"
),
"--formatter",
"json",
"--allow-empty-input",
"--config",
os.path.join(lintargs["root"], ".stylelintrc.js"),
]
+ extra_args
+ exclude_args
+ modified_paths
)
if fix:
cmd_args.append("--fix")
log.debug("Stylelint command: {}".format(" ".join(cmd_args)))
result = run(cmd_args, config, fix)
if result == 1:
return result
return result
def run(cmd_args, config, fix):
shell = False
if (
os.environ.get("MSYSTEM") in ("MINGW32", "MINGW64")
or "MOZILLABUILD" in os.environ
):
# The stylelint binary needs to be run from a shell with msys
shell = True
encoding = "utf-8"
orig = signal.signal(signal.SIGINT, signal.SIG_IGN)
proc = subprocess.Popen(
cmd_args, shell=shell, stdout=subprocess.PIPE, stderr=subprocess.PIPE
)
signal.signal(signal.SIGINT, orig)
try:
output, errors = proc.communicate()
except KeyboardInterrupt:
proc.kill()
return {"results": [], "fixed": 0}
if errors:
errors = errors.decode(encoding, "replace")
print(STYLELINT_ERROR_MESSAGE.format(errors))
# 0 is success, 2 is there was at least 1 rule violation. Anything else
# is more serious.
if proc.returncode != 0 and proc.returncode != 2:
if proc.returncode == 78:
print("Stylelint reported an issue with its configuration file.")
print(output)
return 1
if not output:
return {"results": [], "fixed": 0} # no output means success
output = output.decode(encoding, "replace")
try:
jsonresult = json.loads(output)
except ValueError:
print(STYLELINT_ERROR_MESSAGE.format(output))
return 1
results = []
fixed = 0
for obj in jsonresult:
errors = obj["warnings"] + obj["parseErrors"]
# This will return a number of fixed files, as that's the only thing
# stylelint gives us. Note that it also seems to sometimes list files
# like this where it finds nothing and fixes nothing. It's not clear
# why... but this is why we also check if we were even trying to fix
# anything.
if fix and not errors and not obj.get("ignored"):
fixed += 1
for err in errors:
msg = err.get("text")
if err.get("rule"):
# stylelint includes the rule id in the error message.
# All mozlint formatters that include the error message also already
# separately include the rule id, so that leads to duplication. Fix:
msg = msg.replace("(" + err.get("rule") + ")", "").strip()
err.update(
{
"message": msg,
"level": err.get("severity") or "error",
"lineno": err.get("line") or 0,
"path": obj["source"],
"rule": err.get("rule") or "parseError",
}
)
results.append(result.from_config(config, **err))
return {"results": results, "fixed": fixed}

View File

@ -0,0 +1,5 @@
#foo {
/* Duplicate property: */
font-size: 12px;
font-size: 12px;
}

View File

@ -26,5 +26,7 @@ requirements = tools/lint/rst/requirements.txt
requirements = tools/lint/python/ruff_requirements.txt
[test_rustfmt.py]
[test_shellcheck.py]
[test_stylelint.py]
skip-if = os == "win" # busts the tree for subsequent tasks on the same worker (bug 1708591)
[test_trojan_source.py]
[test_yaml.py]

View File

@ -0,0 +1,65 @@
import mozunit
import pytest
from conftest import build
LINTER = "stylelint"
fixed = 0
@pytest.fixture
def stylelint(lint):
def inner(*args, **kwargs):
# --ignore-path is because stylelint doesn't have the --no-ignore option
# and therefore needs to be given an empty file for the tests to work.
kwargs["extra_args"] = [
"--ignore-path=tools/lint/test/files/eslint/testprettierignore",
]
return lint(*args, **kwargs)
return inner
def test_lint_with_global_exclude(lint, config, paths):
config["exclude"] = ["subdir", "import"]
# This uses lint directly as we need to not ignore the excludes.
results = lint(paths(), config=config, root=build.topsrcdir)
assert len(results) == 0
def test_no_files_to_lint(stylelint, config, paths):
# A directory with no files to lint.
results = stylelint(paths("nolint"), root=build.topsrcdir)
assert results == []
# Errors still show up even when a directory with no files is passed in.
results = stylelint(paths("nolint", "subdir/bad.css"), root=build.topsrcdir)
assert len(results) == 1
def test_stylelint(stylelint, config, create_temp_file):
contents = """#foo {
font-size: 12px;
font-size: 12px;
}
"""
path = create_temp_file(contents, "bad.css")
results = stylelint([path], config=config, root=build.topsrcdir)
assert len(results) == 1
def test_stylelint_fix(stylelint, config, create_temp_file):
contents = """#foo {
font-size: 12px;
font-size: 12px;
}
"""
path = create_temp_file(contents, "bad.css")
stylelint([path], config=config, root=build.topsrcdir, fix=True)
# stylelint returns counts of files fixed, not errors fixed.
assert fixed == 1
if __name__ == "__main__":
mozunit.main()