1439142 - Fix different eslint errors in calendar and common

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[[:MakeMyDay]]

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)

Comment 1

[[:MakeMyDay]]

Attachment: FixDifferentEslint-calendar-V1.diff

Comment 2

[[:MakeMyDay]]

Attachment: FixDifferentEslint-common-V1.diff

Comment 3

[Jorg K (CEST = GMT+2)]

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?

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

(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 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 6

[Jorg K (CEST = GMT+2)]

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 :-)

Comment 7

[[:MakeMyDay]]

Attachment: FixDifferentEslint-calendar-V2.diff

Upadted patch with comments considered.

Comment 8

[[:MakeMyDay]]

(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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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