From ec5fa1d4c0ee7ec6d1cac04433767211cfdf0045 Mon Sep 17 00:00:00 2001 From: hannajones Date: Wed, 21 May 2025 03:51:45 +0000 Subject: [PATCH] Bug 1606785 - Enable Prettier for CSS files r=desktop-theme-reviewers,Standard8,frontend-codestyle-reviewers,emilio Differential Revision: https://phabricator.services.mozilla.com/D248105 --- .prettierignore | 86 +++++++++++++ .prettierignore-css | 2 + .prettierignore-non-css | 9 ++ .prettierrc.js | 10 ++ .stylelintignore | 1 + .../mozbuild/backend/mach_commands.py | 1 + taskcluster/kinds/source-test/mozlint.yml | 1 + tools/lint/eslint/__init__.py | 112 +---------------- tools/lint/eslint/prettier_utils.py | 117 ++++++++++++++++++ tools/lint/file-perm.yml | 1 + tools/lint/mach_commands.py | 2 +- tools/lint/stylelint/__init__.py | 87 ++++++++----- 12 files changed, 288 insertions(+), 141 deletions(-) create mode 100644 .prettierignore-css create mode 100644 .prettierignore-non-css create mode 100644 tools/lint/eslint/prettier_utils.py diff --git a/.prettierignore b/.prettierignore index 3066f2054365..fdd4d79fc513 100644 --- a/.prettierignore +++ b/.prettierignore @@ -8,6 +8,7 @@ !*.xhtml !*.html !*.ts +!*.css # Prettier currently fails to parse this. toolkit/components/extensions/types/ext-tabs-base.d.ts @@ -1256,6 +1257,91 @@ toolkit/components/uniffi-bindgen-gecko-js/components/generated/* # test sourcemaps. devtools/client/inspector/rules/test/doc_sourcemaps2.css +############################################################################## +# The list below is copied from .stylelintignore. Prettier doesn't currently +# support multiple ignore files or dynamic ignore configurations. +# When this is implemented, we'll update the configuration below (bug 1825508) +############################################################################## + +# These files are generated in some way. +browser/components/pocket/content/panels/css/main.compiled.css +browser/components/aboutwelcome/**/*.css +browser/components/asrouter/**/*.css +browser/extensions/newtab/**/*.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 + +# This is intended to simulate a css file generated from a scss file in order to +# test sourcemaps. +devtools/client/inspector/rules/test/doc_sourcemaps2.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/stylesheet_change_events.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 + +# Bug 1893763 +mobile/android/android-components/components/feature/readerview/src/main/assets/extensions/readerview/readerview.css +# Three dashes at top of file (for Jekyll?) cause syntax error: +mobile/android/android-components/docs/assets/main.scss + +# 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 +toolkit/components/pdfjs/content/web/viewer.css +toolkit/components/pdfjs/content/web/viewer-geckoview.css +build/pgo/blueprint/**/*.css + +# Ignore web-platform tests as they are not necessarily under our control. +testing/web-platform/tests/ + ############################################################################## # The list below is copied from ThirdPartyPaths.txt. Prettier doesn't currently # support multiple ignore files or dynamic ignore configurations. diff --git a/.prettierignore-css b/.prettierignore-css new file mode 100644 index 000000000000..a8489a1f0050 --- /dev/null +++ b/.prettierignore-css @@ -0,0 +1,2 @@ +# When running Prettier via ESlint ignore these filetypes. +*.css diff --git a/.prettierignore-non-css b/.prettierignore-non-css new file mode 100644 index 000000000000..944d4510b719 --- /dev/null +++ b/.prettierignore-non-css @@ -0,0 +1,9 @@ +# When running Prettier via Stylelint ignore these filetypes. +*.js +*.json +*.jsx +*.mjs +*.sjs +*.xhtml +*.html +*.ts diff --git a/.prettierrc.js b/.prettierrc.js index 44b8e90381a1..a055d6b18c50 100644 --- a/.prettierrc.js +++ b/.prettierrc.js @@ -10,4 +10,14 @@ module.exports = { printWidth: 80, tabWidth: 2, trailingComma: "es5", + overrides: [ + { + files: "*.css", + options: { + parser: "css", + // Using a larger printWidth to avoid wrapping selectors. + printWidth: 160, + }, + }, + ], }; diff --git a/.stylelintignore b/.stylelintignore index 033f81a57104..beded28df77b 100644 --- a/.stylelintignore +++ b/.stylelintignore @@ -97,6 +97,7 @@ python/mozbuild/mozbuild/test/backend/data/build/foo.css toolkit/components/pdfjs/content/web/debugger.css toolkit/components/pdfjs/content/web/viewer.css toolkit/components/pdfjs/content/web/viewer-geckoview.css +build/pgo/blueprint/**/*.css # Ignore web-platform tests as they are not necessarily under our control. testing/web-platform/tests/ diff --git a/python/mozbuild/mozbuild/backend/mach_commands.py b/python/mozbuild/mozbuild/backend/mach_commands.py index ce304868a9a9..aab852030581 100644 --- a/python/mozbuild/mozbuild/backend/mach_commands.py +++ b/python/mozbuild/mozbuild/backend/mach_commands.py @@ -158,6 +158,7 @@ def setup_vscode(command_context, interactive): "json", "jsonc", "html", + "css", ] for lang in prettier_languages: new_settings[f"[{lang}]"] = { diff --git a/taskcluster/kinds/source-test/mozlint.yml b/taskcluster/kinds/source-test/mozlint.yml index 2decff00af9e..e107bf275d17 100644 --- a/taskcluster/kinds/source-test/mozlint.yml +++ b/taskcluster/kinds/source-test/mozlint.yml @@ -533,6 +533,7 @@ file-perm: - '**/*.c' - '**/*.cc' - '**/*.cpp' + - '**/*.css' - '**/*.flac' - '**/*.h' - '**/*.html' diff --git a/tools/lint/eslint/__init__.py b/tools/lint/eslint/__init__.py index d33c73b0cc98..31cc791f8398 100644 --- a/tools/lint/eslint/__init__.py +++ b/tools/lint/eslint/__init__.py @@ -14,7 +14,7 @@ sys.path.append(os.path.join(os.path.dirname(__file__), "eslint")) from mozbuild.nodeutil import find_node_executable from mozlint import result -from eslint import setup_helper +from eslint import prettier_utils, setup_helper ESLINT_ERROR_MESSAGE = """ An error occurred running eslint. Please check the following error messages: @@ -32,16 +32,6 @@ mach eslint --setup and try again. """.strip() -PRETTIER_ERROR_MESSAGE = """ -An error occurred running prettier. Please check the following error messages: - -{} -""".strip() - -PRETTIER_FORMATTING_MESSAGE = ( - "This file needs formatting with Prettier (use 'mach lint --fix ')." -) - def setup(root, **lintargs): setup_helper.set_project_root(root) @@ -123,6 +113,8 @@ def lint(paths, config, binary=None, fix=None, rules=[], setup=None, **lintargs) ), "--list-different", "--no-error-on-unmatched-pattern", + "--ignore-path=.prettierignore", + "--ignore-path=.prettierignore-css", ] + extra_args # Prettier does not support exclude arguments. @@ -134,7 +126,7 @@ def lint(paths, config, binary=None, fix=None, rules=[], setup=None, **lintargs) if fix: cmd_args.append("--write") - prettier_result = run_prettier(cmd_args, config, fix) + prettier_result = prettier_utils.run_prettier(cmd_args, config, fix) if prettier_result == 1: return prettier_result @@ -203,99 +195,3 @@ def run(cmd_args, config): results.append(result.from_config(config, **err)) return {"results": results, "fixed": fixed} - - -def run_prettier(cmd_args, config, fix): - shell = False - if is_windows(): - # The eslint 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} - - results = [] - - if errors: - errors = errors.decode(encoding, "replace").strip().split("\n") - errors = [ - error - for error in errors - # Unknown options are not an issue for Prettier, this avoids - # errors during tests. - if not ("Ignored unknown option" in error) - ] - if len(errors): - results.append( - result.from_config( - config, - **{ - "name": "eslint", - "path": os.path.abspath("."), - "message": PRETTIER_ERROR_MESSAGE.format("\n".join(errors)), - "level": "error", - "rule": "prettier", - "lineno": 0, - "column": 0, - } - ) - ) - - if not output: - # If we have errors, but no output, we assume something really bad happened. - if errors and len(errors): - return {"results": results, "fixed": 0} - - return {"results": [], "fixed": 0} # no output means success - - output = output.decode(encoding, "replace").splitlines() - - fixed = 0 - - if fix: - # When Prettier is running in fix mode, it outputs the list of files - # that have been fixed, so sum them up here. - # If it can't fix files, it will throw an error, which will be handled - # above. - fixed = len(output) - else: - # When in "check" mode, Prettier will output the list of files that - # need changing, so we'll wrap them in our result structure here. - for file in output: - if not file: - continue - - file = os.path.abspath(file) - results.append( - result.from_config( - config, - **{ - "name": "eslint", - "path": file, - "message": PRETTIER_FORMATTING_MESSAGE, - "level": "error", - "rule": "prettier", - "lineno": 0, - "column": 0, - } - ) - ) - - return {"results": results, "fixed": fixed} - - -def is_windows(): - return ( - os.environ.get("MSYSTEM") in ("MINGW32", "MINGW64") - or "MOZILLABUILD" in os.environ - ) diff --git a/tools/lint/eslint/prettier_utils.py b/tools/lint/eslint/prettier_utils.py new file mode 100644 index 000000000000..300729fdcdc9 --- /dev/null +++ b/tools/lint/eslint/prettier_utils.py @@ -0,0 +1,117 @@ +# -*- 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 os +import signal +import subprocess + +from mozlint import result + +PRETTIER_ERROR_MESSAGE = """ +An error occurred running prettier. Please check the following error messages: + +{} +""".strip() + +PRETTIER_FORMATTING_MESSAGE = ( + "This file needs formatting with Prettier (use 'mach lint --fix ')." +) + + +def run_prettier(cmd_args, config, fix): + shell = False + if is_windows(): + # The eslint 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} + + results = [] + + if errors: + errors = errors.decode(encoding, "replace").strip().split("\n") + errors = [ + error + for error in errors + # Unknown options are not an issue for Prettier, this avoids + # errors during tests. + if not ("Ignored unknown option" in error) + ] + if len(errors): + results.append( + result.from_config( + config, + **{ + "name": "eslint", + "path": os.path.abspath("."), + "message": PRETTIER_ERROR_MESSAGE.format("\n".join(errors)), + "level": "error", + "rule": "prettier", + "lineno": 0, + "column": 0, + } + ) + ) + + if not output: + # If we have errors, but no output, we assume something really bad happened. + if errors and len(errors): + return {"results": results, "fixed": 0} + + return {"results": [], "fixed": 0} # no output means success + + output = output.decode(encoding, "replace").splitlines() + + fixed = 0 + + if fix: + # When Prettier is running in fix mode, it outputs the list of files + # that have been fixed, so sum them up here. + # If it can't fix files, it will throw an error, which will be handled + # above. + fixed = len(output) + else: + # When in "check" mode, Prettier will output the list of files that + # need changing, so we'll wrap them in our result structure here. + for file in output: + if not file: + continue + + file = os.path.abspath(file) + results.append( + result.from_config( + config, + **{ + "name": "eslint", + "path": file, + "message": PRETTIER_FORMATTING_MESSAGE, + "level": "error", + "rule": "prettier", + "lineno": 0, + "column": 0, + } + ) + ) + + return {"results": results, "fixed": fixed} + + +def is_windows(): + return ( + os.environ.get("MSYSTEM") in ("MINGW32", "MINGW64") + or "MOZILLABUILD" in os.environ + ) diff --git a/tools/lint/file-perm.yml b/tools/lint/file-perm.yml index e4e1d9aabf74..a44b5ba141bd 100644 --- a/tools/lint/file-perm.yml +++ b/tools/lint/file-perm.yml @@ -8,6 +8,7 @@ file-perm: - .c - .cc - .cpp + - .css - .flac - .h - .html diff --git a/tools/lint/mach_commands.py b/tools/lint/mach_commands.py index b4bbdfe83d39..d2a0076e5491 100644 --- a/tools/lint/mach_commands.py +++ b/tools/lint/mach_commands.py @@ -23,7 +23,7 @@ if os.path.exists(thunderbird_excludes): GLOBAL_EXCLUDES = ["**/node_modules", "tools/lint/test/files", ".hg", ".git"] -VALID_FORMATTERS = {"black", "clang-format", "eslint", "rustfmt"} +VALID_FORMATTERS = {"black", "clang-format", "eslint", "rustfmt", "stylelint"} VALID_ANDROID_FORMATTERS = {"android-format"} # Code-review bot must index issues from the whole codebase when pushing diff --git a/tools/lint/stylelint/__init__.py b/tools/lint/stylelint/__init__.py index 7c1b9271b963..08c966ae5de6 100644 --- a/tools/lint/stylelint/__init__.py +++ b/tools/lint/stylelint/__init__.py @@ -12,7 +12,7 @@ import subprocess import sys sys.path.append(os.path.join(os.path.dirname(__file__), "eslint")) -from eslint import setup_helper +from eslint import prettier_utils, setup_helper from mozbuild.nodeutil import find_node_executable from mozlint import result @@ -59,7 +59,7 @@ def lint(paths, config, binary=None, fix=None, rules=[], setup=None, **lintargs) modified_paths += [path] else: joined_path = os.path.join(path, "**", exts) - if is_windows(): + if prettier_utils.is_windows(): joined_path = joined_path.replace("\\", "/") modified_paths.append(joined_path) @@ -76,48 +76,78 @@ def lint(paths, config, binary=None, fix=None, rules=[], setup=None, **lintargs) 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"])] + result = {"results": [], "fixed": 0} + + if not lintargs.get("formatonly", False): + exclude_args = [] + for path in config.get("exclude", []): + exclude_args.extend( + ["--ignore-pattern", os.path.relpath(path, lintargs["root"])] + ) + + # Default to $topsrcdir/.stylelintrc.js, but allow override in stylelint.yml + stylelint_rc = config.get("stylelint-rc", ".stylelintrc.js") + + # First run Stylelint + cmd_args = ( + [ + binary, + os.path.join( + module_path, "node_modules", "stylelint", "bin", "stylelint.mjs" + ), + "--formatter", + "json", + "--allow-empty-input", + "--config", + os.path.join(lintargs["root"], stylelint_rc), + ] + + extra_args + + exclude_args + + modified_paths ) - # Default to $topsrcdir/.stylelintrc.js, but allow override in stylelint.yml - stylelint_rc = config.get("stylelint-rc", ".stylelintrc.js") + if fix: + cmd_args.append("--fix") - # First run Stylelint + log.debug("Stylelint command: {}".format(" ".join(cmd_args))) + + result = run(cmd_args, config, fix) + if result == 1: + return result + + # Then run Prettier cmd_args = ( [ binary, os.path.join( - module_path, "node_modules", "stylelint", "bin", "stylelint.mjs" + module_path, "node_modules", "prettier", "bin", "prettier.cjs" ), - "--formatter", - "json", - "--allow-empty-input", - "--config", - os.path.join(lintargs["root"], stylelint_rc), + "--list-different", + "--no-error-on-unmatched-pattern", + "--ignore-path=.prettierignore", + "--ignore-path=.prettierignore-non-css", ] - + extra_args - + exclude_args - + modified_paths + # Prettier does not support exclude arguments. + # + exclude_args + + paths ) + log.debug("Prettier command: {}".format(" ".join(cmd_args))) if fix: - cmd_args.append("--fix") + cmd_args.append("--write") - log.debug("Stylelint command: {}".format(" ".join(cmd_args))) - - result = run(cmd_args, config, fix) - if result == 1: - return result + prettier_result = prettier_utils.run_prettier(cmd_args, config, fix) + if prettier_result == 1: + return prettier_result + result["results"].extend(prettier_result["results"]) + result["fixed"] = result["fixed"] + prettier_result["fixed"] return result def run(cmd_args, config, fix): shell = False - if is_windows(): + if prettier_utils.is_windows(): # The stylelint binary needs to be run from a shell with msys shell = True encoding = "utf-8" @@ -185,10 +215,3 @@ def run(cmd_args, config, fix): results.append(result.from_config(config, **err)) return {"results": results, "fixed": fixed} - - -def is_windows(): - return ( - os.environ.get("MSYSTEM") in ("MINGW32", "MINGW64") - or "MOZILLABUILD" in os.environ - )