Skip to content

Commit

Permalink
New: Add consistent and ..-as-needed to object-shorthand (fixes #5438) (
Browse files Browse the repository at this point in the history
  • Loading branch information
martijndeh authored and ilyavolodin committed Aug 8, 2016
1 parent 7e1bf01 commit 5ef839e
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 5 deletions.
43 changes: 43 additions & 0 deletions docs/rules/object-shorthand.md
Expand Up @@ -93,6 +93,8 @@ The rule takes an option which specifies when it should be applied. It can be se
* `"methods"` ensures the method shorthand is used (also applies to generators).
* `"properties` ensures the property shorthand is used (where the key and variable name match).
* `"never"` ensures that no property or method shorthand is used in any object literal.
* `"consistent"` ensures that either all shorthand or all longform will be used in an object literal.
* `"consistent-as-needed"` ensures that either all shorthand or all longform will be used in an object literal, but ensures all shorthand whenever possible.

You can set the option in configuration like this:

Expand Down Expand Up @@ -152,6 +154,47 @@ var foo = {
};
```

When set to `"consistent"` the following will warn:

```js
/*eslint object-shorthand: [2, "consistent"]*/
/*eslint-env es6*/

var foo = {
a,
b: "foo",
};
```

The following will *not* warn:

```js
/*eslint object-shorthand: [2, "consistent"]*/
/*eslint-env es6*/

var foo = {
a: a,
b: "foo"
};

var bar = {
a,
b,
};
```

When set to `"consistent-as-needed"`, which is very similar to `"consistent"`, the following will warn:

```js
/*eslint object-shorthand: [2, "consistent-as-needed"]*/
/*eslint-env es6*/

var foo = {
a: a,
b: b,
};
```

## When Not To Use It

Anyone not yet in an ES6 environment would not want to apply this rule. Others may find the terseness of the shorthand
Expand Down
91 changes: 89 additions & 2 deletions lib/rules/object-shorthand.js
Expand Up @@ -9,7 +9,9 @@ const OPTIONS = {
always: "always",
never: "never",
methods: "methods",
properties: "properties"
properties: "properties",
consistent: "consistent",
consistentAsNeeded: "consistent-as-needed"
};

//------------------------------------------------------------------------------
Expand All @@ -31,7 +33,7 @@ module.exports = {
type: "array",
items: [
{
enum: ["always", "methods", "properties", "never"]
enum: ["always", "methods", "properties", "never", "consistent", "consistent-as-needed"]
}
],
minItems: 0,
Expand Down Expand Up @@ -87,6 +89,8 @@ module.exports = {
const APPLY_TO_METHODS = APPLY === OPTIONS.methods || APPLY === OPTIONS.always;
const APPLY_TO_PROPS = APPLY === OPTIONS.properties || APPLY === OPTIONS.always;
const APPLY_NEVER = APPLY === OPTIONS.never;
const APPLY_CONSISTENT = APPLY === OPTIONS.consistent;
const APPLY_CONSISTENT_AS_NEEDED = APPLY === OPTIONS.consistentAsNeeded;

const PARAMS = context.options[1] || {};
const IGNORE_CONSTRUCTORS = PARAMS.ignoreConstructors;
Expand All @@ -108,6 +112,16 @@ module.exports = {
return firstChar === firstChar.toUpperCase();
}

/**
* Determines if the property is not a getter and a setter.
* @param {ASTNode} property Property AST node
* @returns {boolean} True if the property is not a getter and a setter, false if it is.
* @private
**/
function isNotGetterOrSetter(property) {
return (property.kind !== "set" && property.kind !== "get");
}

/**
* Checks whether a node is a string literal.
* @param {ASTNode} node - Any AST node.
Expand All @@ -117,11 +131,84 @@ module.exports = {
return node.type === "Literal" && typeof node.value === "string";
}

/**
* Determines if the property is a shorthand or not.
* @param {ASTNode} property Property AST node
* @returns {boolean} True if the property is considered shorthand, false if not.
* @private
**/
function isShorthand(property) {

// property.method is true when `{a(){}}`.
return (property.shorthand || property.method);
}

/**
* Determines if the property's key and method or value are named equally.
* @param {ASTNode} property Property AST node
* @returns {boolean} True if the key and value are named equally, false if not.
* @private
**/
function isRedudant(property) {
return (property.key && (

// A function expression
property.value && property.value.id && property.value.id.name === property.key.name ||

// A property
property.value && property.value.name === property.key.name
));
}

/**
* Ensures that an object's properties are consistently shorthand, or not shorthand at all.
* @param {ASTNode} node Property AST node
* @param {boolean} checkRedundancy Whether to check longform redundancy
* @returns {void}
**/
function checkConsistency(node, checkRedundancy) {

// We are excluding getters and setters as they are considered neither longform nor shorthand.
const properties = node.properties.filter(isNotGetterOrSetter);

// Do we still have properties left after filtering the getters and setters?
if (properties.length > 0) {
const shorthandProperties = properties.filter(isShorthand);

// If we do not have an equal number of longform properties as
// shorthand properties, we are using the annotations inconsistently
if (shorthandProperties.length !== properties.length) {

// We have at least 1 shorthand property
if (shorthandProperties.length > 0) {
context.report(node, "Unexpected mix of shorthand and non-shorthand properties.");
} else if (checkRedundancy) {

// If all properties of the object contain a method or value with a name matching it's key,
// all the keys are redudant.
const canAlwaysUseShorthand = properties.every(isRedudant);

if (canAlwaysUseShorthand) {
context.report(node, "Expected shorthand for all properties.");
}
}
}
}
}

//--------------------------------------------------------------------------
// Public
//--------------------------------------------------------------------------

return {
ObjectExpression: function(node) {
if (APPLY_CONSISTENT) {
checkConsistency(node, false);
} else if (APPLY_CONSISTENT_AS_NEEDED) {
checkConsistency(node, true);
}
},

Property: function(node) {
const isConciseProperty = node.method || node.shorthand;

Expand Down
24 changes: 21 additions & 3 deletions tests/lib/rules/object-shorthand.js
Expand Up @@ -93,7 +93,17 @@ ruleTester.run("object-shorthand", rule, {
{ code: "var x = {'y': y}", parserOptions: { ecmaVersion: 6 }, options: ["properties", {avoidQuotes: true}] },

// ignore object shorthand
{ code: "let {a, b} = o;", parserOptions: { ecmaVersion: 6 }, options: ["never"] }
{ code: "let {a, b} = o;", parserOptions: { ecmaVersion: 6 }, options: ["never"] },

// consistent
{ code: "var x = {a: a, b: b}", parserOptions: { ecmaVersion: 6}, options: ["consistent"] },
{ code: "var x = {a: b, c: d, f: g}", parserOptions: { ecmaVersion: 6}, options: ["consistent"] },
{ code: "var x = {a, b}", parserOptions: { ecmaVersion: 6}, options: ["consistent"] },
{ code: "var x = {a, b, get test() { return 1; }}", parserOptions: { ecmaVersion: 6}, options: ["consistent"] },

// consistent-as-needed
{ code: "var x = {a, b}", parserOptions: { ecmaVersion: 6}, options: ["consistent-as-needed"] },
{ code: "var x = {a, b, get test(){return 1;}}", parserOptions: { ecmaVersion: 6}, options: ["consistent-as-needed"] }
],
invalid: [
{ code: "var x = {x: x}", output: "var x = {x}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected property shorthand.", type: "Property" }] },
Expand Down Expand Up @@ -135,11 +145,19 @@ ruleTester.run("object-shorthand", rule, {
{ code: "var x = {ConstructorFunction(){}, a: b}", output: "var x = {ConstructorFunction: function(){}, a: b}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax.", type: "Property" }], options: ["never"] },
{ code: "var x = {notConstructorFunction(){}, b: c}", output: "var x = {notConstructorFunction: function(){}, b: c}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax.", type: "Property" }], options: ["never"] },

// // avoidQuotes
// avoidQuotes
{ code: "var x = {a: a}", output: "var x = {a}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected property shorthand.", type: "Property" }], options: ["always", {avoidQuotes: true}] },
{ code: "var x = {a: function(){}}", output: "var x = {a(){}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected method shorthand.", type: "Property" }], options: ["methods", {avoidQuotes: true}] },
{ code: "var x = {[a]: function(){}}", output: "var x = {[a](){}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected method shorthand.", type: "Property" }], options: ["methods", {avoidQuotes: true}] },
{ code: "var x = {'a'(){}}", output: "var x = {'a': function(){}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax for string literal keys.", type: "Property" }], options: ["always", {avoidQuotes: true}] },
{ code: "var x = {['a'](){}}", output: "var x = {['a']: function(){}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax for string literal keys.", type: "Property" }], options: ["methods", {avoidQuotes: true}] }
{ code: "var x = {['a'](){}}", output: "var x = {['a']: function(){}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax for string literal keys.", type: "Property" }], options: ["methods", {avoidQuotes: true}] },

// consistent
{ code: "var x = {a: a, b}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Unexpected mix of shorthand and non-shorthand properties.", type: "" }], options: ["consistent"] },
{ code: "var x = {b, c: d, f: g}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Unexpected mix of shorthand and non-shorthand properties.", type: "" }], options: ["consistent"] },

// consistent-as-needed
{ code: "var x = {a: a, b: b}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Expected shorthand for all properties.", type: "" }], options: ["consistent-as-needed"] },
{ code: "var x = {a, z: function z(){}}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Unexpected mix of shorthand and non-shorthand properties.", type: "" }], options: ["consistent-as-needed"] }
]
});

0 comments on commit 5ef839e

Please sign in to comment.