Permalink
Browse files

New: `prefer-destructuring` rule (fixes #6053) (#7741)

* New: `prefer-destructuring` rule (fixes #6053)

* Address general, easy PR feedback

* Remove checks covered by `object-shorthand`

* Add support for AssignmentExpression in `prefer-destructuring`

* Add `requireRenaming` option to the `prefer-destructuring` rule

* More PR feedback

* Add missing semicolon to code example in docs

* Update documentation
  • Loading branch information...
1 parent bb648ce commit 27424cb7b470dd8473e2885c01c556daedb985f9 @alexlafroscia alexlafroscia committed with not-an-aardvark Dec 16, 2016
Showing with 397 additions and 0 deletions.
  1. +1 −0 conf/eslint.json
  2. +83 −0 docs/rules/prefer-destructuring.md
  3. +173 −0 lib/rules/prefer-destructuring.js
  4. +140 −0 tests/lib/rules/prefer-destructuring.js
View
@@ -204,6 +204,7 @@
"padded-blocks": "off",
"prefer-arrow-callback": "off",
"prefer-const": "off",
+ "prefer-destructuring": "off",
"prefer-numeric-literals": "off",
"prefer-reflect": "off",
"prefer-rest-params": "off",
@@ -0,0 +1,83 @@
+# Prefer destructuring from arrays and objects (prefer-destructuring)
+
+With JavaScript ES6, a new syntax was added for creating variables from an array index or object property, called [destructuring](#further-reading). This rule enforces usage of destructuring instead of accessing a property through a member expression.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+// With `array` enabled
+var foo = array[0];
+
+// With `object` enabled
+var foo = object.foo;
+var foo = object['foo'];
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+// With `array` enabled
+var [ foo ] = array;
+var foo = array[someIndex];
+
+// With `object` enabled
+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
+var foo = object.bar;
+```
+
+Examples of **correct** code when `enforceForRenamedProperties` is enabled:
+
+```javascript
+var { bar: foo } = object;
+```
+
+An example configuration, with the defaults filled in, looks like this:
+
+```json
+{
+ "rules": {
+ "prefer-destructuring": ["error", {
+ "array": true,
+ "object": true
+ }, {
+ "enforceForRenamedProperties": false
+ }]
+ }
+}
+```
+
+## 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.
+
+Additionally, if you intend to access large array indices directly, like:
+
+```javascript
+var foo = array[100];
+```
+
+Then the `array` part of this rule is not recommended, as destructuring does not match this use case very well.
+
+
+## Further Reading
+
+If you want to learn more about destructuring, check out the links below:
+
+- [Destructuring Assignment (MDN)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment)
+- [Destructuring and parameter handling in ECMAScript 6 (2ality blog)](http://www.2ality.com/2015/01/es6-destructuring.html)
@@ -0,0 +1,173 @@
+/**
+ * @fileoverview Prefer destructuring from arrays and objects
+ * @author Alex LaFroscia
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+ meta: {
+ docs: {
+ description: "require destructuring from arrays and/or objects",
+ category: "ECMAScript 6",
+ recommended: false
+ },
+
+ schema: [
+ {
+ type: "object",
+ properties: {
+ array: {
+ type: "boolean"
+ },
+ object: {
+ type: "boolean"
+ }
+ },
+ additionalProperties: false
+ },
+ {
+ type: "object",
+ properties: {
+ enforceForRenamedProperties: {
+ type: "boolean"
+ }
+ },
+ additionalProperties: false
+ }
+ ]
+ },
+ create(context) {
+
+ let checkArrays = true;
+ let checkObjects = true;
+ let enforceForRenamedProperties = false;
+ const enabledTypes = context.options[0];
+ const additionalOptions = context.options[1];
+
+ 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;
+ }
+ }
+
+ //--------------------------------------------------------------------------
+ // Helpers
+ //--------------------------------------------------------------------------
+
+ /**
+ * Determines if the given node node is accessing an array index
+ *
+ * This is used to differentiate array index access from object property
+ * access.
+ *
+ * @param {ASTNode} node the node to evaluate
+ * @returns {boolean} whether or not the node is an integer
+ */
+ function isArrayIndexAccess(node) {
+ return Number.isInteger(node.property.value);
+ }
+
+ /**
+ * Report that the given node should use destructuring
+ *
+ * @param {ASTNode} reportNode the node to report
+ * @param {string} type the type of destructuring that should have been done
+ * @returns {void}
+ */
+ function report(reportNode, type) {
+ context.report({ node: reportNode, message: `Use ${type} destructuring` });
+ }
+
+ /**
+ * Check that the `prefer-destructuring` rules are followed based on the
+ * given left- and right-hand side of the assignment.
+ *
+ * Pulled out into a separate method so that VariableDeclarators and
+ * AssignmentExpressions can share the same verification logic.
+ *
+ * @param {ASTNode} leftNode the left-hand side of the assignment
+ * @param {ASTNode} rightNode the right-hand side of the assignment
+ * @param {ASTNode} reportNode the node to report the error on
+ * @returns {void}
+ */
+ function performCheck(leftNode, rightNode, reportNode) {
+ if (rightNode.type !== "MemberExpression") {
+ return;
+ }
+
+ if (checkArrays && isArrayIndexAccess(rightNode)) {
+ report(reportNode, "array");
+ return;
+ }
+
+ if (checkObjects && enforceForRenamedProperties) {
+ report(reportNode, "object");
+ return;
+ }
+
+ if (checkObjects) {
+ const property = rightNode.property;
+
+ if ((property.type === "Literal" && leftNode.name === property.value) ||
+ (property.type === "Identifier" && leftNode.name === property.name)) {
+ report(reportNode, "object");
+ }
+ }
+ }
+
+ /**
+ * Check if a given variable declarator is coming from an property access
+ * that should be using destructuring instead
+ *
+ * @param {ASTNode} node the variable declarator to check
+ * @returns {void}
+ */
+ function checkVariableDeclarator(node) {
+
+ // Skip if variable is declared without assignment
+ if (!node.init) {
+ return;
+ }
+
+ // We only care about member expressions past this point
+ if (node.init.type !== "MemberExpression") {
+ return;
+ }
+
+ performCheck(node.id, node.init, node);
+ }
+
+ /**
+ * Run the `prefer-destructuring` check on an AssignmentExpression
+ *
+ * @param {ASTNode} node the AssignmentExpression node
+ * @returns {void}
+ */
+ function checkAssigmentExpression(node) {
+ performCheck(node.left, node.right, node);
+ }
+
+ //--------------------------------------------------------------------------
+ // Public
+ //--------------------------------------------------------------------------
+
+ return {
+ VariableDeclarator: checkVariableDeclarator,
+ AssignmentExpression: checkAssigmentExpression
+ };
+ }
+};
Oops, something went wrong.

0 comments on commit 27424cb

Please sign in to comment.