1215455 - Use eslint --fix to turn on additional rules

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Description

[Ed Lee :Mardak]

With eslint 1.6 from bug 1210774, we get --fix!

A number of spacing rules can be turned on with --fix autofixing. "indent" doesn't seem to work quite right though. Some rules can be turned on without any code changes too.

Comment 1

[Ed Lee :Mardak]

Attachment: 7 patches in one

Here's 7 patches in one file. The first patch is turning on rules that don't require code changes. The following 6 patches add one rule and run with --fix:

browser/components/loop$ ./standalone/node_modules/.bin/eslint --fix --ext .js --ext .jsm --ext .jsx .

Comment 2

[Ed Lee :Mardak]

Comment on attachment 8674767 [details] [diff] [review]
7 patches in one

>+    "object-curly-spacing": 2,
>-          <li className={cx({ "dropdown-menu-item": true,
>+          <li className={cx({"dropdown-menu-item": true,
>                               "disabled": this.props.blocked,
Hrmm.. not so great at aligning the other properties

Comment 3

[Ed Lee :Mardak]

Attachment: 0 - no code change

Comment 4

[Ed Lee :Mardak]

Attachment: 1 - no-multi-spaces

Comment 5

[Ed Lee :Mardak]

Attachment: 2 - array-bracket-spacing

Comment 6

[Ed Lee :Mardak]

Attachment: 3 - object-curly-spacing

Comment 7

[Ed Lee :Mardak]

Attachment: 4 - space-after-keywords

Comment 8

[Ed Lee :Mardak]

Attachment: 5 - space-before-blocks

Comment 9

[Ed Lee :Mardak]

Attachment: 6 - space-before-fuction-paren

Comment 10

[Ed Lee :Mardak]

Attachment: 3 - object-curly-spacing always

Comment 11

[Ed Lee :Mardak]

Attachment: 0 - no code change

Turns out there's some more rules that can be turned on now without any code changes. The following patches are rebased on top of latest fx-team, so panel contacts removal is in.

Comment 12

[Ed Lee :Mardak]

Attachment: 0 - no code change

Comment 13

[Ed Lee :Mardak]

Attachment: 1 - no-multi-spaces

Comment 14

[Ed Lee :Mardak]

Attachment: 2 - array-bracket-spacing

Comment 15

[Ed Lee :Mardak]

Attachment: 3 - object-curly-spacing

Comment 16

[Ed Lee :Mardak]

Attachment: 4 - space-after-keywords

Comment 17

[Ed Lee :Mardak]

Attachment: 5 - space-before-blocks

Comment 18

[Ed Lee :Mardak]

Attachment: 6 - space-before-fuction-paren

Comment 19

[Ed Lee :Mardak]

Attachment: 7 - operator-assignment

Comment 20

[Ed Lee :Mardak]

Attachment: 8 - space-in-parens

Comment 21

[Ed Lee :Mardak]

Attachment: 9 - react/jsx-curly-spacing

Comment 22

[Ed Lee :Mardak]

Attachment: 10 - consistent-return

Comment 23

[Ed Lee :Mardak]

Attachment: example code before, errors, after

Comment 24

[Ed Lee :Mardak]

Attachment: example code before, errors, after

forgot to include the rules that didn't need code changes

Comment 25

[Mark Banner (:standard8)]

Comment on attachment 8675237 [details] [diff] [review]
2 - array-bracket-spacing

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

::: browser/components/loop/.eslintrc
@@ +33,5 @@
>      // want to audit these rules, and start turning them on and fixing the
>      // problems they find, one at a time.
>  
>      // Eslint built-in rules are documented at <http://eslint.org/docs/rules/>
> +    "array-bracket-spacing": 2,

This needs to be specified as [2, "never"] as that's what it is currently defaulting two, but just reading the file doesn't tell you that.

However, its also slightly strange - this seems to conflict with the object-curly-spacing setting which you're setting to "always".

Maybe we should have a short discussion after a standup about this?

Comment 26

[Mark Banner (:standard8)]

Comment on attachment 8675243 [details] [diff] [review]
8 - space-in-parens

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

::: browser/components/loop/.eslintrc
@@ +109,5 @@
>      "space-before-blocks": 2,
>      "space-before-function-paren": [2, "never"],
>      "space-before-keywords": 2,
>      "space-infix-ops": 2,
> +    "space-in-parens": 2,

We should specify "never" here as well, as it isn't clear from the documentation that this is the case (and would make it clearer for readers).

Comment 27

[Mark Banner (:standard8)]

Comment on attachment 8675244 [details] [diff] [review]
9 - react/jsx-curly-spacing

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

::: browser/components/loop/.eslintrc
@@ +128,5 @@
>      "react/no-unknown-property": 2,
>      "react/prop-types": 2,
>      "react/self-closing-comp": 2,
>      "react/wrap-multilines": 2,
> +    "react/jsx-curly-spacing": 2,

Please add the "never"

Comment 28

[Mark Banner (:standard8)]

Comment on attachment 8675243 [details] [diff] [review]
8 - space-in-parens

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

r+ with the previous comments fixed

Comment 29

[Mark Banner (:standard8)]

Comment on attachment 8675243 [details] [diff] [review]
8 - space-in-parens

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

r+ with the previous comments fixed

Comment 30

[Mark Banner (:standard8)]

Comment on attachment 8675244 [details] [diff] [review]
9 - react/jsx-curly-spacing

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

r+ with the previous comments fixed

Comment 31

[Mark Banner (:standard8)]

Ok, I've flagged everything with + or -, there's just the two that feel like inconsistencies that I think we should ask the wider group.

Comment 32

[Ed Lee :Mardak]

https://hg.mozilla.org/integration/fx-team/rev/ffeca1025cd07845ef400d7bcd1b699d075a6cda
Bug 1215455 - Use eslint --fix to turn on additional rules [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/70d83aad0cc13333ab6ba541f897e6b05e21917a
Bug 1215455 - eslint no-multi-spaces [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/2788036d08a80b062f70832a0a75e37acccc182e
Bug 1215455 - eslint array-bracket-spacing [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/5f023c0c5fb33e4ac636d9c6d13892e377ef4e97
Bug 1215455 - eslint object-curly-spacing [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/412326380fc25f783726a4c9baa54c3d6fe78983
Bug 1215455 - eslint space-after-keywords [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/de78149fb3db564d5a52af4b41a916d0d2703287
Bug 1215455 - eslint space-before-blocks [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/d39db1517fb2fbbbe14c11c71e2beb871725074a
Bug 1215455 - eslint space-before-function-paren [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/39f32e5b6af91d4d7a63f332a472e05d09a2f5de
Bug 1215455 - eslint operator-assignment [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/2eb2b6388613a26f581f7339ea1d8c4082abf3f2
Bug 1215455 - eslint space-in-parens [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/9b5bd211ecde43cc4131c9aeeddd457d72ab878a
Bug 1215455 - eslint react/jsx-curly-spacing [r=Standard8]

https://hg.mozilla.org/integration/fx-team/rev/ef65374a93d6353004c78091cb65a2f41fec7aeb
Bug 1215455 - eslint consistent-return [r=Standard8]

Comment 33

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ffeca1025cd0
https://hg.mozilla.org/mozilla-central/rev/70d83aad0cc1
https://hg.mozilla.org/mozilla-central/rev/2788036d08a8
https://hg.mozilla.org/mozilla-central/rev/5f023c0c5fb3
https://hg.mozilla.org/mozilla-central/rev/412326380fc2
https://hg.mozilla.org/mozilla-central/rev/de78149fb3db
https://hg.mozilla.org/mozilla-central/rev/d39db1517fb2
https://hg.mozilla.org/mozilla-central/rev/39f32e5b6af9
https://hg.mozilla.org/mozilla-central/rev/2eb2b6388613
https://hg.mozilla.org/mozilla-central/rev/9b5bd211ecde
https://hg.mozilla.org/mozilla-central/rev/ef65374a93d6