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
martijndeh committed Feb 29, 2016
1 parent 4a7f991 commit e341f1f
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 4 deletions.
50 changes: 50 additions & 0 deletions docs/rules/object-shorthand.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,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 @@ -117,6 +119,54 @@ 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'
};
```

The following will also *not* warn:

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

var foo = {
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
87 changes: 85 additions & 2 deletions lib/rules/object-shorthand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ var OPTIONS = {
always: "always",
never: "never",
methods: "methods",
properties: "properties"
properties: "properties",
consistent: "consistent",
consistentAsNeeded: "consistent-as-needed"
};

//------------------------------------------------------------------------------
Expand All @@ -21,6 +23,8 @@ module.exports = function(context) {
var APPLY_TO_METHODS = APPLY === OPTIONS.methods || APPLY === OPTIONS.always;
var APPLY_TO_PROPS = APPLY === OPTIONS.properties || APPLY === OPTIONS.always;
var APPLY_NEVER = APPLY === OPTIONS.never;
var APPLY_CONSISTENT = APPLY === OPTIONS.consistent;
var APPLY_CONSISTENT_AS_NEEDED = APPLY === OPTIONS.consistentAsNeeded;

var PARAMS = context.options[1] || {};
var IGNORE_CONSTRUCTORS = PARAMS.ignoreConstructors;
Expand All @@ -41,6 +45,77 @@ module.exports = function(context) {
return firstChar === firstChar.toUpperCase();
}

/**
* 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 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");
}

/**
* 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.
var properties = node.properties.filter(isNotGetterOrSetter),
shorthandProperties,
canAlwaysUseShorthand;

// Do we still have properties left after filtering the getters and setters?
if (properties.length > 0) {
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, "Inconsistently using longform and shortform syntax.");
} 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.
canAlwaysUseShorthand = properties.every(isRedudant);

if (canAlwaysUseShorthand) {
context.report(node, "Properties shouldn't be in longform as all keys are redundant.");
}
}
}
}
}

//--------------------------------------------------------------------------
// Public
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -82,6 +157,14 @@ module.exports = function(context) {
// {"x": x} should be written as {x}
context.report(node, "Expected property shorthand.");
}
},

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

Expand All @@ -93,7 +176,7 @@ module.exports.schema = {
"type": "array",
"items": [
{
"enum": ["always", "methods", "properties", "never"]
"enum": ["always", "methods", "properties", "never", "consistent", "consistent-as-needed"]
}
],
"minItems": 0,
Expand Down
22 changes: 20 additions & 2 deletions tests/lib/rules/object-shorthand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,17 @@ ruleTester.run("object-shorthand", rule, {
{ code: "var x = {ConstructorFunction: function(){}, a: b}", parserOptions: { ecmaVersion: 6 }, options: ["methods", { "ignoreConstructors": true }] },
{ code: "var x = {notConstructorFunction(){}, b: c}", parserOptions: { ecmaVersion: 6 }, options: ["methods", { "ignoreConstructors": true }] },
{ code: "var x = {ConstructorFunction: function(){}, a: b}", parserOptions: { ecmaVersion: 6 }, options: ["never"] },
{ code: "var x = {notConstructorFunction: function(){}, b: c}", parserOptions: { ecmaVersion: 6 }, options: ["never"] }
{ code: "var x = {notConstructorFunction: function(){}, b: c}", 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}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected property shorthand.", type: "Property" }] },
Expand Down Expand Up @@ -109,6 +119,14 @@ ruleTester.run("object-shorthand", rule, {
{ code: "var x = {y, a: b, *x(){}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform property syntax.", type: "Property" }, { message: "Expected longform method syntax.", type: "Property" }], options: ["never"]},
{ code: "var x = {y: {x}}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform property syntax.", type: "Property" }], options: ["never"]},
{ code: "var x = {ConstructorFunction(){}, a: b}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax.", type: "Property" }], options: ["never"] },
{ code: "var x = {notConstructorFunction(){}, b: c}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax.", type: "Property" }], options: ["never"] }
{ code: "var x = {notConstructorFunction(){}, b: c}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected longform method syntax.", type: "Property" }], options: ["never"] },

// consistent
{ code: "var x = {a: a, b}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Inconsistently using longform and shortform syntax.", type: "" }], options: ["consistent"] },
{ code: "var x = {b, c: d, f: g}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Inconsistently using longform and shortform syntax.", type: "" }], options: ["consistent"] },

// consistent-as-needed
{ code: "var x = {a: a, b: b}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Properties shouldn't be in longform as all keys are redundant.", type: "" }], options: ["consistent-as-needed"] },
{ code: "var x = {a, z: function z(){}}", parserOptions: { ecmaVersion: 6}, errors: [{ message: "Inconsistently using longform and shortform syntax.", type: "" }], options: ["consistent-as-needed"] }
]
});

0 comments on commit e341f1f

Please sign in to comment.