1231728 - Enable [no-dupe-args, no-dupe-keys, no-duplicate-case, no-obj-calls, no-with] rules

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

We can easily enable the following rules everywhere:

- no-dupe-args
- no-dupe-keys
- no-duplicate-case
- no-obj-calls
- no-with

Also, we can enable in browser/ 
- no-redeclare
- consistent-return

There are too many hits for these latter two in toolkit but I didn't want to let that block enabling them in browser/ only.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: Fixes some eslint errors/warning to enable more rules

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: Fix a duplicate switch statement in sync/setup.js

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Attachment: Enable rules

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8697327 [details] [diff] [review]
Enable rules

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

r+++ would review again!

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8697325 [details] [diff] [review]
Fixes some eslint errors/warning to enable more rules

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

::: browser/base/content/browser.js
@@ +328,5 @@
>    var backDisabled = backBroadcaster.hasAttribute("disabled");
>    var forwardDisabled = forwardBroadcaster.hasAttribute("disabled");
>    if (backDisabled == aWebNavigation.canGoBack) {
>      if (backDisabled)
> +      backBroadcaster.removeAttribute('disabled');

I don't understand this change.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

(In reply to Marco Bonardo [::mak] from comment #5)
> Comment on attachment 8697325 [details] [diff] [review]
> Fixes some eslint errors/warning to enable more rules
> 
> Review of attachment 8697325 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/browser.js
> @@ +328,5 @@
> >    var backDisabled = backBroadcaster.hasAttribute("disabled");
> >    var forwardDisabled = forwardBroadcaster.hasAttribute("disabled");
> >    if (backDisabled == aWebNavigation.canGoBack) {
> >      if (backDisabled)
> > +      backBroadcaster.removeAttribute('disabled');
> 
> I don't understand this change.

sorry, I was testing the double-quotes rules and accidentally saved this in the file

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 8697325 [details] [diff] [review]
Fixes some eslint errors/warning to enable more rules

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

ok, r=me with the quotes fixed

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/54bb45fa0e7d
https://hg.mozilla.org/integration/fx-team/rev/f4ce7427347e
https://hg.mozilla.org/integration/fx-team/rev/11c67a7b46b5

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/54bb45fa0e7d
https://hg.mozilla.org/mozilla-central/rev/f4ce7427347e
https://hg.mozilla.org/mozilla-central/rev/11c67a7b46b5