Framework: Refine ESLint configuration

[aduth]

This pull request seeks to reconcile our ESLint configuration to make use of new rules and new options for existing rules. None of these changes are meant to be controversial (though I was tempted to raise a few 😄 ). The configuration file was also sorted, so I've included a detailed list of changes and reasoning below:

• New: comma-style (2)
  • Enforces end-of-line commas, conventionally used in Calypso
  • Follow-up: Document in coding guidelines
• New: constructor-super (2)
  • Enforces super call in ES6 class constructor
  • Coding error
• New: curly (1)
  • Requires all blocks to use curly braces
  • Guideline: "if/else/for/while/try blocks should always use braces, and always go on multiple lines."
• New: max-len (1)
  • Warns when line length exceeds 100 characters
  • Guideline: "Lines should usually be no longer than 80 characters, and should not exceed 100 (counting tabs as 4 spaces). This is a “soft” rule, but long lines generally indicate unreadable or disorganized code."
• New: no-const-assign (2)
  • Prevents reassignment of const variables
  • Coding error
• New: no-console (1)
  • Warns when using console
  • Okay to use while debugging, but shouldn't be committed. Intentional usage should disable rule.
  • Follow-up: Change to error, disable rule for existing usage
• New: no-debugger (2)
  • Prevents usage of debugger;
  • Okay to leave in while debugging, but shouldn't be committed.
• New: no-dupe-args (2)
  • Prevents two function arguments with same name
  • Coding error
• New: no-duplicate-case (2)
  • Prevents two case in a switch with the same name
  • Coding error
  • cc @enejb There's an instance of this in plugins/notices.jsx and I'm unsure which of the two cases is preferred
• New: no-negated-in-lhs (2)
  • Prevents ! (not) in the left hand of an in expression without parantheses
  • Almost always unintentional behavior (! is evaluated, i.e. cast to boolean, before in check)
  • Resolved two instances of this in aa1aac4
• New: no-unreachable (1)
  • Warns against unreachable code
  • Follow-up: Change to error, fix remaining issues (most are break; following a return in a switch)
• New: react/no-is-mounted (1)
  • Warns against use of this.isMounted() in a React component
  • Deprecated anti-pattern
• Updated: space-unary-ops (1)
  • Previously disabled because of inability to configure allowing !!, ++, --
• Updated: no-empty (2)
  • Upgraded to error, adding exception for catch

Possible rules to consider as separate pull requests that I was concerned may be too controversial:

• arrow-parens: Require parentheses around arrow function parameters
• react/prefer-es6-class: Prevent usage of React.createClass
• react/prop-types: Require propTypes for React components
• react/jsx-indent: Enforce consistent indentation

And another that's not yet available, but we should keep our eyes on, is react/unused-prop-types.

[enejb]

@aduth Can we combine the 2 messages?
So it reads return i18n.translate( 'Incompatible Archive. The package could not be installed.' );

I tried to commit the change but ran into following errors.

wp-calypso/node_modules/eslint/lib/config/config-validator.js:102
        throw new Error(message.join(""));
        ^
Error: wp-calypso/.eslintrc.js:
    Configuration for rule "max-len" is invalid:
    Value "[object Object]" is the wrong type.

[aduth]

@enejb Looks like error might be caused by outdated dependencies after our recent upgrades (#6116, #6090). Can you try running npm ls -g eslint? If any results appear, indicates that you have ESLint installed globally and should update them to match the versions in our package.json:

npm i -g eslint@2.13.1 eslint-plugin-react@5.2.2 eslint-plugin-wpcalypso@1.3.2

[aduth]

Or, might just be that you switched from a branch where the older version of ESLint was installed, in which case a simple npm install before committing might clear it up.

[enejb]

npm install worked for me thanks!

[folletto]

Require parentheses around arrow function parameters

I cast a +1 vote here :)

Also I'd be fairly on board on all the controversial ones.

[belcherj]

I am very much in favor of all of the listed rules. Voted on first comment via emoticon.

[nylen]

The changes here look good to me, but they do introduce a bunch of new warnings (11466 in master; 17927 on this branch). It would be good to have some follow-up PRs to clean up existing lint violations.

[gziolo]

+1 to all changes. I'm also fine with adding the controversial ones ;)

[aduth]

Thanks all for taking a look. There's a handful of follow-up tasks here so I'll try to tackle them soon or, more likely, create issues so they're at least tracked.