Bug 1917530 - Enable ESLint rule no-shadow by default across the tree, disabling where it currently fails. r=frontend-codestyle-reviewers,mossop

Differential Revision: https://phabricator.services.mozilla.com/D221445
This commit is contained in:
Mark Banner 2024-09-12 21:08:44 +00:00
parent f63765b6ef
commit 0075a7ce1f
22 changed files with 64 additions and 67 deletions

View File

@ -1992,6 +1992,7 @@ const rollouts = [
// TODO: Bug TBD - Finish enabling no-shadow with builtinGlobals: true
// for system modules.
files: [
"browser/components/extensions/Extension*.sys.mjs",
"docshell/base/URIFixup.sys.mjs",
"dom/base/ContentAreaDropListener.sys.mjs",
"dom/manifest/ImageObjectProcessor.sys.mjs",
@ -2028,6 +2029,54 @@ const rollouts = [
"no-shadow": ["warn", { allow: ["event"], builtinGlobals: true }],
},
},
{
files: [
"browser/components/extensions/**",
"docshell/test/**",
"devtools/client/framework/**",
"dom/fetch/tests/**",
"dom/indexedDB/test/**",
"dom/media/**",
"dom/notification/test/browser/browser_permission_dismiss.js",
"dom/security/**",
"dom/tests/browser/**",
"dom/xslt/tests/browser/file_bug1309630.html",
"extensions/spellcheck/tests/chrome/test_add_remove_dictionaries.xhtml",
"gfx/layers/layerviewer/layerTreeView.js",
"image/test/browser/browser_animated_css_image.js",
"js/src/builtin/Promise.js",
"js/xpconnect/tests/**",
"layout/tools/reftest/reftest-content.js",
"mobile/android/geckoview/**",
"mobile/shared/components/extensions/**",
"netwerk/**",
"remote/cdp/**",
"remote/shared/**",
"remote/webdriver-bidi/**",
"security/manager/**",
"security/sandbox/**",
"taskcluster/docker/periodic-updates/scripts/genHPKPStaticPins.js",
"testing/condprofile/condprof/scenarii/bookmark.js",
"testing/**",
"toolkit/components/**",
"toolkit/content/**",
"toolkit/crashreporter/**",
"toolkit/modules/subprocess/subprocess_shared.js",
"toolkit/modules/tests/**",
"toolkit/mozapps/**",
"toolkit/themes/shared/design-system/tests/try-runner.js",
"tools/profiler/tests/**",
"tools/tryselect/selectors/chooser/static/filter.js",
],
excludedFiles: [
"**/*.sys.mjs",
"toolkit/components/narrate/**",
"toolkit/components/reader/**",
],
rules: {
"no-shadow": "off",
},
},
];
module.exports = { rollouts };

View File

@ -29,9 +29,9 @@ function removeOverrides(config) {
return config;
}
function readFile(path) {
function readFile(filePath) {
return fs
.readFileSync(path, { encoding: "utf-8" })
.readFileSync(filePath, { encoding: "utf-8" })
.split("\n")
.filter(p => p && !p.startsWith("#"));
}
@ -154,7 +154,7 @@ module.exports = {
},
{
...removeOverrides(xpcshellTestConfig),
files: testPaths.xpcshell.map(path => `${path}**`),
files: testPaths.xpcshell.map(filePath => `${filePath}**`),
excludedFiles: ["**/*.jsm", "**/*.mjs", "**/*.sjs"],
},
{
@ -162,7 +162,7 @@ module.exports = {
// would require searching the other test files to know if they are used or not.
// This would be expensive and slow, and it isn't worth it for head files.
// We could get developers to declare as exported, but that doesn't seem worth it.
files: testPaths.xpcshell.map(path => `${path}head*.js`),
files: testPaths.xpcshell.map(filePath => `${filePath}head*.js`),
rules: {
"no-unused-vars": [
"error",
@ -179,7 +179,7 @@ module.exports = {
// This is not done in the xpcshell-test configuration as we cannot pull
// in overrides from there. We should at some stage, aim to enable this
// for all files in xpcshell-tests.
files: testPaths.xpcshell.map(path => `${path}test*.js`),
files: testPaths.xpcshell.map(filePath => `${filePath}test*.js`),
rules: {
// No declaring variables that are never used
"no-unused-vars": [
@ -193,12 +193,12 @@ module.exports = {
},
{
...removeOverrides(browserTestConfig),
files: testPaths.browser.map(path => `${path}**`),
files: testPaths.browser.map(filePath => `${filePath}**`),
excludedFiles: ["**/*.jsm", "**/*.mjs", "**/*.sjs"],
},
{
...removeOverrides(mochitestTestConfig),
files: testPaths.mochitest.map(path => `${path}**`),
files: testPaths.mochitest.map(filePath => `${filePath}**`),
excludedFiles: [
"**/*.jsm",
"**/*.mjs",
@ -207,7 +207,7 @@ module.exports = {
},
{
...removeOverrides(chromeTestConfig),
files: testPaths.chrome.map(path => `${path}**`),
files: testPaths.chrome.map(filePath => `${filePath}**`),
excludedFiles: ["**/*.jsm", "**/*.mjs", "**/*.sjs"],
},
{
@ -218,8 +218,8 @@ module.exports = {
"mozilla/simpletest": true,
},
files: [
...testPaths.mochitest.map(path => `${path}/**/*.js`),
...testPaths.chrome.map(path => `${path}/**/*.js`),
...testPaths.mochitest.map(filePath => `${filePath}/**/*.js`),
...testPaths.chrome.map(filePath => `${filePath}/**/*.js`),
],
excludedFiles: ["**/*.jsm", "**/*.mjs", "**/*.sjs"],
},
@ -228,7 +228,7 @@ module.exports = {
// don't work well for HTML-based mochitests, so disable those.
files: testPaths.xpcshell
.concat(testPaths.browser)
.map(path => [`${path}/**/*.html`, `${path}/**/*.xhtml`])
.map(filePath => [`${filePath}/**/*.html`, `${filePath}/**/*.xhtml`])
.flat(),
rules: {
// plain/chrome mochitests don't automatically include Assert, so
@ -269,7 +269,7 @@ module.exports = {
},
{
// Exempt files with these paths since they have to use http for full coverage
files: httpTestingPaths.map(path => `${path}**`),
files: httpTestingPaths.map(filePath => `${filePath}**`),
rules: {
"@microsoft/sdl/no-insecure-url": "off",
},

View File

@ -9,9 +9,9 @@
const fs = require("fs");
const path = require("path");
function readFile(path) {
function readFile(filePath) {
return fs
.readFileSync(path, { encoding: "utf-8" })
.readFileSync(filePath, { encoding: "utf-8" })
.split("\n")
.filter(p => p && !p.startsWith("#"));
}

View File

@ -17,7 +17,6 @@ module.exports = {
"no-multi-str": "error",
"no-proto": "error",
"no-return-assign": "error",
"no-shadow": "error",
"no-unused-vars": ["error", { vars: "all", argsIgnorePattern: "^_" }],
"one-var": ["error", "never"],
radix: "error",

View File

@ -22,7 +22,5 @@ module.exports = {
// Disallow adding to native types
"no-extend-native": "error",
"no-shadow": "error",
},
};

View File

@ -151,7 +151,6 @@ module.exports = {
"no-prototype-builtins": "error",
"no-return-assign": ["error", "except-parens"],
"no-script-url": "error",
"no-shadow": "error",
"no-template-curly-in-string": "error",
"no-undef-init": "error",
"no-unmodified-loop-condition": "error",

View File

@ -117,7 +117,6 @@ module.exports = {
"no-prototype-builtins": "error",
"no-return-assign": ["error", "except-parens"],
"no-script-url": "error",
"no-shadow": "error",
"no-template-curly-in-string": "error",
"no-undef-init": "error",
"no-unmodified-loop-condition": "error",

View File

@ -4,8 +4,4 @@ module.exports = {
env: {
webextensions: true,
},
rules: {
"no-shadow": 0,
},
};

View File

@ -12,7 +12,6 @@ module.exports = {
"no-extend-native": "error",
"no-multi-str": "error",
"no-return-assign": "error",
"no-shadow": "error",
"no-unused-vars": ["error", { argsIgnorePattern: "^_", vars: "all" }],
strict: ["error", "global"],
yoda: "error",

View File

@ -141,7 +141,6 @@ module.exports = {
"no-prototype-builtins": "error",
"no-return-assign": ["error", "except-parens"],
"no-script-url": "error",
"no-shadow": "error",
"no-template-curly-in-string": "error",
"no-undef-init": "error",
"no-unmodified-loop-condition": "error",

View File

@ -54,12 +54,6 @@ module.exports = {
"mozilla/var-only-at-top-level": "off",
},
},
{
files: ["client/framework/**"],
rules: {
"no-shadow": "off",
},
},
{
files: ["client/framework/**"],
rules: {
@ -233,12 +227,6 @@ module.exports = {
// Disallow use of assignment in return statement. It is preferable for a
// single line of code to have only one easily predictable effect.
"no-return-assign": "error",
// Warn about declaration of variables already declared in the outer scope.
// This isn't an error because it sometimes is useful to use the same name
// in a small helper function rather than having to come up with another
// random name.
// Still, making this a warning can help people avoid being confused.
"no-shadow": "error",
// Disallow global and local variables that aren't used. Allow unused
// function arguments prefixed with `_`.
"no-unused-vars": ["error", { argsIgnorePattern: "^_", vars: "all" }],

View File

@ -221,12 +221,6 @@ module.exports = {
"no-self-compare": 2,
// Disallow use of comma operator.
"no-sequences": 2,
// Warn about declaration of variables already declared in the outer scope.
// This isn't an error because it sometimes is useful to use the same name
// in a small helper function rather than having to come up with another
// random name.
// Still, making this a warning can help people avoid being confused.
"no-shadow": 2,
// Disallow sparse arrays, eg. let arr = [,,2].
// Array destructuring is fine though:
// for (let [, breakpointPromise] of aPromises)

View File

@ -1,7 +0,0 @@
"use strict";
module.exports = {
rules: {
"no-shadow": "off",
},
};

View File

@ -163,9 +163,6 @@ module.exports = {
// single line of code to have only one easily predictable effect.
"no-return-assign": "error",
// Don't warn about declaration of variables already declared in the outer scope.
"no-shadow": "off",
// Allow use of synchronous methods (not a node environment).
"no-sync": "off",

View File

@ -4,8 +4,4 @@ module.exports = {
env: {
webextensions: true,
},
rules: {
"no-shadow": "off",
},
};

View File

@ -5,8 +5,4 @@ module.exports = {
browser: true,
webextensions: true,
},
rules: {
"no-shadow": 0,
},
};

View File

@ -18,7 +18,6 @@ module.exports = {
"no-inline-comments": "error",
"no-multi-str": "error",
"no-return-assign": "error",
"no-shadow": "error",
strict: ["error", "global"],
yoda: "error",
},

View File

@ -7,7 +7,6 @@
module.exports = {
rules: {
"no-inner-declarations": "error",
"no-shadow": "error",
"no-unused-vars": ["error", { vars: "all", argsIgnorePattern: "^_" }],
},
};

View File

@ -23,7 +23,6 @@ module.exports = {
rules: {
camelcase: ["error", { properties: "never" }],
"handle-callback-err": ["error", "er"],
"no-shadow": "error",
"no-undef-init": "error",
"one-var": ["error", "never"],
strict: ["error", "global"],

View File

@ -35,6 +35,5 @@ module.exports = {
// Turn off use-chromeutils-generateqi as these tests don't have ChromeUtils
// available.
"mozilla/use-chromeutils-generateqi": "off",
"no-shadow": "error",
},
};

View File

@ -285,7 +285,7 @@ module.exports = {
"no-sequences": "error",
// No declaring variables from an outer scope
// "no-shadow": "error",
"no-shadow": "error",
// Disallow throwing literals (eg. throw "error" instead of
// throw new Error("error")).

View File

@ -58,6 +58,5 @@ module.exports = {
"mozilla/no-comparison-or-assignment-inside-ok": "error",
"mozilla/no-useless-run-test": "error",
"no-shadow": "error",
},
};