diff options
Diffstat (limited to 'docs/code-quality/lint')
92 files changed, 3540 insertions, 0 deletions
diff --git a/docs/code-quality/lint/create.rst b/docs/code-quality/lint/create.rst new file mode 100644 index 0000000000..c4d8b15480 --- /dev/null +++ b/docs/code-quality/lint/create.rst @@ -0,0 +1,373 @@ +Adding a New Linter to the Tree +=============================== + +Linter Requirements +------------------- + +For a linter to be integrated into the mozilla-central tree, it needs to have: + +* Any required dependencies should be installed as part of ``./mach bootstrap`` +* A ``./mach lint`` interface +* Running ``./mach lint`` command must pass (note, linters can be disabled for individual directories) +* Taskcluster/Treeherder integration +* In tree documentation (under ``docs/code-quality/lint``) to give a basic summary, links and any other useful information +* Unit tests (under ``tools/lint/test``) to make sure that the linter works as expected and we don't regress. + +The review group in Phabricator is ``#linter-reviewers``. + +Linter Basics +------------- + +A linter is a yaml file with a ``.yml`` extension. Depending on how the type of linter, there may +be python code alongside the definition, pointed to by the 'payload' attribute. + +Here's a trivial example: + +no-eval.yml + +.. code-block:: yaml + + EvalLinter: + description: Ensures the string eval doesn't show up. + extensions: ['js'] + type: string + payload: eval + +Now ``no-eval.yml`` gets passed into :func:`LintRoller.read`. + + +Linter Types +------------ + +There are four types of linters, though more may be added in the future. + +1. string - fails if substring is found +2. regex - fails if regex matches +3. external - fails if a python function returns a non-empty result list +4. structured_log - fails if a mozlog logger emits any lint_error or lint_warning log messages + +As seen from the example above, string and regex linters are very easy to create, but they +should be avoided if possible. It is much better to use a context aware linter for the language you +are trying to lint. For example, use eslint to lint JavaScript files, use ruff to lint Python +files, etc. + +Which brings us to the third and most interesting type of linter, +external. External linters call an arbitrary python function which is +responsible for not only running the linter, but ensuring the results +are structured properly. For example, an external type could shell out +to a 3rd party linter, collect the output and format it into a list of +:class:`Issue` objects. The signature for this python +function is ``lint(files, config, **kwargs)``, where ``files`` is a list of +files to lint and ``config`` is the linter definition defined in the ``.yml`` +file. + +Structured log linters are much like external linters, but suitable +for cases where the linter code is using mozlog and emits +``lint_error`` or ``lint_warning`` logging messages when the lint +fails. This is recommended for writing novel gecko-specific lints. In +this case the signature for lint functions is ``lint(files, config, logger, +**kwargs)``. + + +Linter Definition +----------------- + +Each ``.yml`` file must have at least one linter defined in it. Here are the supported keys: + +* description - A brief description of the linter's purpose (required) +* type - One of 'string', 'regex' or 'external' (required) +* payload - The actual linting logic, depends on the type (required) +* include - A list of file paths that will be considered (optional) +* exclude - A list of file paths or glob patterns that must not be matched (optional) +* extensions - A list of file extensions to be considered (optional) +* setup - A function that sets up external dependencies (optional) +* support-files - A list of glob patterns matching configuration files (optional) +* find-dotfiles - If set to ``true``, run on dot files (.*) (optional) +* ignore-case - If set to ``true`` and ``type`` is regex, ignore the case (optional) + +In addition to the above, some ``.yml`` files correspond to a single lint rule. For these, the +following additional keys may be specified: + +* message - A string to print on infraction (optional) +* hint - A string with a clue on how to fix the infraction (optional) +* rule - An id string for the lint rule (optional) +* level - The severity of the infraction, either 'error' or 'warning' (optional) + +For structured_log lints the following additional keys apply: + +* logger - A StructuredLog object to use for logging. If not supplied + one will be created (optional) + + +Example +------- + +Here is an example of an external linter that shells out to the Python ruff linter, +let's call the file ``ruff_lint.py`` (`in-tree version <https://searchfox.org/mozilla-central/source/tools/lint/python/ruff.py>`__): + +.. code-block:: python + + import json + import os + import subprocess + from collections import defaultdict + from shutil import which + + from mozlint import result + + + RUFF_NOT_FOUND = """ + Could not find ruff! Install ruff and try again. + """.strip() + + + def lint(paths, config, **lintargs): + binary = which('ruff') + if not binary: + print(RUFF_NOT_FOUND) + return 1 + + + cmd = ["ruff", "check", "--force-exclude", "--format=json"] + paths + output = subprocess.run(cmd, stdout=subprocess.PIPE, env=os.environ).output + + # all passed + if not output: + return [] + + try: + issues = json.loads(output) + except json.JSONDecodeError: + log.error(f"Could not parse output: {output}") + + results = [] + for issue in issues: + # convert ruff's format to mozlint's format + res = { + "path": issue["filename"], + "lineno": issue["location"]["row"], + "column": issue["location"]["column"], + "lineoffset": issue["end_location"]["row"] - issue["location"]["row"], + "message": issue["message"], + "rule": issue["code"], + "level": "error", + } + + if issue["fix"]: + res["hint"] = issue["fix"]["message"] + + results.append(result.from_config(config, **res)) + + return {"results": results, "fixed": fixed} + +Now here is the linter definition that would call it: + +.. code-block:: yaml + + ruff: + description: Python Linter + include: ["."] + extensions: ["py"] + support-files: + - "**/.ruff.toml" + - "**/ruff.toml" + - "**/pyproject.toml" + type: external + payload: py.ruff:lint + +Notice the payload has two parts, delimited by ':'. The first is the module +path, which ``mozlint`` will attempt to import. The second is the object path +within that module (e.g, the name of a function to call). It is up to consumers +of ``mozlint`` to ensure the module is in ``sys.path``. Structured log linters +use the same import mechanism. + +The ``support-files`` key is used to list configuration files or files related +to the running of the linter itself. If using ``--outgoing`` or ``--workdir`` +and one of these files was modified, the entire tree will be linted instead of +just the modified files. + +Result definition +----------------- + +When generating the list of results, the following values are available. + +.. csv-table:: + :header: "Name", "Description", "Optional" + :widths: 20, 40, 10 + + "linter", "Name of the linter that flagged this error", "" + "path", "Path to the file containing the error", "" + "message", "Text describing the error", "" + "lineno", "Line number that contains the error", "" + "column", "Column containing the error", "" + "level", "Severity of the error, either 'warning' or 'error' (default 'error')", "Yes" + "hint", "Suggestion for fixing the error", "Yes" + "source", "Source code context of the error", "Yes" + "rule", "Name of the rule that was violated", "Yes" + "lineoffset", "Denotes an error spans multiple lines, of the form (<lineno offset>, <num lines>)", "Yes" + "diff", "A diff describing the changes that need to be made to the code", "Yes" + + +Automated testing +----------------- + +Every new checker must have associated tests. If your linter is ``mylinter`` then the +test file should be named ``tools/lint/test/test_mylinter.py`` and any example files +named like ``tools/lint/test/files/mylinter/my-example-file``. Be sure that your test +has been added as a section ``["test_mylinter.py"]`` in the manifest ``tools/lint/test/python.toml``. + +They should be pretty easy to write as most of the work is managed by the Mozlint +framework. The key declaration is the ``LINTER`` variable which must match +the linker declaration. + +As an example, the `ruff test <https://searchfox.org/mozilla-central/source/tools/lint/test/test_ruff.py>`_ looks like the following snippet: + +.. code-block:: python + + import mozunit + LINTER = 'ruff' + + def test_lint_ruff(lint, paths): + results = lint(paths('bad.py')) + assert len(results) == 2 + assert results[0].rule == 'F401' + assert results[1].rule == 'E501' + assert results[1].lineno == 5 + + if __name__ == '__main__': + mozunit.main() + +As always with tests, please make sure that enough positive and negative cases +are covered. + +To run the tests: + +.. code-block:: shell + + $ ./mach python-test --subsuite mozlint + +To run a specific test: + +.. code-block:: shell + + ./mach python-test --subsuite mozlint tools/lint/test/test_black.py + +More tests can be `found in-tree <https://searchfox.org/mozilla-central/source/tools/lint/test>`_. + +Tracking fixed issues +--------------------- + +All the linters that provide ``fix support`` returns a dictionary instead of a list. + +``{"results":result,"fixed":fixed}`` + +* results - All the linting errors it was not able to fix +* fixed - Count of fixed errors (for ``fix=False`` this is 0) + +Some linters (example: `codespell <https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/tools/lint/spell/__init__.py#145-163>`_) might require two passes to count the number of fixed issues. +Others might just need `some tuning <https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/tools/lint/file-whitespace/__init__.py#28,60,85,112>`_. + +For adding tests to check your fixed count, add a global variable ``fixed = 0`` +and write a function to add your test as mentioned under ``Automated testing`` section. + + +Here's an example + +.. code-block:: python + + fixed = 0 + + + def test_lint_codespell_fix(lint, create_temp_file): + # Typo has been fixed in the contents to avoid triggering warning + # 'informations' ----> 'information' + contents = """This is a file with some typos and information. + But also testing false positive like optin (because this isn't always option) + or stuff related to our coding style like: + aparent (aParent). + but detects mistakes like mozilla + """.lstrip() + + path = create_temp_file(contents, "ignore.rst") + lint([path], fix=True) + + assert fixed == 2 + + +Bootstrapping Dependencies +-------------------------- + +Many linters, especially 3rd party ones, will require a set of dependencies. It +could be as simple as installing a binary from a package manager, or as +complicated as pulling a whole graph of tools, plugins and their dependencies. + +Either way, to reduce the burden on users, linters should strive to provide +automated bootstrapping of all their dependencies. To help with this, +``mozlint`` allows linters to define a ``setup`` config, which has the same +path object format as an external payload. For example (`in-tree version <https://searchfox.org/mozilla-central/source/tools/lint/ruff.yml>`__): + +.. code-block:: yaml + + ruff: + description: Python linter + include: ['.'] + extensions: ['py'] + type: external + payload: py.ruff:lint + setup: py.ruff:setup + +The setup function takes a single argument, the root of the repository being +linted. In the case of ``ruff``, it might look like: + +.. code-block:: python + + import subprocess + from shutil import which + + def setup(root, **lintargs): + # This is a simple example. Please look at the actual source for better examples. + if not which("ruff"): + subprocess.call(["pip", "install", "ruff"]) + +The setup function will be called implicitly before running the linter. This +means it should return fast and not produce any output if there is no setup to +be performed. + +The setup functions can also be called explicitly by running ``mach lint +--setup``. This will only perform setup and not perform any linting. It is +mainly useful for other tools like ``mach bootstrap`` to call into. + + +Adding the linter to the CI +--------------------------- + +First, the job will have to be declared in Taskcluster. + +This should be done in the `mozlint Taskcluster configuration <https://searchfox.org/mozilla-central/source/taskcluster/ci/source-test/mozlint.yml>`_. +You will need to define a symbol, how it is executed and on what kind of change. + +For example, for ruff, the configuration is the following: + +.. code-block:: yaml + + py-ruff: + description: run ruff over the gecko codebase + treeherder: + symbol: py(ruff) + run: + mach: lint -l ruff -f treeherder -f json:/builds/worker/mozlint.json . + when: + files-changed: + - '**/*.py' + - '**/.ruff.toml' + +If the linter requires an external program, you will have to install it in the `setup script <https://searchfox.org/mozilla-central/source/taskcluster/docker/lint/system-setup.sh>`_ +and maybe install the necessary files in the `Docker configuration <https://searchfox.org/mozilla-central/source/taskcluster/docker/lint/Dockerfile>`_. + +.. note:: + + If the defect found by the linter is minor, make sure that it is logged as + a warning by setting `{"level": "warning"}` in the + :class:`~mozlint.result.Issue`. This means the defect will not cause a + backout if landed, but will still be surfaced by reviewbot at review time, + or when using `-W/--warnings` locally. diff --git a/docs/code-quality/lint/index.rst b/docs/code-quality/lint/index.rst new file mode 100644 index 0000000000..2b2cfdefbd --- /dev/null +++ b/docs/code-quality/lint/index.rst @@ -0,0 +1,33 @@ +Linting +======= + +Linters are used in mozilla-central to help enforce coding style and avoid bad practices. +They cover a wide variety of languages and checks. + +Getting Help +------------ + +If you need help or have questions, please don’t hesitate to contact us via Matrix +in the "Lint and Formatting" room +(`#lint:mozilla.org <https://chat.mozilla.org/#/room/#lint:mozilla.org>`_). + + +.. toctree:: + :caption: Getting Started + :maxdepth: 2 + + usage + +.. toctree:: + :caption: Linter Implementations + :maxdepth: 1 + :glob: + + linters/* + +.. toctree:: + :caption: Linter Specifics + :maxdepth: 1 + + mozlint + create diff --git a/docs/code-quality/lint/linters/android-format.rst b/docs/code-quality/lint/linters/android-format.rst new file mode 100644 index 0000000000..ed23ac64de --- /dev/null +++ b/docs/code-quality/lint/linters/android-format.rst @@ -0,0 +1,31 @@ +Spotless +======== + +`Spotless <https://github.com/diffplug/spotless>`__ is a pluggable formatter +for Gradle and Android. + +In our current configuration, Spotless includes the +`Google Java Format plug-in <https://github.com/google/google-java-format>`__ +which formats all our Java code using the Google Java coding style guidelines, +and `ktlint <https://ktlint.github.io/>`__ which formats all +our Kotlin code using the official Kotlin coding convention and Android Kotlin +Style Guide. + + +Run Locally +----------- + +The mozlint integration of spotless can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter android-format + +Alternatively, omit the ``--linter android-format`` and run all configured linters, which will include +spotless. + + +Autofix +------- + +The spotless linter provides a ``--fix`` option. diff --git a/docs/code-quality/lint/linters/black.rst b/docs/code-quality/lint/linters/black.rst new file mode 100644 index 0000000000..60a06ce95b --- /dev/null +++ b/docs/code-quality/lint/linters/black.rst @@ -0,0 +1,36 @@ +Black +===== + +`Black <https://black.readthedocs.io/en/stable/>`__ is a opinionated python code formatter. + + +Run Locally +----------- + +The mozlint integration of black can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter black <file paths> + +Alternatively, omit the ``--linter black`` and run all configured linters, which will include +black. + + +Configuration +------------- + +To enable black on new directory, add the path to the include +section in the :searchfox:`black.yml <tools/lint/black.yml>` file. + +Autofix +------- + +The black linter provides a ``--fix`` option. + + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/black.yml>` +* :searchfox:`Source <tools/lint/python/black.py>` diff --git a/docs/code-quality/lint/linters/clang-format.rst b/docs/code-quality/lint/linters/clang-format.rst new file mode 100644 index 0000000000..a528af4358 --- /dev/null +++ b/docs/code-quality/lint/linters/clang-format.rst @@ -0,0 +1,35 @@ +clang-format +============ + +`clang-format <https://clang.llvm.org/docs/ClangFormat.html>`__ is a tool to reformat C/C++ to the right coding style. + +Run Locally +----------- + +The mozlint integration of clang-format can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter clang-format <file paths> + + +Configuration +------------- + +To enable clang-format on new directory, add the path to the include +section in the :searchfox:`clang-format.yml <tools/lint/clang-format.yml>` file. + +While excludes: will work, this linter will read the ignore list from :searchfox:`.clang-format-ignore file <.clang-format-ignore>` +at the root directory. This because it is also used by the ./mach clang-format -p command. + +Autofix +------- + +clang-format can reformat the code with the option `--fix` (based on the upstream option `-i`). +To highlight the results, we are using the ``--dry-run`` option (from clang-format 10). + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/clang-format.yml>` +* :searchfox:`Source <tools/lint/clang-format/__init__.py>` diff --git a/docs/code-quality/lint/linters/clippy.rst b/docs/code-quality/lint/linters/clippy.rst new file mode 100644 index 0000000000..40db532b88 --- /dev/null +++ b/docs/code-quality/lint/linters/clippy.rst @@ -0,0 +1,36 @@ +clippy +====== + +`clippy`_ is the tool for Rust static analysis. + +Run Locally +----------- + +The mozlint integration of clippy can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter clippy <file paths> + +.. note:: + + clippy expects a path or a .rs file. It doesn't accept Cargo.toml + as it would break the mozlint workflow. + +Configuration +------------- + +To enable clippy on new directory, add the path to the include +section in the `clippy.yml <https://searchfox.org/mozilla-central/source/tools/lint/clippy.yml>`_ file. + +Autofix +------- + +This linter provides a ``--fix`` option. +Please note that this option does not fix all detected issues. + +Sources +------- + +* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/clippy.yml>`_ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/clippy/__init__.py>`_ diff --git a/docs/code-quality/lint/linters/codespell.rst b/docs/code-quality/lint/linters/codespell.rst new file mode 100644 index 0000000000..9299a81b6e --- /dev/null +++ b/docs/code-quality/lint/linters/codespell.rst @@ -0,0 +1,36 @@ +Codespell +========= + +`codespell <https://github.com/codespell-project/codespell/>`__ is a popular tool to look for typical typos in the source code. + +It is enabled mostly for the documentation and English locale files. + +Run Locally +----------- + +The mozlint integration of codespell can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter codespell <file paths> + + +Configuration +------------- + +To enable codespell on new directory, add the path to the include +section in the :searchfox:`codespell.yml <tools/lint/codespell.yml>` file. + +This job is configured as `tier 2 <https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Overview_of_the_Job_Visibility_Tiers>`_. + +Autofix +------- + +Codespell provides a ``--fix`` option. It is based on the ``-w`` option provided by upstream. + + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/codespell.yml>` +* :searchfox:`Source <tools/lint/spell/__init__.py>` diff --git a/docs/code-quality/lint/linters/condprof-addons.rst b/docs/code-quality/lint/linters/condprof-addons.rst new file mode 100644 index 0000000000..c302eef5be --- /dev/null +++ b/docs/code-quality/lint/linters/condprof-addons.rst @@ -0,0 +1,85 @@ +CondProf Addons +=============== + +CondProf Addons is a linter for condprof customization JSON files (see :searchfox:`testing/condprofile/condprof/customization`), +it reports linting errors if: + +- any of the addons required by the customization files (e.g. see :searchfox:`testing/condprofile/condprof/customization/webext.json`) + is not found in the tar file fetched through the `firefox-addons` fetch task (see :searchfox:`taskcluster/ci/fetch/browsertime.yml`) +- or the expected `firefox-addons` fetch task has not been found + +Run Locally +----------- + +The mozlint integration of condprof-addons can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter condprof-addons <file paths> + +Alternatively, if the ``--linter condprof-addons`` is omitted, the ``condprof-addons`` will still be selected automatically if +any of the files paths passed explicitly is detected to be part of the condprof customization directory. + +The ``condprof-addons`` will also be running automatically on ``mach lint --outgoing`` if there are customization files changes +detected in the outgoing patches. + +Run on Taskcluster +------------------ + +The condprof-addons job shows up as ``misc(condprof-addons)`` in the linting job. It should run automatically if changes are made +to condprof customization JSON files. + +Fix reported errors +------------------- + +XPI file is missing from the firefox-addons.tar archive +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This linting errors is expected to be reported if the linter detected that a confprof customization file +requires an addon but the related xpi filename is not included in the firefox-addons.tar file fetched +through the `firefox-addons` fetch task (see :searchfox:`taskcluster/ci/fetch/browsertime.yml`). + +If the patch or phabricator revision is not meant to be landed, but only used as a temporary patch +pushed on try or only applied locally (e.g. to run the tp6/tp6m webextensions perftests with a given +third party extension installed to gather some metrics and/or GeckoProfiler data), then it can be +safely ignored. + +On the contrary, if the patch or phabricator revision is meant to be landed on mozilla-central, +the linting error have to be fixed before or along landing the change, either by: + +- removing the addition to the customization file if it wasn't intended to include that addon to all runs + of the tp6/tp6m webextensions perftests + +- updating the `firefox-addons` fetch task as defined in :searchfox:`taskcluster/ci/fetch/browsertime.yml` + by creating a pull request in the github repository where the asset is stored, and ask a review from + a peer of the `#webextensions-reviewer` and `#perftests-reviewers` review groups. + +firefox-addons taskcluster config 'add-prefix' attribute should be set to 'firefox-addons/' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +If this linting error is hit, then the `firefox-addons` task defined in :searchfox:`taskcluster/ci/fetch/browsertime.yml` +is missing the `add-prefix` attribute or its value is not set to the expected 'firefox-addons/' subdir name. + +This is enforced as a linting rule because when the condprof utility is going to build a conditioned profile +for which some add-ons xpi files are expected to be sideloaded (see :searchfox:`testing/condprofile/condprof/customization.webext.json`), +to avoid re-downloading the same xpi from a remote urls every time the conditioned profile is built on the build infrastructure +(which for tp6/tp6m perftests will happen once per job being executed) condprof is going to look first if the expected xpi file +names are already available in `$MOZ_FETCHES_DIR/firefox-addons`. + +firefox-addons taskcluser fetch config section not found +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This linting error is hit if the linter does not find the expected `firefox-addons` task defined in :searchfox:`taskcluster/ci/fetch/browsertime.yml` +or it is missing the expected `fetch` attribute. + +Configuration +------------- + +ConfProf Addons does not currently provide any configuration files. + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/condprof-addons/yml>` +* :searchfox:`Source <tools/lint/condprof-addons/__init__.py>` +* :searchfox:`Test <tools/lint/test/test_condprof_addons.py>` diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla.rst new file mode 100644 index 0000000000..be6e158e86 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla.rst @@ -0,0 +1,60 @@ +===================== +Mozilla ESLint Plugin +===================== + +This is the documentation of Mozilla ESLint PLugin. + +Environments +============ + +The plugin implements the following environments: + + +.. toctree:: + :maxdepth: 2 + + eslint-plugin-mozilla/environment + +Rules +===== + +The plugin implements the following rules: + +.. toctree:: + :maxdepth: 1 + :glob: + + eslint-plugin-mozilla/rules/* + +Tests +===== + +The tests for eslint-plugin-mozilla are run via `mochajs`_ on top of node. Most +of the tests use the `ESLint Rule Unit Test framework`_. + +.. _mochajs: https://mochajs.org/ +.. _ESLint Rule Unit Test Framework: http://eslint.org/docs/developer-guide/working-with-rules#rule-unit-tests + +Running Tests +------------- + +The tests for eslint-plugin-mozilla are run via `mochajs`_ on top of node. Most +of the tests use the `ESLint Rule Unit Test framework`_. + +The rules have some self tests, these can be run via: + +.. code-block:: shell + + $ cd tools/lint/eslint/eslint-plugin-mozilla + $ npm install + $ npm run test + +Disabling tests +--------------- + +In the unlikely event of needing to disable a test, currently the only way is +by commenting-out. Please file a bug if you have to do this. Bugs should be filed +in the *Testing* product under *Lint*. + +.. _mochajs: https://mochajs.org/ +.. _ESLint Rule Unit Test Framework: http://eslint.org/docs/developer-guide/working-with-rules#rule-unit-tests diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/environment.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/environment.rst new file mode 100644 index 0000000000..928fa3bc8b --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/environment.rst @@ -0,0 +1,91 @@ +Environment +=========== + +These environments are available by specifying a comment at the top of the file, +e.g. + +.. code-block:: js + + /* eslint-env mozilla/chrome-worker */ + +There are also built-in ESLint environments available as well. Find them here: http://eslint.org/docs/user-guide/configuring#specifying-environments + +browser-window +-------------- + +Defines the environment for scripts that are in the main browser.xhtml scope. + +chrome-script +------------- + +Defines the environment for scripts loaded by +``SpecialPowers.loadChromeScript``. + +chrome-worker +------------- + +Defines the environment for chrome workers. This differs from normal workers by +the fact that `ctypes` can be accessed as well. + +frame-script +------------ + +Defines the environment for scripts loaded by ``Services.mm.loadFrameScript``. + +jsm +--- + +Defines the environment for jsm files (javascript modules). + +privileged +---------- + +Defines the environment for privileged JS files. + +process-script +-------------- + +Defines the environment for scripts loaded by +``Services.ppmm.loadProcessScript``. + +remote-page +----------- + +Defines the environment for scripts loaded with ``<script src="...">`` in +``about:`` pages. + +simpletest +---------- + +Defines the environment for scripts that use the SimpleTest mochitest harness. + +sjs +--- + +Defines the environment for sjs files. + +special-powers-sandbox +---------------------- + +Defines the environment for scripts evaluated inside ``SpecialPowers`` sandbox +with the default options. + +testharness +----------- + +Defines the environment the globals that are injected from +:searchfox:`dom/imptests/testharness.js <dom/imptests/testharness.js>`. + +It is injected automatically into (x)html files which include: + +.. code-block:: html + + <script src="/resources/testharness.js"></script> + +It may need to be included manually in JavaScript files which are loaded into +the same scope. + +xpcshell +-------- + +Defines the environment for xpcshell test files. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/avoid-Date-timing.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/avoid-Date-timing.rst new file mode 100644 index 0000000000..b01b568a28 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/avoid-Date-timing.rst @@ -0,0 +1,30 @@ +avoid-Date-timing +================= + +Rejects grabbing the current time via Date.now() or new Date() for timing +purposes when the less problematic performance.now() can be used instead. + +The performance.now() function returns milliseconds since page load. To +convert that to milliseconds since the epoch, use: + +.. code-block:: js + + performance.timing.navigationStart + performance.now() + +Often timing relative to the page load is adequate and that conversion may not +be necessary. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + Date.now() + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + new Date('2017-07-11'); + performance.now() diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/avoid-removeChild.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/avoid-removeChild.rst new file mode 100644 index 0000000000..15ece94d0d --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/avoid-removeChild.rst @@ -0,0 +1,20 @@ +avoid-removeChild +================= + +Rejects using ``element.parentNode.removeChild(element)`` when ``element.remove()`` +can be used instead. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + elt.parentNode.removeChild(elt); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + elt.remove(); + elt.parentNode.removeChild(elt2); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/balanced-listeners.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/balanced-listeners.rst new file mode 100644 index 0000000000..f53c11e7aa --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/balanced-listeners.rst @@ -0,0 +1,20 @@ +balanced-listeners +================== + +Checks that for every occurrence of 'addEventListener' or 'on' there is an +occurrence of 'removeEventListener' or 'off' with the same event name. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + elt.addEventListener('click', handler, false); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + elt.addEventListener('event', handler); + elt.removeEventListener('event', handler); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/balanced-observers.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/balanced-observers.rst new file mode 100644 index 0000000000..b169a520a3 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/balanced-observers.rst @@ -0,0 +1,20 @@ +balanced-observers +================== + +Checks that for every occurrence of ``addObserver`` there is an +occurrence of ``removeObserver`` with the same topic. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + Services.obs.addObserver(observer, 'observable'); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + Services.obs.addObserver(observer, 'observable'); + Services.obs.removeObserver(observer, 'observable'); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/consistent-if-bracing.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/consistent-if-bracing.rst new file mode 100644 index 0000000000..7bf6b796ef --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/consistent-if-bracing.rst @@ -0,0 +1,23 @@ +consistent-if-bracing +===================== + +Checks that if/elseif/else bodies are braced consistently, so either all bodies +are braced or unbraced. Doesn't enforce either of those styles though. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + if (true) {1} else 0 + if (true) 1; else {0} + if (true) {1} else if (true) 2; else {0} + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + if (true) {1} else {0} + if (false) 1; else 0 + if (true) {1} else if (true) {2} else {0} diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-browser-window-globals.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-browser-window-globals.rst new file mode 100644 index 0000000000..35c09cc8fd --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-browser-window-globals.rst @@ -0,0 +1,8 @@ +import-browser-window-globals +============================= + +For scripts included in browser-window, this will automatically inject the +browser-window global scopes into the file. + +This is a rule rather than an environment, as it allowed us to automatically +select the files to include. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-content-task-globals.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-content-task-globals.rst new file mode 100644 index 0000000000..f2550a1412 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-content-task-globals.rst @@ -0,0 +1,14 @@ +import-content-task-globals +=========================== + +For files containing ContentTask.spawn calls, this will automatically declare +the frame script variables in the global scope. ContentTask is only available +to test files, so by default the configs only specify it for the mochitest based +configurations. + +This saves setting the file as a mozilla/frame-script environment. + +Note: due to the way ESLint works, it appears it is only easy to declare these +variables on a file global scope, rather than function global. This may mean that +they are incorrectly allowed, but given they are test files, this should be +detected during testing. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-globals-from.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-globals-from.rst new file mode 100644 index 0000000000..c2956ba05a --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-globals-from.rst @@ -0,0 +1,18 @@ +import-globals-from +=================== + +Parses a file for globals defined in various unique Mozilla ways. + +When a ``/* import-globals-from <path> */`` comment is found in a file, then all +globals from the file at <path> will be imported in the current scope. This will +also operate recursively. + +This is useful for scripts that are loaded as <script> tag in a window and rely +on each other's globals. + +If <path> is a relative path, then it must be relative to the file being +checked by the rule. + +Note: ``import-globals-from`` does not support loading globals from ES modules. +These should be imported as variable definitions directly, or the file where +they are imported should be referenced. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-globals.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-globals.rst new file mode 100644 index 0000000000..c420a1ca81 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-globals.rst @@ -0,0 +1,5 @@ +import-globals +============== + +Checks ``ChromeUtils.defineLazyGetter`` etc and adds the name to the global +scope. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-headjs-globals.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-headjs-globals.rst new file mode 100644 index 0000000000..df8ef81300 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/import-headjs-globals.rst @@ -0,0 +1,26 @@ +import-headjs-globals +===================== + +Import globals from head.js and from any files that were imported by +head.js (as far as we can correctly resolve the path). + +This rule is included in the test configurations. + +The following file import patterns are supported: + +- ``Services.scriptloader.loadSubScript(path)`` +- ``loader.loadSubScript(path)`` +- ``loadSubScript(path)`` +- ``loadHelperScript(path)`` +- ``import-globals-from path`` + +If path does not exist because it is generated e.g. +``testdir + "/somefile.js"`` we do our best to resolve it. + +The following patterns are supported: + +- ``Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm");`` +- ``loader.lazyRequireGetter(this, "name2"`` +- ``loader.lazyServiceGetter(this, "name3"`` +- ``loader.lazyGetter(this, "toolboxStrings"`` +- ``ChromeUtils.defineLazyGetter(this, "clipboardHelper"`` diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/lazy-getter-object-name.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/lazy-getter-object-name.rst new file mode 100644 index 0000000000..7d58fd631b --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/lazy-getter-object-name.rst @@ -0,0 +1,25 @@ +lazy-getter-object-name +============================= + +Enforce the standard object variable name ``lazy`` for +``ChromeUtils.defineESModuleGetters`` + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + const obj = {}; + ChromeUtils.defineESModuleGetters(obj, { + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", + }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const lazy = {}; + ChromeUtils.defineESModuleGetters(lazy, { + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", + }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/mark-exported-symbols-as-used.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/mark-exported-symbols-as-used.rst new file mode 100644 index 0000000000..92e315a249 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/mark-exported-symbols-as-used.rst @@ -0,0 +1,23 @@ +mark-exported-symbols-as-used +============================= + +Marks variables listed in ``EXPORTED_SYMBOLS`` as used so that ``no-unused-vars`` +does not complain about them. + +This rule also checks that ``EXPORTED_SYMBOLS`` is not defined using ``let`` as +``let`` isn't allowed as the lexical scope may die after the script executes. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + let EXPORTED_SYMBOLS = ["foo"]; + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + var EXPORTED_SYMBOLS = ["foo"]; + const EXPORTED_SYMBOLS = ["foo"]; diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/mark-test-function-used.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/mark-test-function-used.rst new file mode 100644 index 0000000000..a518d7415b --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/mark-test-function-used.rst @@ -0,0 +1,8 @@ +mark-test-function-used +======================= + +Simply marks ``test`` (the test method) or ``run_test`` as used when in mochitests +or xpcshell tests respectively. This avoids ESLint telling us that the function +is never called. + +This rule is included in the test configurations. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-aArgs.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-aArgs.rst new file mode 100644 index 0000000000..7e398bcbbe --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-aArgs.rst @@ -0,0 +1,22 @@ +no-aArgs +======== + +Checks that function argument names don't start with lowercase 'a' followed by +a capital letter. This is to prevent the use of Hungarian notation whereby the +first letter is a prefix that indicates the type or intended use of a variable. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + function(aFoo, aBar) {} + (aFoo, aBar) => {} + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + function(foo, bar) {} + (foo, bar) => {}) diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-addtask-setup.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-addtask-setup.rst new file mode 100644 index 0000000000..f26a869371 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-addtask-setup.rst @@ -0,0 +1,27 @@ +no-addtask-setup +================ + +Reject using ``add_task(async function setup() { ... })`` in favour of +``add_setup(async function() { ... })``. + +Using semantically separate setup functions makes ``.only`` work correctly +and will allow for future improvements to setup/cleanup abstractions. + +This option can be autofixed (``--fix``). + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + add_task(async function setup() { ... }); + add_task(function setup() { ... }); + add_task(function init() { ... }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + add_setup(async function() { ... }); + add_setup(function() { ... }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-arbitrary-setTimeout.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-arbitrary-setTimeout.rst new file mode 100644 index 0000000000..a7d62e74ba --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-arbitrary-setTimeout.rst @@ -0,0 +1,23 @@ +no-arbitrary-setTimeout +======================= + +Disallows setTimeout with non-zero values in tests. Using arbitrary times for +setTimeout may cause intermittent failures in tests. A value of zero is allowed +as this is letting the event stack unwind, however also consider the use +of ``TestUtils.waitForTick``. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + function(aFoo, aBar) {} + (aFoo, aBar) => {} + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + function(foo, bar) {} + (foo, bar) => {}) diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-browser-refs-in-toolkit.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-browser-refs-in-toolkit.rst new file mode 100644 index 0000000000..9f2501ba23 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-browser-refs-in-toolkit.rst @@ -0,0 +1,24 @@ +no-browser-refs-in-toolkit +========================== + +Rejects for using any code from ``browser/`` (Desktop Firefox) inside +``toolkit``, as ``toolkit`` is for code shared with other Gecko consumers +like Firefox on Android, Thunderbird, etc. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + "chrome://browser/content/browser.xhtml" + "resource:///modules/BrowserWindowTracker.sys.mjs" + "browser/browser.ftl" + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + "chrome://global/content/aboutAbout.html" + "resource://gre/modules/AppConstants.sys.mjs" + "toolkit/global/aboutFoo.ftl" diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-compare-against-boolean-literals.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-compare-against-boolean-literals.rst new file mode 100644 index 0000000000..b7785f2fc2 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-compare-against-boolean-literals.rst @@ -0,0 +1,23 @@ +no-compare-against-boolean-literals +=================================== + +Checks that boolean expressions do not compare against literal values +of ``true`` or ``false``. This is to prevent overly verbose code such as +``if (isEnabled == true)`` when ``if (isEnabled)`` would suffice. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + if (foo == true) {} + if (foo != false) {} + if (false == foo) {} + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + if (!foo) {} + if (!!foo) {} diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-comparison-or-assignment-inside-ok.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-comparison-or-assignment-inside-ok.rst new file mode 100644 index 0000000000..33776433db --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-comparison-or-assignment-inside-ok.rst @@ -0,0 +1,69 @@ +no-comparison-or-assignment-inside-ok +===================================== + +This rule prevents two misuses of the ``ok()`` test framework function. + +Assignments +----------- + +The first is one where people accidentally put assignments into ``ok``, like so: + +.. code-block:: js + + ok(foo = bar, "Foo should be equal to bar"); + +This is similar to the builtin eslint rule `no-cond-assign`_. + +There is no autofix as the linter cannot tell if the assignment was intentional +and should be moved out of the ``ok()`` call, or if it was intended to be some +kind of binary comparison expression (and if so, with what operator). + +.. _no-cond-assign: https://eslint.org/docs/latest/rules/no-cond-assign + +Comparisons +----------- + +The second is a stylistic/legibility/debuggability preference, where the linter +encourages use of the dedicated ``Assert`` framework comparison functions. For +more details on ``Assert``, see :ref:`assert-module`. + +This rule is autofixed, and will correct incorrect examples such as the +following: + +Examples of incorrect code for this rule: +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. code-block:: js + + ok(foo == bar); + ok(foo < bar); + ok(foo !== bar); + +to something like: + +Examples of correct code for this rule: +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. code-block:: js + + Assert.equal(foo, bar); + Assert.less(foo, bar); + Assert.notStrictEqual(foo, bar); + + +Rationale +^^^^^^^^^ + +There are a few reasons the more verbose form is preferable: + +- On failure, the framework will log both values rather than just one, making + it much easier to debug failing tests without having to manually add + logging, push to try, and then potentially retrigger to reproduce intermittents + etc. +- The :ref:`assert-module` is standardized and more widely understood than the old + mocha/mochikit forms. +- It is harder to make typos like the assignment case above, and accidentally + end up with tests that assert non-sensical things. +- Subjectively, when an additional message is long enough to cause ``prettier`` + to split the statement across multiple lines, this makes it easier to + see what the two operands of the comparison are. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-cu-reportError.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-cu-reportError.rst new file mode 100644 index 0000000000..9f5a0def27 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-cu-reportError.rst @@ -0,0 +1,23 @@ +no-cu-reportError +================= + +Disallows Cu.reportError. This has been deprecated and should be replaced by +console.error. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + Cu.reportError("message"); + Cu.reportError("message", stack); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + console.error("message"); + let error = new Error("message"); + error.stack = stack; + console.error(error); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-define-cc-etc.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-define-cc-etc.rst new file mode 100644 index 0000000000..4421f4dd54 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-define-cc-etc.rst @@ -0,0 +1,23 @@ +no-define-cc-etc +================ + +Disallows old-style definitions for ``Cc``/``Ci``/``Cu``/``Cr``. These are now +defined globally for all chrome contexts. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + var Cc = Components.classes; + var Ci = Components.interfaces; + var {Ci: interfaces, Cc: classes, Cu: utils} = Components; + var Cr = Components.results; + + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const CC = Components.Constructor; diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-redeclare-with-import-autofix.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-redeclare-with-import-autofix.rst new file mode 100644 index 0000000000..d7d4edab50 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-redeclare-with-import-autofix.rst @@ -0,0 +1,21 @@ +no-redeclare-with-import-autofix +================================ + +This is the +`builtin eslint rule no-redeclare <https://eslint.org/docs/latest/rules/no-redeclare>`_, +but with an additional fixer that can automatically remove superfluous +(duplicate) imports. + +For redeclarations that are not imports, there is no automatic fix, as +the author will likely have to rename variables so that they do not +redeclare existing variables or conflict with the name of a builtin +property or global variable. + +Typical duplicate imports happen when a `head.js` file, imports +a module and a test then subsequently also attempts to import the same +module. + +In browser mochitests, an additional typical scenario is importing +modules that are already imported in the main browser window. Because +these tests run in a scope that inherits from the main browser window +one, there is no need to re-import such modules. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-throw-cr-literal.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-throw-cr-literal.rst new file mode 100644 index 0000000000..0f9222de30 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-throw-cr-literal.rst @@ -0,0 +1,38 @@ +no-throw-cr-literal +=================== + +This is similar to the ESLint built-in rule no-throw-literal. It disallows +throwing Components.results code directly. + +Throwing bare literals is inferior to throwing Exception objects, which provide +stack information. Cr.ERRORs should be be passed as the second argument to +``Components.Exception()`` to create an Exception object with stack info, and +the correct result property corresponding to the NS_ERROR that other code +expects. +Using a regular ``new Error()`` to wrap just turns it into a string and doesn't +set the result property, so the errors can't be recognised. + +This option can be autofixed (``--fix``). + +.. code-block:: js + + performance.timing.navigationStart + performance.now() + +Often timing relative to the page load is adequate and that conversion may not +be necessary. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + throw Cr.NS_ERROR_UNEXPECTED; + throw Components.results.NS_ERROR_ABORT; + throw new Error(Cr.NS_ERROR_NO_INTERFACE); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + throw Components.Exception("Not implemented", Cr.NS_ERROR_NOT_IMPLEMENTED); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-parameters.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-parameters.rst new file mode 100644 index 0000000000..485caf6522 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-parameters.rst @@ -0,0 +1,26 @@ +no-useless-parameters +===================== + +Reject common XPCOM methods called with useless optional parameters (eg. +``Services.io.newURI(url, null, null)``, or non-existent parameters (eg. +``Services.obs.removeObserver(name, observer, false)``). + +This option can be autofixed (``--fix``). + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + elt.addEventListener('click', handler, false); + Services.io.newURI('http://example.com', null, null); + Services.obs.notifyObservers(obj, 'topic', null); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + elt.addEventListener('click', handler); + Services.io.newURI('http://example.com'); + Services.obs.notifyObservers(obj, 'topic'); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-removeEventListener.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-removeEventListener.rst new file mode 100644 index 0000000000..ce314ab58d --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-removeEventListener.rst @@ -0,0 +1,20 @@ +no-useless-removeEventListener +============================== + +Reject calls to removeEventListener where ``{once: true}`` could be used instead. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + elt.addEventListener('click', function listener() { + elt.removeEventListener('click', listener); + }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + elt.addEventListener('click', handler, {once: true}); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-run-test.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-run-test.rst new file mode 100644 index 0000000000..a079405696 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/no-useless-run-test.rst @@ -0,0 +1,6 @@ +no-useless-run-test +=================== + +Designed for xpcshell-tests. Rejects definitions of ``run_test()`` where the +function only contains a single call to ``run_next_test()``. xpcshell's head.js +already defines a utility function so there is no need for duplication. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/prefer-boolean-length-check.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/prefer-boolean-length-check.rst new file mode 100644 index 0000000000..cd6ee4e544 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/prefer-boolean-length-check.rst @@ -0,0 +1,24 @@ +prefer-boolean-length-check +=========================== + +Prefers using a boolean length check rather than comparing against zero. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + if (foo.length == 0) {} + if (foo.length > 0) {} + if (foo && foo.length == 0) {} + function bar() { return foo.length > 0 } + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + if (foo.length && foo.length) {} + if (!foo.length) {} + var a = foo.length > 0 + function bar() { return !!foo.length } diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/prefer-formatValues.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/prefer-formatValues.rst new file mode 100644 index 0000000000..88eedee792 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/prefer-formatValues.rst @@ -0,0 +1,23 @@ +prefer-formatValues +=================== + +Rejects multiple calls to document.l10n.formatValue in the same code block, to +reduce localization overheads. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + { + document.l10n.formatValue('foobar'); + document.l10n.formatValue('foobaz'); + } + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + document.l10n.formatValue('foobar'); + document.l10n.formatValues(['foobar', 'foobaz']); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-addtask-only.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-addtask-only.rst new file mode 100644 index 0000000000..e540b24416 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-addtask-only.rst @@ -0,0 +1,6 @@ +reject-addtask-only +=================== + +Designed for JavaScript tests using the add_task pattern. Rejects chaining +.only() to an add_task() call, which is useful for local testing to run a +single task in isolation but is easy to land into the tree by accident. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-chromeutils-import-params.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-chromeutils-import-params.rst new file mode 100644 index 0000000000..4710878f8d --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-chromeutils-import-params.rst @@ -0,0 +1,22 @@ +reject-chromeutils-import-params +================================ + +``ChromeUtils.import`` used to be able to be called with two arguments, however +the second argument is no longer supported. Exports from modules should now be +explicit, and the imported symbols being accessed from the returned object. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + ChromeUtils.import("resource://gre/modules/AppConstants.jsm", this); + ChromeUtils.import("resource://gre/modules/AppConstants.jsm", null); + ChromeUtils.import("resource://gre/modules/AppConstants.jsm", {}); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const { AppConstants } = ChromeUtils.import("resource://gre/modules/AppConstants.jsm"); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-chromeutils-import.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-chromeutils-import.rst new file mode 100644 index 0000000000..5c310fb607 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-chromeutils-import.rst @@ -0,0 +1,27 @@ +reject-chromeutils-import +================================ + +While ``ChromeUtils.import`` supports the fallback to ESMified module, +in-tree files should use ``ChromeUtils.importESModule`` for ESMified modules. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + ChromeUtils.import("resource://gre/modules/AppConstants.jsm"); + + ChromeUtils.defineModuleGetter( + obj, "AppConstants", "resource://gre/modules/AppConstants.jsm"); + XPCOMUtils.defineLazyModuleGetters( + obj, { AppConstants: "resource://gre/modules/AppConstants.jsm" }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + ChromeUtils.importESModule("resource://gre/modules/AppConstants.sys.mjs"); + + ChromeUtils.defineESModuleGetters( + obj, { AppConstants: "resource://gre/modules/AppConstants.sys.mjs" }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-eager-module-in-lazy-getter.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-eager-module-in-lazy-getter.rst new file mode 100644 index 0000000000..4dc29f4431 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-eager-module-in-lazy-getter.rst @@ -0,0 +1,30 @@ +reject-eager-module-in-lazy-getter +================================== + +Rejects defining a lazy getter for module that's known to be loaded early in the +startup process and it is not necessary to lazy load it. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + ChromeUtils.defineESModuleGetters(lazy, { + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", + }); + XPCOMUtils.defineLazyModuleGetters(lazy, { + XPCOMUtils: "resource://gre/modules/XPCOMUtils.jsm", + }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; + const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" + ); + const { AppConstants } = ChromeUtils.import( + "resource://gre/modules/AppConstants.jsm" + ); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-global-this.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-global-this.rst new file mode 100644 index 0000000000..b3d94321f5 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-global-this.rst @@ -0,0 +1,29 @@ +reject-global-this +====================== + +Rejects global ``this`` usage in JSM files. The global ``this`` is not +available in ESM, and this is a preparation for the migration. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + this.EXPORTED_SYMBOLS = ["foo"]; + + XPCOMUtils.defineLazyModuleGetters(this, { + AddonManager: "resource://gre/modules/AddonManager.jsm", + }); + + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const EXPORTED_SYMBOLS = ["foo"]; + + const lazy = {}; + XPCOMUtils.defineLazyModuleGetters(lazy, { + AddonManager: "resource://gre/modules/AddonManager.jsm", + }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-globalThis-modification.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-globalThis-modification.rst new file mode 100644 index 0000000000..dd4fc4b2af --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-globalThis-modification.rst @@ -0,0 +1,19 @@ +reject-globalThis-modification +============================== + +Reject any modification to ``globalThis`` inside the system modules. + +``globalThis`` is the shared global inside the system modules, and modification +on it is visible from all modules, and it shouldn't be done unless it's really +necessary. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + globalThis.foo = 10; + Object.defineProperty(globalThis, "bar", { value: 20}); + ChromeUtils.defineESModuleGetters(globalThis, { + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", + }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-import-system-module-from-non-system.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-import-system-module-from-non-system.rst new file mode 100644 index 0000000000..d168676745 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-import-system-module-from-non-system.rst @@ -0,0 +1,36 @@ +reject-import-system-module-from-non-system +=========================================== + +Rejects static import declaration for system modules (``.sys.mjs``) from non-system +modules. + +Using static import for a system module into a non-system module would create a separate instance of the imported object(s) that is not shared with the other system modules and would break the per-process singleton expectation. + +The reason for this is that inside system modules, a static import will load the module into the shared global. Inside non-system modules, the static import will load into a different global (e.g. window). This will cause the module to be loaded into different scopes, and hence create separate instances. The fix is to use ``ChromeUtils.importESModule`` which will import the object via the system module shared global scope. + + +Examples of incorrect code for this rule: +----------------------------------------- + +Inside a non-system module: + +.. code-block:: js + + import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; + +Examples of correct code for this rule: +--------------------------------------- + +Inside a non-system module: + +.. code-block:: js + + const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" + ); + +Inside a system module: + +.. code-block:: js + + import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-importGlobalProperties.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-importGlobalProperties.rst new file mode 100644 index 0000000000..60e2a3887b --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-importGlobalProperties.rst @@ -0,0 +1,48 @@ +reject-importGlobalProperties +============================= + +Rejects calls to ``Cu.importGlobalProperties`` or +``XPCOMUtils.defineLazyGlobalGetters``. + +In system modules all the required properties should already be available. In +non-module code or non-system modules, webidl defined interfaces should already +be available and hence do not need importing. + +For sjs test files, if the relevant global is not already available, then consider +extending the `list of globals available from the httpd server <https://searchfox.org/mozilla-central/rev/e9b338c2d597067f99e96d5f20769f41f312fa8f/netwerk/test/httpserver/httpd.sys.mjs#2875-2889>`_. + +Options +------- + +* "everything": Disallows using the import/getters completely. +* "allownonwebidl": Disallows using the import functions for webidl symbols. Allows + other symbols. + +everything +---------- + +Incorrect code for this option: + +.. code-block:: js + + Cu.importGlobalProperties(['TextEncoder']); + XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder']); + +allownonwebidl +-------------- + +Incorrect code for this option: + +.. code-block:: js + + // AnimationEffect is a webidl property. + Cu.importGlobalProperties(['AnimationEffect']); + XPCOMUtils.defineLazyGlobalGetters(this, ['AnimationEffect']); + +Correct code for this option: + +.. code-block:: js + + // TextEncoder is not defined by webidl. + Cu.importGlobalProperties(['TextEncoder']); + XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder']); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-lazy-imports-into-globals.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-lazy-imports-into-globals.rst new file mode 100644 index 0000000000..7fc2beb8ae --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-lazy-imports-into-globals.rst @@ -0,0 +1,33 @@ +reject-lazy-imports-into-globals +================================ + +Rejects importing lazy items into ``window`` or ``globalThis`` when in a +non-system module scope. + +Importing into the ``window`` scope (or ``globalThis``) will share the imported +global with everything else in the same window. In modules, this is generally +unnecessary and undesirable because each module imports what it requires. +Additionally, sharing items via the global scope makes it more difficult for +linters to determine the available globals. + +Instead, the globals should either be imported directly, or into a lazy object. +If there is a good reason for sharing the globals via the ``window`` scope, then +this rule may be disabled as long as a comment is added explaining the reasons. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + ChromeUtils.defineESModuleGetters(globalThis, { foo: "foo.sys.mjs" }); + ChromeUtils.defineESModuleGetters(window, { "foo": "foo.sys.mjs" }); + ChromeUtils.defineLazyGetter(globalThis, "foo", () => {}); + ChromeUtils.defineLazyGetter(window, "foo", () => {}); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const lazy = {}; + ChromeUtils.defineLazyGetter(lazy, "foo", () => {}); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-mixing-eager-and-lazy.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-mixing-eager-and-lazy.rst new file mode 100644 index 0000000000..51524f4e14 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-mixing-eager-and-lazy.rst @@ -0,0 +1,22 @@ +reject-mixing-eager-and-lazy +================================== + +Rejects defining a lazy getter for a module that's eagerly imported at +top-level script unconditionally. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + const { SomeProp } = ChromeUtils.import("resource://gre/modules/Foo.jsm"); + ChromeUtils.defineLazyModuleGetters(lazy, { + OtherProp: "resource://gre/modules/Foo.jsm", + }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const { SomeProp, OtherProp } = ChromeUtils.import("resource://gre/modules/Foo.jsm"); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-multiple-getters-calls.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-multiple-getters-calls.rst new file mode 100644 index 0000000000..7ea048402b --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-multiple-getters-calls.rst @@ -0,0 +1,27 @@ +reject-multiple-getters-calls +============================= + +Rejects multiple calls on ``ChromeUtils.defineESModuleGetters`` for the same +target in the same context. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + ChromeUtils.defineESModuleGetters(lazy, { + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", + }); + ChromeUtils.defineESModuleGetters(lazy, { + PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs", + }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + ChromeUtils.defineESModuleGetters(lazy, { + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", + PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs", + }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-relative-requires.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-relative-requires.rst new file mode 100644 index 0000000000..4387041b26 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-relative-requires.rst @@ -0,0 +1,22 @@ +reject-relative-requires +======================== + +Rejects calls to require which use relative directories. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + require("./relative/path") + require("../parent/folder/path") + loader.lazyRequireGetter(this, "path", "../parent/folder/path", true) + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + require("devtools/absolute/path") + require("resource://gre/modules/SomeModule.jsm") + loader.lazyRequireGetter(this, "path", "devtools/absolute/path", true) diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-requires-await.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-requires-await.rst new file mode 100644 index 0000000000..2a8618939f --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-requires-await.rst @@ -0,0 +1,20 @@ +reject-requires-await +===================== + +`Assert.rejects` must be preceded by an `await`, otherwise the assertion +may not be completed before the test finishes, might not be caught +and might cause intermittent issues in other tests. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + Assert.rejects(myfunc(), /startup failed/, "Should reject"); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + await Assert.rejects(myfunc(), /startup failed/, "Should reject"); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-scriptableunicodeconverter.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-scriptableunicodeconverter.rst new file mode 100644 index 0000000000..8f6ae39060 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-scriptableunicodeconverter.rst @@ -0,0 +1,13 @@ +reject-scriptableunicodeconverter +================================================ + +Rejects calls into ``Ci.nsIScriptableUnicodeConverter``. This is configured as a warning. +You should use |TextEncoder|_ or |TextDecoder|_ for new code. +If modifying old code, please consider swapping it in if possible; if this is tricky please ensure +a bug is on file. + +.. |TextEncoder| replace:: ``TextEncoder`` +.. _TextEncoder: https://searchfox.org/mozilla-central/source/dom/webidl/TextEncoder.webidl + +.. |TextDecoder| replace:: ``TextDecoder`` +.. _TextDecoder: https://searchfox.org/mozilla-central/source/dom/webidl/TextDecoder.webidl diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-some-requires.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-some-requires.rst new file mode 100644 index 0000000000..476dcbcb94 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-some-requires.rst @@ -0,0 +1,6 @@ +reject-some-requires +==================== + +This takes an option, a regular expression. Invocations of +``require`` with a string literal argument are matched against this +regexp; and if it matches, the ``require`` use is flagged. diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-top-level-await.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-top-level-await.rst new file mode 100644 index 0000000000..38be0b2d22 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/reject-top-level-await.rst @@ -0,0 +1,26 @@ +reject-top-level-await +====================== + +Rejects ``await`` at the top-level of code in modules. Top-level ``await`` is +not currently support in Gecko's component modules, so this is rejected. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + await foo; + + if (expr) { + await foo; + } + + for await (let x of [1, 2, 3]) { } + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + async function() { await foo; } + async function() { for await (let x of [1, 2, 3]) { } } diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-cc-etc.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-cc-etc.rst new file mode 100644 index 0000000000..902b4a630c --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-cc-etc.rst @@ -0,0 +1,26 @@ +use-cc-etc +====================== + +This requires using ``Cc`` rather than ``Components.classes``, and the same for +``Components.interfaces``, ``Components.results`` and ``Components.utils``. +This has a slight performance advantage by avoiding the use of the dot. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + let foo = Components.classes['bar']; + let bar = Components.interfaces.bar; + Components.results.NS_ERROR_ILLEGAL_INPUT; + Components.utils.reportError('fake'); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + let foo = Cc['bar']; + let bar = Ci.bar; + Cr.NS_ERROR_ILLEGAL_INPUT; + Cu.reportError('fake'); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-definelazygetter.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-definelazygetter.rst new file mode 100644 index 0000000000..61d13f4e63 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-definelazygetter.rst @@ -0,0 +1,22 @@ +use-chromeutils-definelazygetter +================================ + +Require use of ``ChromeUtils.defineLazyGetter`` rather than ``XPCOMUtils.defineLazyGetter``. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + XPCOMUtils.defineLazyGetter(lazy, "textEncoder", function () { + return new TextEncoder(); + }); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + ChromeUtils.defineLazyGetter(lazy, "textEncoder", function () { + return new TextEncoder(); + }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-generateqi.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-generateqi.rst new file mode 100644 index 0000000000..3da22e139a --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-generateqi.rst @@ -0,0 +1,33 @@ +use-chromeutils-generateqi +========================== + +Reject use of ``XPCOMUtils.generateQI`` and JS-implemented QueryInterface +methods in favor of ``ChromeUtils``. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + X.prototype.QueryInterface = XPCOMUtils.generateQI(["nsIMeh"]); + X.prototype = { QueryInterface: XPCOMUtils.generateQI(["nsIMeh"]) }; + X.prototype = { QueryInterface: function QueryInterface(iid) { + if ( + iid.equals(Ci.nsISupports) || + iid.equals(Ci.nsIMeh) || + iid.equals(nsIFlug) || + iid.equals(Ci.amIFoo) + ) { + return this; + } + throw Components.Exception("", Cr.NS_ERROR_NO_INTERFACE); + } }; + + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + X.prototype.QueryInterface = ChromeUtils.generateQI(["nsIMeh"]); + X.prototype = { QueryInterface: ChromeUtils.generateQI(["nsIMeh"]) } diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-import.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-import.rst new file mode 100644 index 0000000000..3072861492 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-chromeutils-import.rst @@ -0,0 +1,19 @@ +use-chromeutils-import +====================== + +Require use of ``ChromeUtils.import`` rather than ``Components.utils.import``. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + Components.utils.import("resource://gre/modules/AppConstants.jsm", this); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + ChromeUtils.import("resource://gre/modules/AppConstants.jsm", this); + ChromeUtils.defineModuleGetter(this, "AppConstants", "resource://gre/modules/AppConstants.jsm"); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-console-createInstance.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-console-createInstance.rst new file mode 100644 index 0000000000..b505cfd8fb --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-console-createInstance.rst @@ -0,0 +1,20 @@ +use-console-createInstance +========================== + +Rejects usage of Console.sys.mjs or Log.sys.mjs, preferring use of +``console.createInstance`` instead. See `Javascript Logging </toolkit/javascript-logging.html>`__ for more information. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + "resource://gre/modules/Console.sys.mjs" + "resource://gre/modules/Log.sys.mjs" + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + console.createInstance({ prefix: "Foo" }); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-default-preference-values.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-default-preference-values.rst new file mode 100644 index 0000000000..2392709e89 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-default-preference-values.rst @@ -0,0 +1,19 @@ +use-default-preference-values +============================= + +Require providing a second parameter to ``get*Pref`` methods instead of +using a try/catch block. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + try { blah = branch.getCharPref('blah'); } catch(e) {} + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + blah = branch.getCharPref('blah', true); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-includes-instead-of-indexOf.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-includes-instead-of-indexOf.rst new file mode 100644 index 0000000000..bb65ebea2c --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-includes-instead-of-indexOf.rst @@ -0,0 +1,21 @@ +use-includes-instead-of-indexOf +=============================== + +Use ``.includes`` instead of ``.indexOf`` to check if something is in an array +or string. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + let a = foo.indexOf(bar) >= 0; + let a = foo.indexOf(bar) == -1; + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + let a = foo.includes(bar); + let a = foo.indexOf(bar) > 0; diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-isInstance.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-isInstance.rst new file mode 100644 index 0000000000..7ba4163228 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-isInstance.rst @@ -0,0 +1,41 @@ +use-isInstance +============== + +Prefer ``.isInstance()`` in chrome scripts over the standard ``instanceof`` +operator for DOM interfaces, since the latter will return false when the object +is created from a different context. + +These files are covered: + +- ``*.sys.mjs`` +- ``*.jsm`` +- ``*.xhtml`` with ``there.is.only.xul`` +- ``*.js`` with a heuristic + +Since there is no straightforward way to detect chrome scripts, currently the +linter assumes that any script including the following words are chrome +privileged. This of course may not be sufficient and is open for change: + +- ``ChromeUtils``, but not ``SpecialPowers.ChromeUtils`` +- ``BrowserTestUtils``, ``PlacesUtils`` +- ``document.createXULElement`` +- ``loader.lazyRequireGetter`` +- ``Services.foo``, but not ``SpecialPowers.Services.foo`` + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + node instanceof Node + text instanceof win.Text + target instanceof this.contentWindow.HTMLAudioElement + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + Node.isInstance(node) + win.Text.isInstance(text) + this.contentWindow.HTMLAudioElement.isInstance(target) diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-ownerGlobal.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-ownerGlobal.rst new file mode 100644 index 0000000000..5d9905fc9f --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-ownerGlobal.rst @@ -0,0 +1,20 @@ +use-ownerGlobal +=============== + +Require ``.ownerGlobal`` instead of ``.ownerDocument.defaultView``. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + aEvent.target.ownerDocument.defaultView; + this.DOMPointNode.ownerDocument.defaultView.getSelection(); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + aEvent.target.ownerGlobal; + this.DOMPointNode.ownerGlobal.getSelection(); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-returnValue.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-returnValue.rst new file mode 100644 index 0000000000..1280703747 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-returnValue.rst @@ -0,0 +1,20 @@ +use-returnValue +=============== + +Warn when idempotent methods are called and their return value is unused. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + foo.concat(bar) + baz.concat() + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + a = foo.concat(bar) + b = baz.concat() diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-services.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-services.rst new file mode 100644 index 0000000000..1a57e3da10 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-services.rst @@ -0,0 +1,21 @@ +use-services +============ + +Requires the use of ``Services`` rather than ``Cc[].getService()`` where a +service is already defined in ``Services``. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + Cc["@mozilla.org/appshell/window-mediator;1"].getService(Ci.nsIWindowMediator); + Components.classes["@mozilla.org/toolkit/app-startup;1"].getService(Components.interfaces.nsIAppStartup); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + Services.wm.addListener() + Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator) diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-static-import.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-static-import.rst new file mode 100644 index 0000000000..9090dd80b7 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/use-static-import.rst @@ -0,0 +1,21 @@ +use-static-import +================= + +Requires the use of static imports in system ES module files (``.sys.mjs``) +where possible. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + const { XPCOMUtils } = ChromeUtils.importESModule("resource://gre/modules/XPCOMUtils.sys.mjs"); + const { XPCOMUtils: foo } = ChromeUtils.importESModule("resource://gre/modules/XPCOMUtils.sys.mjs"); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; + import { XPCOMUtils as foo } from "resource://gre/modules/XPCOMUtils.sys.mjs"; diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-ci-uses.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-ci-uses.rst new file mode 100644 index 0000000000..440d730e05 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-ci-uses.rst @@ -0,0 +1,42 @@ +valid-ci-uses +============= + +Ensures that interface accesses on ``Ci`` are valid, and property accesses on +``Ci.<interface>`` are also valid. + +This rule requires a full build to run, and is not turned on by default. To run +this rule manually, use: + +.. code-block:: console + + MOZ_OBJDIR=objdir-ff-opt ./mach eslint --rule="mozilla/valid-ci-uses: error" * + +Examples of incorrect code for this rule: +----------------------------------------- + +``nsIFoo`` does not exist. + +.. code-block:: js + + Ci.nsIFoo + +``UNKNOWN_CONSTANT`` does not exist on nsIURIFixup. + +.. code-block:: js + + Ci.nsIURIFixup.UNKNOWN_CONSTANT + +Examples of correct code for this rule: +--------------------------------------- + +``nsIFile`` does exist. + +.. code-block:: js + + Ci.nsIFile + +``FIXUP_FLAG_NONE`` does exist on nsIURIFixup. + +.. code-block:: js + + Ci.nsIURIFixup.FIXUP_FLAG_NONE diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-lazy.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-lazy.rst new file mode 100644 index 0000000000..a7edeeb5fc --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-lazy.rst @@ -0,0 +1,55 @@ +valid-lazy +========== + +Ensures that definitions and uses of properties on the ``lazy`` object are valid. +This rule checks for using unknown properties, duplicated symbols, unused +symbols, and also lazy getter used at top-level unconditionally. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + const lazy = {}; + if (x) { + // Unknown lazy member property {{name}} + lazy.bar.foo(); + } + +.. code-block:: js + + const lazy = {}; + ChromeUtils.defineLazyGetter(lazy, "foo", "foo.jsm"); + + // Duplicate symbol foo being added to lazy. + ChromeUtils.defineLazyGetter(lazy, "foo", "foo1.jsm"); + if (x) { + lazy.foo3.bar(); + } + +.. code-block:: js + + const lazy = {}; + // Unused lazy property foo + ChromeUtils.defineLazyGetter(lazy, "foo", "foo.jsm"); + +.. code-block:: js + + const lazy = {}; + ChromeUtils.defineLazyGetter(lazy, "foo", "foo.jsm"); + // Used at top-level unconditionally. + lazy.foo.bar(); + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + const lazy = {}; + ChromeUtils.defineLazyGetter(lazy, "foo1", () => {}); + XPCOMUtils.defineLazyModuleGetters(lazy, { foo2: "foo2.jsm" }); + + if (x) { + lazy.foo1.bar(); + lazy.foo2.bar(); + } diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-services-property.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-services-property.rst new file mode 100644 index 0000000000..c6c61abac2 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-services-property.rst @@ -0,0 +1,30 @@ +valid-services-property +======================= + +Ensures that accesses of properties of items accessed via the ``Services`` +object are valid. + +This rule requires a full build to run, and is not turned on by default. To run +this rule manually, use: + +.. code-block:: console + + MOZ_OBJDIR=objdir-ff-opt ./mach eslint --rule="mozilla/valid-services-property: error" * + +Examples of incorrect code for this rule: +----------------------------------------- + +Assuming ``foo`` is not defined within ``Ci.nsISearchService``. + +.. code-block:: js + + Services.search.foo(); + +Examples of correct code for this rule: +--------------------------------------- + +Assuming ``bar`` is defined within ``Ci.nsISearchService``. + +.. code-block:: js + + Services.search.bar(); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-services.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-services.rst new file mode 100644 index 0000000000..bd76cb52ac --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/valid-services.rst @@ -0,0 +1,24 @@ +valid-services +============== + +Ensures that accesses of the ``Services`` object are valid. +``Services`` are defined in ``tools/lint/eslint/eslint-plugin-mozilla/lib/services.json`` and can be added by copying from +``<objdir>/xpcom/components/services.json`` after a build. + +Examples of incorrect code for this rule: +----------------------------------------- + +Assuming ``foo`` is not defined within Services. + +.. code-block:: js + + Services.foo.fn(); + +Examples of correct code for this rule: +--------------------------------------- + +Assuming ``bar`` is defined within Services. + +.. code-block:: js + + Services.bar.fn(); diff --git a/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/var-only-at-top-level.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/var-only-at-top-level.rst new file mode 100644 index 0000000000..d21fc1b299 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-mozilla/rules/var-only-at-top-level.rst @@ -0,0 +1,21 @@ +var-only-at-top-level +===================== + +Marks all var declarations that are not at the top level invalid. + +Examples of incorrect code for this rule: +----------------------------------------- + +.. code-block:: js + + { var foo; } + function() { var bar; } + +Examples of correct code for this rule: +--------------------------------------- + +.. code-block:: js + + var foo; + { let foo; } + function () { let bar; } diff --git a/docs/code-quality/lint/linters/eslint-plugin-spidermonkey-js.rst b/docs/code-quality/lint/linters/eslint-plugin-spidermonkey-js.rst new file mode 100644 index 0000000000..e20c8562b6 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint-plugin-spidermonkey-js.rst @@ -0,0 +1,18 @@ +============================== +Mozilla ESLint SpiderMonkey JS +============================== + +This plugin adds a processor and an environment for the SpiderMonkey JS code. + +Processors +========== + +The processor is used to pre-process all `*.js` files and deals with the macros +that SpiderMonkey uses. + +Environments +============ + +The plugin provides a custom environment for SpiderMonkey's self-hosted code. It +adds all self-hosting functions, error message numbers, and other self-hosting +definitions as global, read-only identifiers. diff --git a/docs/code-quality/lint/linters/eslint.rst b/docs/code-quality/lint/linters/eslint.rst new file mode 100644 index 0000000000..3f409fb688 --- /dev/null +++ b/docs/code-quality/lint/linters/eslint.rst @@ -0,0 +1,226 @@ +ESLint +====== + +`ESLint`__ is a popular linter for JavaScript. The ESLint integration also uses +`Prettier`_ to enforce code formatting. + +.. contents:: + :local: + +Run Locally +----------- + +The mozlint integration of ESLint can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter eslint <file paths> + +Alternatively, omit the ``--linter eslint`` and run all configured linters, which will include +ESLint. + +ESLint also supports the ``--fix`` option to autofix most errors raised from most of the rules. + +See the `Usage guide`_ for more options. + +Custom Configurations +--------------------- + +Our ESLint configuration has a number of custom configurations that define +globals and rules for various code based on the pattern file path and names. + +Using the correct patterns helps ESLint to know about the correct details, so +that you don't get warnings about undefined or unused variables. + +* ``.mjs`` - A module file. +* ``.sys.mjs`` - A system module, this is typically a singleton in the process it is loaded into. +* ``.worker.(m)js`` - A file that is a web worker. + + * Workers that use ctypes should use ``/* eslint-env mozilla/chrome-worker */`` + +* Test files, see the section on :ref:`adding tests <adding-tests>` + + +Understanding Rules and Errors +------------------------------ + +* Only some files are linted, see the :searchfox:`configuration <tools/lint/eslint.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 `eslint.org's rule list`_ for more + information about it. +* For Mozilla specific rules (with the mozilla/ prefix), see these for more details: + + * `eslint-plugin-mozilla`_ + * `eslint-plugin-spidermonkey-js`_ + +.. _eslint_common_issues: + +Enabling new rules and adding plugins +------------------------------------- + +Please see `this page for enabling new rules <eslint/enabling-rules.html>`_. + +Common Issues and How To Solve Them +----------------------------------- + +My editor says that ``mozilla/whatever`` is unknown +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +* Run ``./mach eslint --setup``, and restart your editor. + +My editor doesn't understand a new global I've just added (e.g. to a content file or head.js file) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +* This is a limitation which is a mixture of our ESLint setup and how we share globals across files. +* Restarting your editor should pick up the new globals. +* You can always double check via ``./mach lint --linter eslint <file path>`` on the command line. + +I am getting a linter error "Unknown Services member property" +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Make sure to add any new Services to ``tools/lint/eslint/eslint-plugin-mozilla/lib/services.json``. For example by copying from +``<objdir>/xpcom/components/services.json`` after a build. + +.. _adding-tests: + +I'm adding tests, how do I set up the right configuration? +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Please note we prefer the tests to be in named directories as this makes it +easier to identify the types of tests developers are working with. Additionally, +it is not possible to scope ESLint rules to individual files based on .ini +files without a build step that would break editors, or an expensive loading +cycle. + +* If the directory path of the tests is one of the `known ones`_, then ESLint will + do the right thing for that test type. This is the preferred option. + + * For example placing xpcshell-tests in ``browser/components/foo/test/unit/`` + will set up ESLint correctly. + +* If you really can't match the directory name, e.g. like the + ``browser/base/content/tests/*``, then you'll need to add a new entry in + :searchfox:`.eslintrc-test-paths.js <.eslintrc-test-paths.js>`. + +Please do not add new cases of multiple types of tests within a single directory, +this is `difficult for ESLint to handle`_. Currently this may cause: + +* Rules to be incorrectly applied to the wrong types of test file. +* Extra definitions for globals in tests which means that the no undefined variables + rule does not get triggered in some cases. + +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:`.eslintignore <.eslintignore>`. + +This code shouldn't be formatted +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The vast majority of code should be formatted, however we allow some limited +cases where it makes sense, for example: + +* A table in an array where laying it out in a table fashion makes it more readable. +* Other structures or function calls where layout is more readable in a particular format. + +To disable prettier for code like this, ``// prettier-ignore`` may be used on +the line previous to where you want it disabled. +See the `prettier ignore docs`_ for more information. + +I have valid code that is failing the ``no-undef`` rule or can't be parsed +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +* Please do not add this to :searchfox:`.eslintignore <.eslintignore>`. Generally + this can be fixed, if the following tips don't help, please `seek help`_. +* If you are adding a new test directory, see the :ref:`section above <adding-tests>` + +* If you are writing a script loaded into special environment (e.g. frame script) you may need to tell ESLint to use the `environment definitions`_ for each case: + + * ``/* eslint-env mozilla/frame-script */`` + +* I use ``Services.scriptloader.loadSubScript``: + + * ``/* import-globals-from relative/path/to/file.js`` + +Configuration +------------- + +The global configuration file lives in ``topsrcdir/.eslintrc``. This global configuration can be +overridden by including an ``.eslintrc`` in the appropriate subdirectory. For an overview of the +supported configuration, see `ESLint'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/eslint.yml>` +* :searchfox:`Source <tools/lint/eslint/__init__.py>` + +Builders +-------- + +`Mark Banner (standard8) <https://people.mozilla.org/s?query=standard8>`__ owns +the builders. Questions can also be asked on #lint:mozilla.org on Matrix. + +ESLint (ES) +^^^^^^^^^^^ + +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 eslint --fix path/to/file``. + +For test harness issues, file bugs in Developer Infrastructure :: Lint and Formatting. + +ESLint-build (ES-B) +^^^^^^^^^^^^^^^^^^^ + +This is a tier-2 task that is run once a day at midnight UTC via a cron job. + +It currently runs the ESLint rules plus two additional rules: + +* `valid-ci-uses <eslint-plugin-mozilla/valid-ci-uses.html>`__ +* `valid-services-property <eslint-plugin-mozilla/valid-services-property.html>`__ + +These are two rules that both require build artifacts. + +To run them manually, you can run: + +``MOZ_OBJDIR=objdir-ff-opt ./mach eslint --rule "mozilla/valid-ci-uses: error" --rule "mozilla/valid-services-property: error" *`` + +For test failures, the regression causing bug may be able to be found by: + + * Determining if the file where the error is reported has been changed recently. + * Seeing if an associated ``.idl`` file has been changed. + +If no regressing bug can easily be found, file a bug in the relevant +product/component for the file where the failure is and cc :standard8. + +For test harness issues, file bugs in Developer Infrastructure :: Lint and Formatting. + +.. toctree:: + :hidden: + + eslint-plugin-mozilla + eslint-plugin-spidermonkey-js + +.. __: https://eslint.org/ +.. _Prettier: https://prettier.io/ +.. _Usage guide: ../usage.html +.. _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 +.. _seek help: ../index.html#getting-help +.. _patterns in .eslintrc.js: https://searchfox.org/mozilla-central/rev/9399e5832979755cd340383f4ca4069dd5fc7774/.eslintrc.js#24-38 +.. _environment definitions: ./eslint-plugin-mozilla/environment.html +.. _known ones: https://searchfox.org/mozilla-central/rev/287583a4a605eee8cd2d41381ffaea7a93d7b987/.eslintrc.js#24-40 +.. _difficult for ESLint to handle: https://bugzilla.mozilla.org/show_bug.cgi?id=1379669 +.. _prettier ignore docs: https://prettier.io/docs/en/ignore.html diff --git a/docs/code-quality/lint/linters/eslint/enabling-rules.rst b/docs/code-quality/lint/linters/eslint/enabling-rules.rst new file mode 100644 index 0000000000..d20a877d4e --- /dev/null +++ b/docs/code-quality/lint/linters/eslint/enabling-rules.rst @@ -0,0 +1,177 @@ +Enabling Rules and Adding Plugins to ESLint +=========================================== + +This guide is intended to give helpful pointers on how to enable rules and add +plugins to our ESLint configuration. + +.. contents:: + :local: + +General Notes +============= + +* Enabling of new rules and adding plugins should happen in agreement with the + `JavaScript Usage, Tools and Style module owner and peers </mots/index.html#javascript-usage-tools-and-style>`_. + +* Enabling of rules for sub-components should also be discussed with the owner + and peers. + + * Generally we wish to harmonize rules across the entire code base, and so + would prefer to avoid specialisms for different sub-components. + * Exceptions may be made on a sub-component basis. + +Enabling a New Rule +=================== + +The general process for enabling new rules is to file a bug under the +``Developer Infrastructure`` product in the ``Lint and Formatting`` component. + +The rule should then be added to the relevant configurations and existing issues +fixed. For large amounts of existing issues, we may do a staged roll-out +as discussed below. + +Options for Roll-Outs +--------------------- + +For rolling out new rules, we prefer that there is a plan and owner for ensuring +the existing failures are resolved over time. They do not always need to be fixed +immediately, but there should be some agreement as to how existing failures +are addressed, so that we do not end up with a large, potentially complicated +set of exclusions, or significant amounts of warnings that never get addressed. + +This is not to say the developer adding the rule needs to be the owner of the +plan, but they should ensure that there is an agreed way forward. + +There are several options available for roll-outs, depending on how many +errors are found and how much work it is to fix existing issues. + +* Fix any issues and enable the rule everywhere + + * This is most suited to cases where there are a small amount of errors which + are easy to fix up-front + +* Enable the rule everywhere, selectively disabling the rule on existing failures + + * This may be appropriate for cases where fixing the failures may take + a bit longer. + +* Enable the rule as a warning + + * This will raise issues as warnings, which will not prevent patches from + landing with issues, but should at least highlight them during code review. + * This may be more appropriate in situations where there are a large amount + of issues that are non-critical, such as preferring use of one method over + another. + +* Enable the rule as an error on passing code, but a warning on code with failures + + * This is a hybrid approach which is suited to cases where there is an issue + that is more critical, and we want to stop new cases making it into the tree, + and highlight the existing cases if the code gets touched. + +The options here are not firmly set, the list should be used as a guide. + +Where to Add +------------ + +New rules should be added in one of the configurations in +:searchfox:`eslint-plugin-mozilla <tools/lint/eslint/eslint-plugin-mozilla/lib/configs>`. + +These will then automatically be applied to the relevant places. +eslint-plugin-mozilla is used by a few projects outside of mozilla-central, +so they will pick up the rule addition when eslint-plugin-mozilla is next released. + +Where existing failures are disabled/turned to warnings, these should be handled +in the :searchfox:`top-level .eslintrc.js file <.eslintrc.js>`, and follow-up bugs +must be filed before landing and referenced in the appropriate sections. The +follow-up bugs should block +`bug 1596191 <https://bugzilla.mozilla.org/show_bug.cgi?id=1596191>`_ + +Adding a New ESLint Plugin +========================== + +License checks +-------------- + +When a new plugin is proposed, it should be checked to ensure that the licenses +of the node module and all dependent node modules are compatible with the Mozilla +code base. Mozilla employees can consult the +`Licensing & Contributor Agreements Runbook <https://mozilla-hub.atlassian.net/l/cp/bgfp6Be7>`_ +for more details. + +A site such as `npmgraph <https://npmgraph.js.org/>`_ can help with checking +licenses. + +When filing the bug or reviewing a patch, it should be stated if the module +has passed the license checks. + +Adding to the Repository +------------------------ + +If the new plugin is going to have rules defined within a configuration within +eslint-plugin-mozilla, then the module should be referenced in the peer +dependencies of +:searchfox:`eslint-plugin-mozilla's package.json<tools/lint/eslint/eslint-plugin-mozilla/package.json>` +file. + +To add the new module to the node system, run: + +.. code-block:: shell + + ./mach npm install --save-exact --save-dev packagename + +We use exact version matching to make it explicit about the version we are using +and when we upgrade the versions. + +The plugin can then be used with ESLint in the +`normal way <https://eslint.org/docs/latest/use/configure/plugins>`_. + +Packaging node_modules +---------------------- + +For our continuous integration (CI) builders, we package ``node_modules`` for +both the top-level directory, and eslint-plugin-mozilla. These are uploaded to +our CI before the patch is released. + +Currently `Mark Banner (standard8) <https://people.mozilla.org/s?query=standard8>`_ +is the only person that does this regularly, and will be automatically added as +a blocking reviewer on patches that touch the relevant ``package.json`` files. + +A Release Engineering team member would likely have permissions to upload the +files as well. + +To upload the files, the process is: + +* Obtain ToolTool credentials for the public tooltool upload space. + + * Download the `taskcluster shell from here <https://github.com/taskcluster/taskcluster/tree/main/clients/client-shell>`_, + if you haven't already. + * Run the following command. This will open a page for you to log in, and + set environment variables for the following commands to use. + +.. code-block:: shell + + eval `TASKCLUSTER_ROOT_URL=https://firefox-ci-tc.services.mozilla.com taskcluster signin -s 'project:releng:services/tooltool/api/upload/public'` + +* Upload the eslint-plugin-mozilla packages: + +.. code-block:: shell + + cd tools/lint/eslint/eslint-plugin-mozilla/ + ./update.sh + <follow the instructions> + +* Upload the top-level packages: + +.. code-block:: shell + + cd .. + ./update.sh + <follow the instructions> + +* Add the changes to the commit that changes ``package.json``. + +The update scripts automatically clean out the ``node_modules`` directories, +removes the ``package-lock.json`` files, and then does a fresh installation. This +helps to ensure a "clean" directory with only the required modules, and an up to +date ``package-lock.json`` file. diff --git a/docs/code-quality/lint/linters/file-perm.rst b/docs/code-quality/lint/linters/file-perm.rst new file mode 100644 index 0000000000..5c3a02fa1b --- /dev/null +++ b/docs/code-quality/lint/linters/file-perm.rst @@ -0,0 +1,42 @@ +File permission +=============== + +This linter verifies if a file has unnecessary permissions. +If a file has execution permissions (+x), file-perm will +generate a warning. + +It will ignore files starting with ``#!`` for types of files +that typically have shebang lines (such as python, node or +shell scripts). + +This linter does not have any affect on Windows. + + +Run Locally +----------- + +This mozlint linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter file-perm <file paths> + + +Configuration +------------- + +This linter is enabled on the whole code base. + +This job is configured as `tier 2 <https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Overview_of_the_Job_Visibility_Tiers>`_. + +Autofix +------- + +This linter provides a ``--fix`` option. The python script is doing the change itself. + + +Sources +------- + +* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/file-perm.yml>`_ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/file-perm/__init__.py>`_ diff --git a/docs/code-quality/lint/linters/file-whitespace.rst b/docs/code-quality/lint/linters/file-whitespace.rst new file mode 100644 index 0000000000..201f10d123 --- /dev/null +++ b/docs/code-quality/lint/linters/file-whitespace.rst @@ -0,0 +1,38 @@ +Trailing whitespaces +==================== + +This linter verifies if a file has: + +* unnecessary trailing whitespaces, +* Windows carriage return, +* empty lines at the end of file, +* if file ends with a newline or not + + +Run Locally +----------- + +This mozlint linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter file-whitespace <file paths> + + +Configuration +------------- + +This linter is enabled on most of the code base. + +This job is configured as `tier 2 <https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Overview_of_the_Job_Visibility_Tiers>`_. + +Autofix +------- + +This linter provides a ``--fix`` option. The python script is doing the change itself. + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/file-whitespace.yml>` +* :searchfox:`Source <tools/lint/file-whitespace/__init__.py>` diff --git a/docs/code-quality/lint/linters/fluent-lint.rst b/docs/code-quality/lint/linters/fluent-lint.rst new file mode 100644 index 0000000000..809adb4a5e --- /dev/null +++ b/docs/code-quality/lint/linters/fluent-lint.rst @@ -0,0 +1,47 @@ +Fluent Lint +=========== + +Fluent lint is a linter for Fluent files (.ftl). Currently, it includes: + +* Checks for invalid typography in messages (e.g. straight single or double quotes). +* Checks for comments layout. +* Checks for identifiers (minimum length, allowed characters). +* Hard-coded brand names. + + +Run Locally +----------- + +The mozlint integration of fluent-lint can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter fluent-lint <file paths> + +Alternatively, omit the ``--linter fluent-lint`` and run all configured linters, which will include +fluent-lint. + + +Run on Taskcluster +------------------ + +The fluent-lint job shows up as text(fluent) in the linting job. It should run automatically if +changes are made to fluent (ftl) files. + + +Configuration +------------- + +The main configuration file is found in :searchfox:`tools/lint/fluent-lint/exclusions.yml`. This provides +a way of excluding identifiers or files from checking. In general, exclusions are only to be +used for identifiers that are generated programmatically, but unfortunately, there are other +exclusions that are required for historical reasons. In almost all cases, it should *not* be +necessary to add new exclusions to this file. + + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/fluent-lint.yml>` +* :searchfox:`Source <tools/lint/fluent-lint/__init__.py>` +* :searchfox:`Test <tools/lint/test/test_fluent_lint.py>` diff --git a/docs/code-quality/lint/linters/l10n.rst b/docs/code-quality/lint/linters/l10n.rst new file mode 100644 index 0000000000..de4ce990c8 --- /dev/null +++ b/docs/code-quality/lint/linters/l10n.rst @@ -0,0 +1,45 @@ +L10n +==== + +The l10n linter checks for mistakes and problems in the localizable files. +Most of the code lives inside the +`compare-locales <https://pypi.org/project/compare-locales/>`_ +package, and is shipping as the ``moz-l10n-lint`` command. + +The linter checks for fundamental issues like parsing errors, but it also +finds more subtle mistakes like duplicated messages. It also warns if you're +trying to change a string without changing the ID, or to add a string that's +still in use in a stable channel with a different value. + +The warnings on string ID changes get reported on phabricator, but they're +not making the build fail. To find out when to change IDs and when not to, +read the :ref:`Lifecycle & Workflow <Localization>` section in the +localization documentation. + +Run Locally +----------- + +The can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter l10n <file paths> + +Alternatively, omit the ``--linter l10n`` and run all configured linters, which +will include the l10n linter. + + +Updating the Reference +---------------------- + +The linter checks out the cross-channel localization files into your +``.mozbuild`` state directory. By default this is updated automatically after +48 hours. There might be new strings anyway, if you want to ensure an +updated clone, remove the marker file in +``~/.mozbuild/gecko-strings/.hg/l10n_pull_marker``. + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/l10n.yml>` +* :searchfox:`Source <tools/lint/python/l10n_lint.py>` diff --git a/docs/code-quality/lint/linters/license.rst b/docs/code-quality/lint/linters/license.rst new file mode 100644 index 0000000000..0533333218 --- /dev/null +++ b/docs/code-quality/lint/linters/license.rst @@ -0,0 +1,39 @@ +License +======= + +This linter verifies if a file has a known license header. + +By default, Firefox uses MPL-2 license with the `appropriate headers <https://www.mozilla.org/en-US/MPL/headers/>`_. +In some cases (thirdpardy code), a file might have a different header file. +If this is the case, one of the significant line of the header should be listed in the list `of valid licenses +<https://searchfox.org/mozilla-central/source/tools/lint/license/valid-licenses.txt>`_. + +Run Locally +----------- + +This mozlint linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter license <file paths> + + +Configuration +------------- + +This linter is enabled on most of the whole code base. + +Autofix +------- + +This linter provides a ``--fix`` option. The python script is doing the change itself +and will use the right header MPL-2 header depending on the language. +It will add the license at the right place in case the file is a script (ie starting with ``!#`` +or a XML file ``<?xml>``). + + +Sources +------- + +* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/license.yml>`_ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/license/__init__.py>`_ diff --git a/docs/code-quality/lint/linters/lintpref.rst b/docs/code-quality/lint/linters/lintpref.rst new file mode 100644 index 0000000000..ca19089172 --- /dev/null +++ b/docs/code-quality/lint/linters/lintpref.rst @@ -0,0 +1,32 @@ +Lintpref +======== + +The lintpref linter is a simple linter for libpref files to check for duplicate +entries between :searchfox:`modules/libpref/init/all.js` and +:searchfox:`modules/libpref/init/StaticPrefList.yaml`. If a duplicate is found, +lintpref will raise an error and emit the ``all.js`` line where you can find +the duplicate entry. + + +Running Locally +--------------- + +The linter can be run using mach: + + .. parsed-literal:: + + $ mach lint --linter lintpref + + +Fixing Lintpref Errors +---------------------- + +In most cases, duplicate entries should be avoided and the duplicate removed +from ``all.js``. If for any reason a pref should exist in both files, the pref +should be added to ``IGNORE_PREFS`` in :searchfox:`tools/lint/libpref/__init__.py`. + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/lintpref.yml>` +* :searchfox:`Source <tools/lint/libpref/__init__.py>` diff --git a/docs/code-quality/lint/linters/mingw-capitalization.rst b/docs/code-quality/lint/linters/mingw-capitalization.rst new file mode 100644 index 0000000000..e6c51a4d14 --- /dev/null +++ b/docs/code-quality/lint/linters/mingw-capitalization.rst @@ -0,0 +1,28 @@ +MinGW capitalization +==================== + +This linter verifies that Windows include file are lowercase. +It might break the mingw build otherwise. + + +Run Locally +----------- + +This mozlint linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter mingw-capitalization <file paths> + + +Configuration +------------- + +This linter is enabled on the whole code base except WebRTC + + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/mingw-capitalization.yml>` +* :searchfox:`Source <tools/lint/cpp/mingw-capitalization.py>` diff --git a/docs/code-quality/lint/linters/perfdocs.rst b/docs/code-quality/lint/linters/perfdocs.rst new file mode 100644 index 0000000000..1ec2e33e68 --- /dev/null +++ b/docs/code-quality/lint/linters/perfdocs.rst @@ -0,0 +1,84 @@ +PerfDocs +======== + +`PerfDocs`_ is a tool that checks to make sure all performance tests are documented in tree. + +At the moment, it is only used for this documentation verification, but in the future it will also auto-generate documentation from these descriptions that will be displayed in the source-docs documentation page (rather than the wiki, which is where they currently reside). + +Run Locally +----------- + +The mozlint integration of PerfDocs can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter perfdocs . + + +Configuration +------------- + +There are no configuration options available for this linter. It scans the full source tree under ``testing``, looking for folders named ``perfdocs`` and then validates their content. This has only been implemented for Raptor so far, but Talos will be added in the future. We also hope to expand this to search outside the ``testing`` directory. + +The ``perfdocs`` folders, there needs to be an ``index.rst`` file and it needs to contain the string ``{documentation}`` in some location in the file which is where the test documentation will be placed. The folders must also have a ``config.yml`` file following this schema: + +.. code-block:: python + + CONFIG_SCHEMA = { + "type": "object", + "properties": { + "name": {"type": "string"}, + "manifest": {"type": "string"}, + "suites": { + "type": "object", + "properties": { + "suite_name": { + "type": "object", + "properties": { + "tests": { + "type": "object", + "properties": { + "test_name": {"type": "string"}, + } + }, + "description": {"type": "string"}, + }, + "required": [ + "description" + ] + } + } + } + }, + "required": [ + "name", + "manifest", + "suites" + ] + } + +Here is an example of a configuration file for the Raptor framework: + +.. parsed-literal:: + + name: raptor + manifest: testing/raptor/raptor/raptor.toml + suites: + desktop: + description: "Desktop tests." + tests: + raptor-tp6: "Raptor TP6 tests." + mobile: + description: "Mobile tests" + benchmarks: + description: "Benchmark tests." + tests: + wasm: "All wasm tests." + +Note that there needs to be a FrameworkGatherer implemented for the framework being documented since each of them may have different ways of parsing test manifests for the tests. See `RaptorGatherer <https://searchfox.org/mozilla-central/source/tools/lint/perfdocs/framework_gatherers.py>`_ for an example gatherer that was implemented for Raptor. + +Sources +------- + +* `Configuration <https://searchfox.org/mozilla-central/source/tools/lint/perfdocs.yml>`__ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/perfdocs>`__ diff --git a/docs/code-quality/lint/linters/rejected-words.rst b/docs/code-quality/lint/linters/rejected-words.rst new file mode 100644 index 0000000000..9afe7df27e --- /dev/null +++ b/docs/code-quality/lint/linters/rejected-words.rst @@ -0,0 +1,28 @@ +Rejected words +============== + +Reject some words we don't want to use in the code base for various reasons. + +Run Locally +----------- + +The mozlint integration of codespell can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter rejected-words <file paths> + + +Configuration +------------- + +This linter is enabled on the whole code base. Issues existing in the code base +are listed in the exclude list in the :searchfox:`rejected-words.yml +<tools/lint/rejected-words.yml>` file. + +New words can be added in the `payload` section. + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/rejected-words.yml>` diff --git a/docs/code-quality/lint/linters/rstlinter.rst b/docs/code-quality/lint/linters/rstlinter.rst new file mode 100644 index 0000000000..46a68d5849 --- /dev/null +++ b/docs/code-quality/lint/linters/rstlinter.rst @@ -0,0 +1,32 @@ +RST Linter +========== + +`rstcheck`_ is a popular linter for restructuredtext. + + +Run Locally +----------- + +The mozlint integration of rst linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter rst <file paths> + + +Configuration +------------- + +All directories will have rst linter run against them. +If you wish to exclude a subdirectory of an included one, you can add it to the ``exclude`` +directive. + + +.. _rstcheck: https://github.com/myint/rstcheck + + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/rst.yml>` +* :searchfox:`Source <tools/lint/rst/__init__.py>` diff --git a/docs/code-quality/lint/linters/ruff.rst b/docs/code-quality/lint/linters/ruff.rst new file mode 100644 index 0000000000..359e8dcbf6 --- /dev/null +++ b/docs/code-quality/lint/linters/ruff.rst @@ -0,0 +1,44 @@ +Ruff +==== + +`Ruff <https://github.com/charliermarsh/ruff>`_ is an extremely fast Python +linter and formatter, written in Rust. It can process all of mozilla-central in +under a second, and implements rule sets from a large array of Python linters +and formatters, including: + +* flake8 (pycodestyle, pyflakes and mccabe) +* isort +* pylint +* pyupgrade +* and many many more! + +Run Locally +----------- + +The mozlint integration of ruff can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter ruff <file paths> + + +Configuration +------------- + +Ruff is configured in the root `pyproject.toml`_ file. Additionally, ruff will +pick up any ``pyproject.toml`` or ``ruff.toml`` files in subdirectories. The +settings in these files will only apply to files contained within these +subdirs. For more details on configuration discovery, see the `configuration +documentation`_. + +For a list of options, see the `settings documentation`_. + +Sources +------- + +* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/ruff.yml>`_ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/python/ruff.py>`_ + +.. _pyproject.toml: https://searchfox.org/mozilla-central/source/pyproject.toml +.. _configuration documentation: https://beta.ruff.rs/docs/configuration/ +.. _settings documentation: https://beta.ruff.rs/docs/settings/ diff --git a/docs/code-quality/lint/linters/rustfmt.rst b/docs/code-quality/lint/linters/rustfmt.rst new file mode 100644 index 0000000000..eb7e75fa6b --- /dev/null +++ b/docs/code-quality/lint/linters/rustfmt.rst @@ -0,0 +1,33 @@ +Rustfmt +======= + +`rustfmt <https://github.com/rust-lang/rustfmt>`__ is the tool for Rust coding style. + +Run Locally +----------- + +The mozlint integration of rustfmt can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter rustfmt <file paths> + + +Configuration +------------- + +To enable rustfmt on new directory, add the path to the include +section in the :searchfox:`rustfmt.yml <tools/lint/rustfmt.yml>` file. + + +Autofix +------- + +Rustfmt is reformatting the code by default. To highlight the results, we are using +the ``--check`` option. + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/rustfmt.yml>` +* :searchfox:`Source <tools/lint/rust/__init__.py>` diff --git a/docs/code-quality/lint/linters/stylelint.rst b/docs/code-quality/lint/linters/stylelint.rst new file mode 100644 index 0000000000..1e8b6c7ff9 --- /dev/null +++ b/docs/code-quality/lint/linters/stylelint.rst @@ -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/ diff --git a/docs/code-quality/lint/linters/test-manifest-toml.rst b/docs/code-quality/lint/linters/test-manifest-toml.rst new file mode 100644 index 0000000000..1060b2756b --- /dev/null +++ b/docs/code-quality/lint/linters/test-manifest-toml.rst @@ -0,0 +1,35 @@ +Test Manifest TOML +================== + +This linter verifies syntax for ManifestParser TOML files. + +Run Locally +----------- + +This mozlint linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter test-manifest-toml <file paths> + + +Configuration +------------- + +The configuration excludes all non-ManifestParser TOML files (as well as +generated TOML manifests). + +Sources +------- + +* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/test-manifest-toml.yml>`_ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/test-manifest-toml/__init__.py>`_ + +Errors Detected +--------------- +* Invalid TOML +* Missing DEFAULT section (fixable) +* Sections not in alphabetical order (fixable) +* Section name not double quoted (fixable) +* Disabling a path by commenting out the section +* Conditional contains explicit || diff --git a/docs/code-quality/lint/linters/trojan-source.rst b/docs/code-quality/lint/linters/trojan-source.rst new file mode 100644 index 0000000000..250bdd9afe --- /dev/null +++ b/docs/code-quality/lint/linters/trojan-source.rst @@ -0,0 +1,34 @@ +Trojan Source +============= + +This linter verifies if a change is using some invalid unicode. + +The goal of this linter is to identify some potential usage of this +technique: + +https://trojansource.codes/ + +The code is inspired by the Red Hat script published: + +https://access.redhat.com/security/vulnerabilities/RHSB-2021-007#diagnostic-tools + +Run Locally +----------- + +This mozlint linter can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter trojan-source <file paths> + + +Configuration +------------- + +This linter is enabled on most of the code base on C/C++, Python and Rust. + +Sources +------- + +* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/trojan-source.yml>`_ +* `Source <https://searchfox.org/mozilla-central/source/tools/lint/trojan-source/__init__.py>`_ diff --git a/docs/code-quality/lint/linters/yamllint.rst b/docs/code-quality/lint/linters/yamllint.rst new file mode 100644 index 0000000000..e148a6aace --- /dev/null +++ b/docs/code-quality/lint/linters/yamllint.rst @@ -0,0 +1,31 @@ +yamllint +======== + +`yamllint <https://github.com/adrienverge/yamllint>`__ is a linter for YAML files. + + +Run Locally +----------- + +The mozlint integration of yamllint can be run using mach: + +.. parsed-literal:: + + $ mach lint --linter yaml <file paths> + +Alternatively, omit ``--linter yaml`` to run all configured linters, including +yamllint. + + +Configuration +------------- + +To enable yamllint on a new directory, add the path to the include section in +the :searchfox:`yaml.yml <tools/lint/yaml.yml>` file. + + +Sources +------- + +* :searchfox:`Configuration (YAML) <tools/lint/yaml.yml>` +* :searchfox:`Source <tools/lint/yamllint_/__init__.py>` diff --git a/docs/code-quality/lint/mozlint.rst b/docs/code-quality/lint/mozlint.rst new file mode 100644 index 0000000000..48a6a78a6f --- /dev/null +++ b/docs/code-quality/lint/mozlint.rst @@ -0,0 +1,23 @@ +MozLint +======= + +Linters are used in mozilla-central to help enforce coding style and avoid bad practices. +Due to the wide variety of languages in use, this is not always an easy task. +In addition, linters should be runnable from editors, from the command line, from review tools +and from continuous integration. It's easy to see how the complexity of running all of these +different kinds of linters in all of these different places could quickly balloon out of control. + +``Mozlint`` is a library that accomplishes several goals: + +1. It provides a standard method for adding new linters to the tree, which can be as easy as + defining a config object in a ``.yml`` file. This helps keep lint related code localized, and + prevents different teams from coming up with their own unique lint implementations. +2. It provides a streamlined interface for running all linters at once. Instead of running N + different lint commands to test your patch, a single ``mach lint`` command will automatically run + all applicable linters. This means there is a single API surface that other tools can use to + invoke linters. +3. With a simple taskcluster configuration, Mozlint provides an easy way to execute all these jobs + at review phase. + +``Mozlint`` isn't designed to be used directly by end users. Instead, it can be consumed by things +like mach, phabricator and taskcluster. diff --git a/docs/code-quality/lint/usage.rst b/docs/code-quality/lint/usage.rst new file mode 100644 index 0000000000..49e0ac81cc --- /dev/null +++ b/docs/code-quality/lint/usage.rst @@ -0,0 +1,138 @@ +Running Linters Locally +======================= + +Using the Command Line +---------------------- + +You can run all the various linters in the tree using the ``mach lint`` command. Simply pass in the +directory or file you wish to lint (defaults to current working directory): + +.. parsed-literal:: + + ./mach lint path/to/files + +Multiple paths are allowed: + +.. parsed-literal:: + + ./mach lint path/to/foo.js path/to/bar.py path/to/dir + +To force execution on a directory that would otherwise be excluded: + +.. parsed-literal:: + + ./mach lint -n path/in/the/exclude/list + +``Mozlint`` will automatically determine which types of files exist, and which linters need to be run +against them. For example, if the directory contains both JavaScript and Python files then mozlint +will automatically run both ESLint and Flake8 against those files respectively. + +To restrict which linters are invoked manually, pass in ``-l/--linter``: + +.. parsed-literal:: + + ./mach lint -l eslint path/to/files + +You can see a list of the available linters by running: + +.. parsed-literal:: + + ./mach lint --list + +Finally, ``mozlint`` can lint the files touched by outgoing revisions or the working directory using +the ``-o/--outgoing`` and ``-w/--workdir`` arguments respectively. These work both with mercurial and +git. In the case of ``--outgoing``, the default remote repository the changes would be pushed to is +used as the comparison. If desired, a remote can be specified manually. In git, you may only want to +lint staged commits from the working directory, this can be accomplished with ``--workdir=staged``. +Examples: + +.. parsed-literal:: + + ./mach lint --workdir + ./mach lint --workdir=staged + ./mach lint --outgoing + ./mach lint --outgoing origin/master + ./mach lint -wo + +.. _lint-vcs-hook: + +Using a VCS Hook +---------------- + +There are also both pre-commit and pre-push version control hooks that work in +either hg or git. To enable a pre-push hg hook, add the following to hgrc: + +.. parsed-literal:: + + [hooks] + pre-push.lint = python:./tools/lint/hooks.py:hg + + +To enable a pre-commit hg hook, add the following to hgrc: + +.. parsed-literal:: + + [hooks] + pretxncommit.lint = python:./tools/lint/hooks.py:hg + + +To enable a pre-push git hook, run the following command: + +.. parsed-literal:: + + $ ln -s ../../tools/lint/hooks.py .git/hooks/pre-push + + +To enable a pre-commit git hook, run the following command: + +.. parsed-literal:: + + $ ln -s ../../tools/lint/hooks.py .git/hooks/pre-commit + +Note that the symlink will be interpreted as ``.git/hooks/../../tools/lint/hooks.py``. + +Automatically Fixing Lint Errors +-------------------------------- + +``Mozlint`` has a best-effort ability to fix lint errors: + +.. parsed-literal:: + + $ ./mach lint --fix + +Not all linters support fixing, and even the ones that do can not usually fix +all types of errors. Any errors that cannot be automatically fixed, will be +printed to stdout like normal. In that case, you can also fix errors manually: + +.. parsed-literal:: + + $ ./mach lint --edit + +This requires the $EDITOR environment variable be defined. For most editors, +this will simply open each file containing errors one at a time. For vim (or +neovim), this will populate the `quickfix list`_ with the errors. + +The ``--fix`` and ``--edit`` arguments can be combined, in which case any +errors that can be fixed automatically will be, and the rest will be opened +with your $EDITOR. + +Editor Integration +================== + +.. note:: + + See details on `how to set up your editor here </contributing/editor.html#editor-ide-integration>`_ + +Editor integrations are highly recommended for linters, as they let you see +errors in real time, and can help you fix issues before you compile or run tests. + +Although mozilla-central does not currently have an integration available for +`./mach lint`, there are various integrations available for some of the major +linting tools that we use: + +* `ESLint`_ +* `Black (Python)`_ + +.. _quickfix list: http://vimdoc.sourceforge.net/htmldoc/quickfix.html +.. _ESLint: https://eslint.org/docs/user-guide/integrations#editors +.. _Black (Python): https://black.readthedocs.io/en/stable/editor_integration.html |