diff options
Diffstat (limited to '')
66 files changed, 2045 insertions, 0 deletions
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. |