tools: some eslint rules proposals

[vsemozhetbyt]

Recently some big PRs with churn was mostly rejected or gave sporadic -1, so these are some preliminarily proposals to check the opinions before investing time.

1. Rule: http://eslint.org/docs/rules/no-unused-vars

   Proposal:

   no-unused-vars: ['error', {args: 'after-used'}]
   

   Hits:

   *.md:          190 (soundly disabled in doc/.eslintrc.yaml)
   
   *.js benchmark: 33
   *.js lib:       45
   *.js test:     300
   *.js tools:      1 (will be fixed in #14251)
   

   Refs: tools: delete an unused argument #14251

2. Rule: http://eslint.org/docs/rules/prefer-arrow-callback

   Proposal:

   `prefer-arrow-callback: ['error', {allowNamedFunctions: false, allowUnboundThis: true}]`
   

   Hits:

   *.md:            15
   
   *.js benchmark: 108
   *.js lib:       114
   *.js test:     4002 (wow...)
   *.js tools:      51
   

3. Rule: http://eslint.org/docs/rules/prefer-destructuring

   Proposal:

   prefer-destructuring: ['error',
                          {array: true, object: true},
                          {enforceForRenamedProperties: false}]
   

   Hits:

   array: *.md:           3
   
   array: *.js benchmark: 7
   array: *.js lib:      59
   array: *.js test:    124
   array: *.js tools:     9
   
   
   object: *.md:             1
   
   object: *.js benchmark: 141
   object: *.js lib:       268
   object: *.js test:      562
   object: *.js tools:      22
   

If there is some consensus for any proposal, it can be used as a starting point for code-and-learn or similar events.

If some or all approved proposals are not appropriate for code-and-learn, I can do any or all if there are volunteers to review.

[Trott]

Ref: #10129

[Trott]

no-unused-vars: ['error', {args: 'after-used'}]

FWIW, {args: 'after-used'} is the default, so it can just be: no-unused-vars: error.

[vsemozhetbyt]

@Trott Yes, I've used some defaults to be more explicit. Some rules can be shortened.

[silverwind]

+1 to no-unused-vars, but I'd suggest these options:

{args: all, argsIgnorePattern: ^_, varsIgnorePattern: ^_}

This way, one can just underscore-prefix unused variabled in function arguments or during destructuring:

doSomething((_unused, used) => {});
const [_unused, used] = doSomething();

[vsemozhetbyt]

@silverwind With varsIgnorePattern we can miss some unused "internal" variables outside of destructuring, while in destructuring we can simply omit the unused variables:

const [, , file] = process.argv;

But I am OK with this decision if there are some +1.

[sebdeckers]

Any chance of adopting StandardJS? Avoids lots of stylistic bikeshedding. Many projects use it nowadays. As one example, migration was relatively painless for NPM (npm/npm#9954). The auto-formatter has improved since then, though it may not be perfect.

[vsemozhetbyt]

@sebdeckers I doubt we will have consensus about "no semicolons" or "space after function name" :) The churn will be tremendous :)

[vsemozhetbyt]

It seems this has not got enough approvement and can be closed for now. Feel free to reopen if something changes.

[refack]

+1 to no-unused-vars, but I'd suggest these options:

{args: all, argsIgnorePattern: ^, varsIgnorePattern: ^}

@vsemozhetbyt I think no-unused-vars on it's own will get wider support (personally I like all with an ^_+$ ignore pattern)