1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Mark Banner (:standard8)]

We're now in a position that we can enable no-undef for some of toolkit - everything bar components/ and content/. Those directories still need a bit of work for common failures.

There's a few actual issues outside of those directory, which this patch covers as well.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues.

Review commit: https://reviewboard.mozilla.org/r/108772/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/108772/

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8832528 [details]
Bug 1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues.

https://reviewboard.mozilla.org/r/108772/#review109976

::: toolkit/modules/ObjectUtils.jsm:23
(Diff revision 1)
>  
>  // Used only to cause test failures.
>  XPCOMUtils.defineLazyModuleGetter(this, "Promise",
>    "resource://gre/modules/Promise.jsm");
>  
> +var pSlice = Array.prototype.slice;

This fixes an issue that "pSlice" isn't declared further down in the file. I've added a test for this, but I strongly suspect the particular case isn't hit in the tree.

::: toolkit/modules/ZipUtils.jsm:60
(Diff revision 1)
>  
>    function readFailed(error) {
>      try {
>        aStream.close();
>      } catch (e) {
> -      logger.error("Failed to close JAR stream for " + aPath);
> +      Cu.reportError("Failed to close JAR stream for " + aPath);

These were moved from a file that had logger, but this one doesn't.

::: toolkit/modules/moz.build:14
(Diff revision 1)
>  MOCHITEST_MANIFESTS += ['tests/mochitest/mochitest.ini']
>  MOCHITEST_CHROME_MANIFESTS += ['tests/chrome/chrome.ini']
>  
>  TESTING_JS_MODULES += [
> -    'tests/MockDocument.jsm',
> -    'tests/PromiseTestUtils.jsm',
> +    'tests/modules/MockDocument.jsm',
> +    'tests/modules/PromiseTestUtils.jsm',

These moved as it means we can enable the xpcshell eslint config for them, but not enable the xpcshell particulars for the other tests in the sub-directories.

::: toolkit/modules/tests/xpcshell/test_sqlite.js
(Diff revision 1)
>  
>      handleError(error) {
>        print("Error when executing SQL (" + error.result + "): " +
>              error.message);
>        print("Original error: " + error.error);
> -      errors.push(error);

It was like this when first committed. Given the promise is rejected, the test will do the right thing.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8832528 [details]
Bug 1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues.

https://reviewboard.mozilla.org/r/108772/#review110030

::: toolkit/modules/tests/browser/browser_InlineSpellChecker.js:6
(Diff revision 1)
> +var InlineSpellChecker;
> +
>  function test() {
>    let tempScope = {};
>    Components.utils.import("resource://gre/modules/InlineSpellChecker.jsm", tempScope);
> -  let InlineSpellChecker = tempScope.InlineSpellChecker;
> +   InlineSpellChecker = tempScope.InlineSpellChecker;

The indentation seems off here.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8832528 [details]
Bug 1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108772/diff/1-2/

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8832528 [details]
Bug 1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108772/diff/2-3/

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8832528 [details]
Bug 1335813 - Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108772/diff/3-4/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c237089b317
Enable eslint no-undef for toolkit, apart from components/ and content/, and fix various issues. r=jaws

Comment 8

[Mark Banner (:standard8)]

Note: the updates were addressing the nit, and ensuring the PromiseTestUtils.jsm was correctly marked as moved rather than delete/add.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

\o/

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0c237089b317