Enable the ESLint no-shadow rule
#11745
Conversation
This rule is *not* currently enabled in mozilla-central, but it appears commented out[1] in the ESLint definition file; see https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js#238-239 Unfortunately this rule is, for fairly obvious reasons, impossible to `--fix` automatically (even partially) and each case thus required careful manual analysis. Hence this ESLint rule is, by some margin, probably the most difficult one that we've enabled thus far. However, using this rule does seem like a good idea in general since allowing variable shadowing could lead to subtle (and difficult to find) bugs or at the very least confusing code. Please find additional details about the ESLint rule at https://eslint.org/docs/rules/no-shadow --- [1] Most likely, a very large number of lint errors have prevented this rule from being enabled thus far.
…linting errors Given the way that "classes" were previously implemented in PDF.js, using regular functions and closures, there's a fair number of false positives when the `no-shadow` ESLint rule was enabled. Note that while *some* of these `eslint-disable` statements can be removed if/when the relevant code is converted to proper `class`es, we'll probably never be able to get rid of all of them given our naming/coding conventions (however I don't really see this being a problem).
…n `test/webbrowser.js` file This looks entirely like something which was left-over from debugging, and that line hasn't been touched since PR 4515, especially considering that the corresponding branch in `FirefoxBrowser` doesn't print anything.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/70dbe39139e8f02/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/70dbe39139e8f02/output.txt Total script time: 2.56 mins Published |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/f6e8fff0935d284/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/06fb75efab0792d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/f6e8fff0935d284/output.txt Total script time: 19.81 mins
Image differences available at: http://54.67.70.0:8877/f6e8fff0935d284/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/06fb75efab0792d/output.txt Total script time: 25.12 mins
Image differences available at: http://54.215.176.217:8877/06fb75efab0792d/reftest-analyzer.html#web=eq.log |
|
Really good to have this enabled; thanks! |
This rule is not currently enabled in mozilla-central, but it appears commented out[1] in the ESLint definition file; see https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js#238-239
Unfortunately this rule is, for fairly obvious reasons, impossible to
--fixautomatically (even partially) and each case thus required careful manual analysis.Hence this ESLint rule is, by some margin, probably the most difficult one that we've enabled thus far. However, using this rule does seem like a good idea in general since allowing variable shadowing could lead to subtle (and difficult to find) bugs or at the very least confusing code.
Please find additional details about the ESLint rule at https://eslint.org/docs/rules/no-shadow
[1] Most likely, a very large number of lint errors have prevented this rule from being enabled thus far.