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

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 2

2 years ago
mozreview-review
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 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.
Attachment #8832528 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

2 years ago
Pushed by mbanner@mozilla.com:
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
(Assignee)

Comment 8

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

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0c237089b317
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.