1571567 - Enable no-fallthrough rule in the recommended config

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

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

This rule would have prevented bug 1570945.

Comment 1

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

Attachment: Bug 1571567 - Enable the no-fallthrough eslint rule. r?standard8

Comment 2

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /browser. r?k88hudson!

Comment 3

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /toolkit. r?MattN

Comment 4

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /docshell. r?Standard8

Comment 5

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /dom. r?smaug

Comment 6

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /js. r?jorendorff

Comment 7

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /layout. r?dholbert

Comment 8

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /netwerk. r?mayhemer

Comment 9

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /services. r?markh

Comment 10

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /testing. r?jmaher

Comment 11

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

Attachment: Bug 1571567 - Fix no-fallthrough errors in /toolkit.

Comment 12

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

I have to unassign myself from this bug. Mark, are you able to fix up any remaining issues and land this?

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/606bafb8211c
Enable the no-fallthrough eslint rule. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/7d5fc57b2809
Fix no-fallthrough errors in /browser. r=MattN,k88hudson
https://hg.mozilla.org/integration/autoland/rev/42df735c8556
Fix no-fallthrough errors in /toolkit. r=MattN
https://hg.mozilla.org/integration/autoland/rev/7ec086bb5bd5
Fix no-fallthrough errors in /docshell. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/2c67015f12c6
Fix no-fallthrough errors in /dom. r=smaug
https://hg.mozilla.org/integration/autoland/rev/e2ed4620f232
Fix no-fallthrough errors in /js. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/fbdf6b75a484
Fix no-fallthrough errors in /layout. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/777d79076e99
Fix no-fallthrough errors in /netwerk. r=dragana
https://hg.mozilla.org/integration/autoland/rev/7aa97ba7cce9
Fix no-fallthrough errors in /services. r=markh
https://hg.mozilla.org/integration/autoland/rev/ce83fa75ae32
Fix no-fallthrough errors in /testing. r=jmaher

Comment 14

[Mark Banner (:standard8)]

Attachment: Bug 1571567 - Fix test failures from enabling no-fallthrough. r?daleharvey

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f699e0bfbad
Fix test failures from enabling no-fallthrough. r=daleharvey CLOSED TREE

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 8f699e0bfbad (Bug 1571567) for es lint failure on browser/head.js

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linting%2Copt%2Csource-test-mozlint-eslint%2C%28es%29%2C738873c683f243d90c2843e79eb3f4c7f7871045&fromchange=c44d035213c1b4e8f2293c69dbdde06cdeaa15a8&tochange=50d4fa5e5e09d705380c92764df96ca156d86595&selectedJob=260598465

Backout link: https://hg.mozilla.org/integration/autoland/rev/50d4fa5e5e09d705380c92764df96ca156d86595

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260598465&repo=autoland&lineNumber=275

[task 2019-08-08T14:39:45.849Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2019-08-08T14:53:54.511Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/mozapps/extensions/test/browser/head.js:1413:7 | Delete ?? (prettier/prettier)
[taskcluster 2019-08-08 14:53:55.576Z] === Task Finished ===
[taskcluster 2019-08-08 14:53:56.923Z] Unsuccessful task run with exit code: 1 completed in 1103.291 seconds

I'll also backout the initial push

Comment 17

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1bdc57b9b3e
Backed out 10 changesets complementary backout after es lint failure on the patch CLOSED TREE

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/328db867c177
Enable the no-fallthrough eslint rule. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/1e7c298b1d68
Fix no-fallthrough errors in /browser. r=MattN,k88hudson
https://hg.mozilla.org/integration/autoland/rev/c0ba1b368b58
Fix no-fallthrough errors in /toolkit. r=MattN
https://hg.mozilla.org/integration/autoland/rev/eb557d4ba927
Fix no-fallthrough errors in /docshell. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/e54f2402aa49
Fix no-fallthrough errors in /dom. r=smaug
https://hg.mozilla.org/integration/autoland/rev/cade7a6a7948
Fix no-fallthrough errors in /js. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/9e23c2da81e3
Fix no-fallthrough errors in /layout. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/2db0ad03c4af
Fix no-fallthrough errors in /netwerk. r=dragana
https://hg.mozilla.org/integration/autoland/rev/34e6203ba194
Fix no-fallthrough errors in /services. r=markh
https://hg.mozilla.org/integration/autoland/rev/381a12355930
Fix no-fallthrough errors in /testing. r=jmaher

Comment 19

[Mark Banner (:standard8)]

I've fixed the issues and relanded, so hopefully good now. Re-assigning back to Jared as he did most of the work here.

Comment 20

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/328db867c177
https://hg.mozilla.org/mozilla-central/rev/1e7c298b1d68
https://hg.mozilla.org/mozilla-central/rev/c0ba1b368b58
https://hg.mozilla.org/mozilla-central/rev/eb557d4ba927
https://hg.mozilla.org/mozilla-central/rev/e54f2402aa49
https://hg.mozilla.org/mozilla-central/rev/cade7a6a7948
https://hg.mozilla.org/mozilla-central/rev/9e23c2da81e3
https://hg.mozilla.org/mozilla-central/rev/2db0ad03c4af
https://hg.mozilla.org/mozilla-central/rev/34e6203ba194
https://hg.mozilla.org/mozilla-central/rev/381a12355930

Comment 21

[Jorg K (CEST = GMT+2)]

We're looking at doing the same for TB. Why is this OK to remove?
https://hg.mozilla.org/mozilla-central/rev/c0ba1b368b5880253f4c6a5b3deced3d7044cf19#l13.12

Comment 22

[Jorg K (CEST = GMT+2)]

There was even a comment in https://phabricator.services.mozilla.com/D40746
I believe this change in behaviour is incorrect if this comment (https://searchfox.org/mozilla-central/rev/e62c920f7f6463239c6634113f8a8351e263b936/toolkit/modules/PopupNotifications.jsm#1309-1310) is correct.

Comment 23

[Mark Banner (:standard8)]

Looks like Jared made changes andmarked that comment as done, so I didn't notice it when landing.

I think what happened here is that initially the break; was moved from inside the if to a higher level. When Matt commented, it then got changed from a break; to a fall-through without re-inserting the original break;.

This looks like just an optimisation to avoid unnecessary updates, but probably safer to put it back in for now - I've filed bug 1575197 to handle that.

Comment 24

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

(In reply to Mark Banner (:standard8) from comment #23)

Looks like Jared made changes andmarked that comment as done, so I didn't notice it when landing.

I think what happened here is that initially the break; was moved from inside the if to a higher level. When Matt commented, it then got changed from a break; to a fall-through without re-inserting the original break;.

This looks like just an optimisation to avoid unnecessary updates, but probably safer to put it back in for now - I've filed bug 1575197 to handle that.

Yeah, that's what happened. Sorry about that and thank you for fixing it.