javascript icon indicating copy to clipboard operation
javascript copied to clipboard

Consider incorporating rules from eslint-plugin-unicorn

Open kripod opened this issue 8 years ago • 7 comments

By using code from eslint-plugin-unicorn, the following style rules could be enforced:

I also think that introducing new rules from the aforementioned plugin may help paving the way towards a more consistent coding style. For reference, see the rules below:

Avoid unexpected bugs

  • no-array-instanceof - Require Array.isArray() instead of instanceof Array. (fixable)
  • custom-error-definition - Enforce correct Error subclassing. (fixable)
  • no-fn-reference-in-iterator - Prevents passing a function reference directly to iterator methods. (fixable)

Enforce style consistency

  • no-abusive-eslint-disable - Enforce specifying rules to disable in eslint-disable comments.
  • throw-new-error - Require new when throwing an error. (fixable)
  • number-literal-case - Enforce lowercase identifier and uppercase value for number literals. (fixable)
  • escape-case - Require escape sequences to use uppercase values. (fixable)
  • no-hex-escape - Enforce the use of unicode escapes instead of hexadecimal escapes. (fixable)
  • prefer-starts-ends-with - Prefer String#startsWith & String#endsWith over more complex alternatives.
  • prefer-type-error - Enforce throwing TypeError in type checking conditions. (fixable)
  • import-index - Enforce importing index files with .. (fixable)
  • new-for-builtins - Enforce the use of new for all builtins, except String, Number and Boolean. (fixable)

Warn about API deprecation

  • no-new-buffer - Enforce the use of Buffer.from() and Buffer.alloc() instead of the deprecated new Buffer(). (fixable)

kripod avatar Aug 24 '17 19:08 kripod

Due to the nature of eslint shared configs, it's a pretty large burden on users to add a new peer dependency (ie, a new plugin).

Most of these rules do seem useful to us - import-index tho, we definitely prefer ./ over ., so that rule wouldn't work for us.

Does https://eslint.org/docs/rules/no-buffer-constructor not cover no-new-buffer?

ljharb avatar Aug 25 '17 22:08 ljharb

It seems that https://eslint.org/docs/rules/no-buffer-constructor covers no-new-buffer, so there is no need for the latter. If it isn't possible or convenient to add eslint-plugin-unicorn as a peer dependency, then I suggest asking for permission to copy the rules with attribution.

kripod avatar Aug 26 '17 13:08 kripod

They'd have to be copied into one of our existing plugins, which are only -import, -react, and -jsx-a11y; besides, copying code like that wouldn't be an improvement over adding a peer dep.

ljharb avatar Aug 26 '17 16:08 ljharb

I see your point, adding unicorn as a peer dependency would clutter the package.json files of developers, but the plugin doesn't really fit into any of -import, -react or -jsx-a11y. I'll leave this issue open and would be happy to see more discussion about it.

kripod avatar Aug 26 '17 21:08 kripod

another solution is to add at least references to those rules

tunnckoCore avatar Oct 07 '17 23:10 tunnckoCore

Advertising other plugins that aren't enabled in our config wouldn't make much sense.

ljharb avatar Oct 08 '17 00:10 ljharb

I'm using the following subset of rules from Unicorn as of today:

const { rules: baseES6Rules } = require('eslint-config-airbnb-base/rules/es6');

module.exports = {
  extends: 'plugin:unicorn/recommended',

  rules: {
    'unicorn/import-index': 'off',
    'unicorn/prevent-abbreviations': 'off',

    'unicorn/custom-error-definition': 'error',
    'unicorn/explicit-length-check': ['error', { 'non-zero': 'greater-than' }],
    'unicorn/no-fn-reference-in-iterator': 'error',
    'unicorn/no-unsafe-regex': 'error',
    'unicorn/no-unused-properties': 'error',

    // A base filename should exactly match the name of its default export
    // See: https://github.com/airbnb/javascript#naming--filename-matches-export
    'unicorn/filename-case': [
      'error',
      { cases: { camelCase: true, pascalCase: true, kebabCase: true } },
    ],

    // Improve compatibility with `unicorn/no-unreadable-array-destructuring`
    // See: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/master/docs/rules/no-unreadable-array-destructuring.md#note
    'prefer-destructuring': [
      'error',
      {
        ...baseES6Rules['prefer-destructuring'][1],
        VariableDeclarator: {
          ...baseES6Rules['prefer-destructuring'][1].VariableDeclarator,
          array: false,
        },
        AssignmentExpression: {
          ...baseES6Rules['prefer-destructuring'][1].AssignmentExpression,
          array: false,
        },
      },
    ],
  },
};

They seem to extend the base config's functionality in a nice manner.

kripod avatar Oct 29 '19 13:10 kripod