1228363 - "mach eslint devtools" is broken

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

It appears to be related to the balanced-listeners rule from the mozilla plugin:

$ mach eslint devtools/
 0:00.11 Running /usr/bin/eslint in devtools/
 0:00.11 /usr/bin/eslint --ext [.js,.jsm,.jsx] .
/home/past/src/gecko/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js:41
      type: node.arguments[0].value,
                             ^

TypeError: Cannot read property 'value' of undefined
    at addAddedListener (/home/past/src/gecko/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js:41:30)
    at EventEmitter.CallExpression (/home/past/src/gecko/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js:91:11)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.enterNode (/usr/lib/node_modules/eslint/lib/util/node-event-generator.js:42:22)
    at CommentEventGenerator.enterNode (/usr/lib/node_modules/eslint/lib/util/comment-event-generator.js:98:23)
    at Controller.controller.traverse.enter (/usr/lib/node_modules/eslint/lib/eslint.js:782:36)
    at Controller.__execute (/usr/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/usr/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:495:28)
    at EventEmitter.module.exports.api.verify (/usr/lib/node_modules/eslint/lib/eslint.js:779:24)

I see this on both Mac and Linux and is not fixed by "mach eslint --setup".

Comment 1

[Patrick Brosset <:pbro>]

Thanks Panos.
This seems to happen with (at least) this file: devtools\client\performance\legacy\actors.js
On line 92, it does: `this._poller.on();`
And this tricks the rule into thinking it's a new listener being added.
So we should fix the rule to make sure it does not assume that all instances of '.on(' are actually event listeners.

Comment 2

[Patrick Brosset <:pbro>]

Attachment: Bug_1228363_-_Don_t_assume_on_off_addEventListener.diff

Panos, do you mind reviewing this change?

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: MozReview Request: Bug 1228363 - stop mach eslint devtools throwing on empty on() and off() calls r?=past

Bug 1228363 - stop mach eslint devtools throwing on empty on() and off() calls r?=past

The error is triggered by this._poller.on() because we assume we are adding an event listener and on() has no arguments.

Easy fix.

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8692584 [details]
MozReview Request: Bug 1228363 - stop mach eslint devtools throwing on empty on() and off() calls r?=past

https://reviewboard.mozilla.org/r/26287/#review23731

Stealing review from :past.
Let's land this.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e0a029b6cdde

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e0a029b6cdde