Closed Bug 1439142 Opened 2 years ago Closed 2 years ago

Fix different eslint errors in calendar and common

Categories

(Calendar :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: MakeMyDay, Assigned: MakeMyDay)

Details

Attachments

(2 files, 1 obsolete file)

We currently have some linting errors in calendar and a few in common:

[task 2018-02-16T22:55:19.808Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/calendar-common-sets.js:117:23 | Method 'isCommandEnabled' has a complexity of 87. (complexity)
[task 2018-02-16T22:55:19.808Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/calendar-common-sets.js:287:39 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.808Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/calendar-management.js:182:13 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.808Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/calendar-task-view.js:56:17 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/calendar-widgets.xml:414:37 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/minimonth.xml:469:14 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/minimonth.xml:1026:16 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/minimonth.xml:1027:20 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/minimonth.xml:1032:23 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/calExtract.jsm:118:13 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/calExtract.jsm:872:23 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/calExtract.jsm:879:23 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/calExtract.jsm:887:23 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/calExtract.jsm:903:23 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/lightning/content/lightning-item-iframe.js:518:26 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/lightning/content/lightning-item-iframe.js:755:44 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/providers/wcap/calWcapCalendarItems.js:274:39 | Function has a complexity of 90. (complexity)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/test/mozmill/shared-modules/test-calendar-utils.js:328:16 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/test/mozmill/shared-modules/test-calendar-utils.js:537:30 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/test/mozmill/shared-modules/test-calendar-utils.js:562:26 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/test/mozmill/shared-modules/test-timezone-utils.js:125:33 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.809Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/common/bindings/preferences.xml:812:18 | Don't compare for inexact equality against boolean literals (mozilla/no-compare-against-boolean-literals)
[task 2018-02-16T22:55:19.810Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/common/src/extensionSupport.jsm:7:5 | Ci is now defined in global scope, a separate definition is no longer necessary. (mozilla/no-define-cc-etc)
[task 2018-02-16T22:55:19.810Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/common/src/extensionSupport.jsm:8:5 | Cc is now defined in global scope, a separate definition is no longer necessary. (mozilla/no-define-cc-etc)
[task 2018-02-16T22:55:19.810Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/common/src/extensionSupport.jsm:9:5 | Cu is now defined in global scope, a separate definition is no longer necessary. (mozilla/no-define-cc-etc)
Attachment #8951889 - Flags: review?(philipp)
Attachment #8951890 - Flags: review?(jorgk)
Comment on attachment 8951890 [details] [diff] [review]
FixDifferentEslint-common-V1.diff

Review of attachment 8951890 [details] [diff] [review]:
-----------------------------------------------------------------

::: common/bindings/preferences.xml
@@ +808,5 @@
>              var cancel = !aTarget.dispatchEvent(event);
>              if (aTarget.hasAttribute("on" + aEventName)) {
>                var fn = new Function("event", aTarget.getAttribute("on" + aEventName));
>                var rv = fn.call(aTarget, event);
> +              if (!rv)

Hmm, if (rv == false) was forked from M-C recently, still in m-esr52:
https://dxr.mozilla.org/mozilla-esr52/rev/f7affebe68e41b22f3a4c751487dce8021873f46/toolkit/content/widgets/preferences.xml#802
How did it pass linting when this code was still in M-C?

This is twisted code. Say aEventName=="Load". So this checks whether aTarget has an "onLoad" attribute, if so, to constructs a function 'fn' with the body retrieved from the "onLoad" attribute and runs it, then checks the return value.

How do we know what the function the "onLoad" function returns. If it returns true or false, then your change is fine, if it doesn't return anything or zero, then you've changed the behaviour of that code.

Maybe if(typeof(rv) == "boolean" && !rv).

Or why do you think your change is correct under all circumstances? Am I missing something?
(In reply to Jorg K (GMT+1) from comment #3)
> Maybe if(typeof(rv) == "boolean" && !rv).
This is essentially if (rv === false), which is ok by the new rules. What was there before was a inexact match, so anything that is falsey, boolean or not. Therefore !rv is equivalent to rv == false.
Comment on attachment 8951889 [details] [diff] [review]
FixDifferentEslint-calendar-V1.diff

Review of attachment 8951889 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with these changes.

::: calendar/base/content/calendar-common-sets.js
@@ +113,5 @@
>          }
>          return false;
>      },
>  
> +    /* eslint-disable complexity */

Let's just raise complexity to 90. Looks like they changed the calculation.

::: calendar/base/content/calendar-management.js
@@ +178,5 @@
>              tooltipText = cal.calGetString("calendar", "tooltipCalendarReadOnly", [calendar.name]);
>          }
>      }
>      setElementValue("calendar-list-tooltip", tooltipText, "label");
> +    return (typeof tooltipText == "string");

I'd go with tooltipText !== false here.
Attachment #8951889 - Flags: review?(philipp) → review+
Comment on attachment 8951890 [details] [diff] [review]
FixDifferentEslint-common-V1.diff

OK then, thanks for the clarification. I'll land it with r=philipp,jorgk then :-)
Attachment #8951890 - Flags: review?(jorgk) → review+
Upadted patch with comments considered.
Attachment #8951889 - Attachment is obsolete: true
Attachment #8951992 - Flags: review+
(In reply to Jorg K (GMT+1) from comment #3)

> Or why do you think your change is correct under all circumstances? Am I
> missing something?

Philipp already explained that the previous check was already not strict (and otherwise the rule wouldn't have thrown), so the ! check is the equivalent here.
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/4644f1f698d8
Fix eslint errors for no-define-cc-etc and no-compare-against-boolean-literals in common. r=philipp
https://hg.mozilla.org/comm-central/rev/51c1c477a926
Fix eslint errors for complexity and no-compare-against-boolean-literals in calendar. r=philipp
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 6.2
You need to log in before you can comment on or make changes to this bug.