Skip to content

Commit

Permalink
Update: support more options in prefer-destructuring (#8796)
Browse files Browse the repository at this point in the history 
* Update: support more options in prefer-destructuring (fixes #7886)

* fix up

* add comment to doc
  • Loading branch information
@VictorHom @ilyavolodin
VictorHom authored and ilyavolodin committed Jul 21, 2017
1 parent 3bebcfd commit 91dccdf
Show file tree
Hide file tree
Showing 3 changed files with 306 additions and 48 deletions.
93 changes: 84 additions & 9 deletions docs/rules/prefer-destructuring.md
View file
Expand Up @@ -4,6 +4,18 @@ With JavaScript ES6, a new syntax was added for creating variables from an array

## Rule Details

### Options

This rule takes two sets of configuration objects. The first object parameter determines what types of destructuring the rule applies to.

The two properties, `array` and `object`, can be used to turn on or off the destructuring requirement for each of those types independently. By default, both are true.

Alternatively, you can use separate configurations for different assignment types. It accepts 2 other keys instead of `array` and `object`.

One key is `VariableDeclarator` and the other is `AssignmentExpression`, which can be used to control the destructuring requirement for each of those types independently. Each property accepts an object that accepts two properties, `array` and `object`, which can be used to control the destructuring requirement for each of `array` and `object` independently for variable declarations and assignment expressions. By default, `array` and `object` are set to true for both `VariableDeclarator` and `AssignmentExpression`.

The rule has a second object with a single key, `enforceForRenamedProperties`, which determines whether the `object` destructuring applies to renamed variables.

Examples of **incorrect** code for this rule:

```javascript
Expand All @@ -27,14 +39,6 @@ var { foo } = object;
var foo = object.bar;
```

### Options

This rule takes two sets of configuration objects; the first controls the types that the rule is applied to, and the second controls the way those objects are evaluated.

The first has two properties, `array` and `object`, which can be used to turn on or off the destructuring requirement for each of those types independently. By default, both are `true`.

The second has a single property, `enforceForRenamedProperties`, that controls whether or not the `object` destructuring rules are applied in cases where the variable requires the property being access to be renamed.

Examples of **incorrect** code when `enforceForRenamedProperties` is enabled:

```javascript
Expand All @@ -47,7 +51,7 @@ Examples of **correct** code when `enforceForRenamedProperties` is enabled:
var { bar: foo } = object;
```

An example configuration, with the defaults filled in, looks like this:
An example configuration, with the defaults `array` and `object` filled in, looks like this:

```json
{
Expand All @@ -62,6 +66,77 @@ An example configuration, with the defaults filled in, looks like this:
}
```

The two properties, `array` and `object`, which can be used to turn on or off the destructuring requirement for each of those types independently. By default, both are true.

For example, the following configuration enforces only object destructuring, but not array destructuring:

```json
{
"rules": {
"prefer-destructuring": ["error", {"object": true, "array": false}]
}
}
```

An example configuration, with the defaults `VariableDeclarator` and `AssignmentExpression` filled in, looks like this:

```json
{
"rules": {
"prefer-destructuring": ["error", {
"VariableDeclarator": {
"array": false,
"object": true
},
"AssignmentExpression": {
"array": true,
"object": true
}
}, {
"enforceForRenamedProperties": false
}]
}
}
```

The two properties, `VariableDeclarator` and `AssignmentExpression`, which can be used to turn on or off the destructuring requirement for `array` and `object`. By default, all values are true.

For example, the following configuration enforces object destructuring in variable declarations and enforces array destructuring in assignment expressions.

```json
{
"rules": {
"prefer-destructuring": ["error", {
"VariableDeclarator": {
"array": false,
"object": true
},
"AssignmentExpression": {
"array": true,
"object": false
}
}, {
"enforceForRenamedProperties": false
}]
}
}

```

Examples of **correct** code when object destructuring in `VariableDeclarator` is enforced:

```javascript
/* eslint prefer-destructuring: ["error", {VariableDeclarator: {object: true}}] */
var {bar: foo} = object;
```

Examples of **correct** code when array destructuring in `AssignmentExpression` is enforced:

```javascript
/* eslint prefer-destructuring: ["error", {AssignmentExpression: {array: true}}] */
[bar] = array;
```

## When Not To Use It

If you want to be able to access array indices or object properties directly, you can either configure the rule to your tastes or disable the rule entirely.
Expand Down
102 changes: 70 additions & 32 deletions lib/rules/prefer-destructuring.js
View file
Expand Up @@ -15,19 +15,55 @@ module.exports = {
category: "ECMAScript 6",
recommended: false
},

schema: [
{
type: "object",
properties: {
array: {
type: "boolean"

// old support {array: Boolean, object: Boolean}
// new support {VariableDeclarator: {}, AssignmentExpression: {}}
oneOf: [
{
type: "object",
properties: {
VariableDeclarator: {
type: "object",
properties: {
array: {
type: "boolean"
},
object: {
type: "boolean"
}
},
additionalProperties: false
},
AssignmentExpression: {
type: "object",
properties: {
array: {
type: "boolean"
},
object: {
type: "boolean"
}
},
additionalProperties: false
}
},
additionalProperties: false
},
object: {
type: "boolean"
{
type: "object",
properties: {
array: {
type: "boolean"
},
object: {
type: "boolean"
}
},
additionalProperties: false
}
},
additionalProperties: false
]
},
{
type: "object",
Expand All @@ -42,34 +78,36 @@ module.exports = {
},
create(context) {

let checkArrays = true;
let checkObjects = true;
let enforceForRenamedProperties = false;
const enabledTypes = context.options[0];
const additionalOptions = context.options[1];
const enforceForRenamedProperties = context.options[1] && context.options[1].enforceForRenamedProperties;
let normalizedOptions = {
VariableDeclarator: { array: true, object: true },
AssignmentExpression: { array: true, object: true }
};

if (enabledTypes) {
if (typeof enabledTypes.array !== "undefined") {
checkArrays = enabledTypes.array;
}

if (typeof enabledTypes.object !== "undefined") {
checkObjects = enabledTypes.object;
}
}

if (additionalOptions) {
if (typeof additionalOptions.enforceForRenamedProperties !== "undefined") {
enforceForRenamedProperties = additionalOptions.enforceForRenamedProperties;
}
normalizedOptions = typeof enabledTypes.array !== "undefined" || typeof enabledTypes.object !== "undefined"
? { VariableDeclarator: enabledTypes, AssignmentExpression: enabledTypes }
: enabledTypes;
}

//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------

/**
* Determines if the given node node is accessing an array index
* @param {string} nodeType "AssignmentExpression" or "VariableDeclarator"
* @param {string} destructuringType "array" or "object"
* @returns {boolean} `true` if the destructuring type should be checked for the given node
*/
function shouldCheck(nodeType, destructuringType) {
return normalizedOptions &&
normalizedOptions[nodeType] &&
normalizedOptions[nodeType][destructuringType];
}

/**
* Determines if the given node is accessing an array index
*
* This is used to differentiate array index access from object property
* access.
Expand Down Expand Up @@ -110,22 +148,22 @@ module.exports = {
}

if (isArrayIndexAccess(rightNode)) {
if (checkArrays) {
if (shouldCheck(reportNode.type, "array")) {
report(reportNode, "array");
}
return;
}

if (checkObjects && enforceForRenamedProperties) {
if (shouldCheck(reportNode.type, "object") && enforceForRenamedProperties) {
report(reportNode, "object");
return;
}

if (checkObjects) {
if (shouldCheck(reportNode.type, "object")) {
const property = rightNode.property;

if ((property.type === "Literal" && leftNode.name === property.value) ||
(property.type === "Identifier" && leftNode.name === property.name)) {
if ((property.type === "Literal" && leftNode.name === property.value) || (property.type === "Identifier" &&
leftNode.name === property.name)) {
report(reportNode, "object");
}
}
Expand Down

0 comments on commit 91dccdf

Please sign in to comment.