1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement, P2)

Description

[Mark Banner (:standard8)]

generator-star-spacing is part of ESLint style related rules to help make code more consistent.

http://eslint.org/docs/rules/generator-star-spacing

It is already in use for some areas, so we should expand it further. Although we're starting to move away from a lot of generators, I think it is still useful for consistency for the remaining instances.

https://dxr.mozilla.org/mozilla-central/search?q=generator-star-spacing&redirect=false

I'm willing to mentor this bug. Approximate steps:

- Ensure you've got ESLint setup on mozilla-central (https://developer.mozilla.org/docs/ESLint).
- In tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js un-comment the generator-star-spacing line.
- Remove the generator-star-spacing lines from the other .eslintrc.js files (* apart from devtools/ *)
- Run:

./mach eslint

- You should be able to fix the failing instances automatically with:

./mach eslint --fix

or maybe

./mach eslint --fix <directory>

- Check the diff in case there's indentation that needs adjusting.
- Commit to a patch and push it to mozreview (http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html)

Comment 1

[Florian Quèze [:florian]]

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

> Although we're starting to move away from a lot of generators, I think it is
> still useful for consistency for the remaining instances.

You may want to wait until I've ran my Task.jsm removal scripts on the rest of the tree before fixing this throughout the tree.

Comment 2

[Mark Banner (:standard8)]

Ok, dropping it from the mentored list for now.

Comment 3

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> (In reply to Mark Banner (:standard8) from comment #0)
> 
> > Although we're starting to move away from a lot of generators, I think it is
> > still useful for consistency for the remaining instances.
> 
> You may want to wait until I've ran my Task.jsm removal scripts on the rest
> of the tree before fixing this throughout the tree.

This was referring to bug 1374282 which is now fixed.

Comment 4

[Mark Banner (:standard8)]

We can start moving this forward again - see comment 0 for details.

Comment 5

[Steve Armand]

Attachment: Bug 1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central.

Review commit: https://reviewboard.mozilla.org/r/175890/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175890/

Comment 6

[Steve Armand]

I followed the steps in comment #0 (including avoiding the devtools/ files) and completed the clean-up. Patch posted...

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8904113 [details]
Bug 1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central.

https://reviewboard.mozilla.org/r/175890/#review180908

Thank you for the patch Steve, it looks good, there's just one minor bit to fix up, then we'll be good to go.

::: tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js:116
(Diff revision 1)
>  
>      // No spaces between function name and parentheses
>      "func-call-spacing": "error",
>  
>      // Require function* name()
> -    // "generator-star-spacing": ["error", {"before": false, "after": true}],
> +    "generator-star-spacing": ["error", {"before": false, "after": true}],

This isn't really your fault, the line needs to change to:

"generator-star-spacing": ["error", {"after": true, "before": false}],

This is due to the sort-keys rule being on the eslint-plugin-mozilla. You can re-check it with:

./mach eslint tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js

Comment 8

[Steve Armand]

Comment on attachment 8904113 [details]
Bug 1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175890/diff/1-2/

Comment 9

[Steve Armand]

Mark, I had seen a message about "expected object keys to be in ascending order" message when I tested eslint and wasn't quite sure if I should make the change, so thanks for clarifying - definitely makes sense now.

I updated the patch with corrected tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js...

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8904113 [details]
Bug 1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central.

https://reviewboard.mozilla.org/r/175890/#review181168

Thank you for the update. Looks good. r=Standard8

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb3853161334
Enable the ESLint generator-star-spacing rule across mozilla-central. r=standard8

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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