From 1254c2700d470745a208d20f95e89252aca46215 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew@andrewduthie.com>
Date: Wed, 31 Jul 2019 08:38:03 -0400
Subject: [PATCH] ESLint Plugin: no-unused-vars-before-return: Exempt
 destructuring only if to multiple properties (#16799)

* ESLint Plugin: Destructure references immediatley prior to use

* ESLint Plugin: Exempt object destructuring only if destructuring to more than one property
---
 packages/eslint-plugin/CHANGELOG.md           |  6 +++++
 .../__tests__/no-unused-vars-before-return.js | 23 +++++++++++++++++++
 .../rules/no-unused-vars-before-return.js     | 17 +++++++++++++-
 .../rules/react-no-unsafe-timeout.js          |  3 +--
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 61a4a84c98e74..f2c32ff387823 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -1,3 +1,9 @@
+## Master
+
+### Breaking Changes
+
+- The [`@wordpress/no-unused-vars-before-return` rule](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars-before-return.md) has been improved to exempt object destructuring only if destructuring to more than one property.
+
 ## 2.4.0 (2019-08-05)
 
 ### New Features
diff --git a/packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js b/packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js
index ae00f9cffe814..c8fa8c4a216ae 100644
--- a/packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js
+++ b/packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js
@@ -24,6 +24,17 @@ function example( number ) {
 	}
 
 	const foo = doSomeCostlyOperation();
+	return number + foo;
+}`,
+		},
+		{
+			code: `
+function example() {
+	const { foo, bar } = doSomeCostlyOperation();
+	if ( number > 10 ) {
+		return number + bar + 1;
+	}
+
 	return number + foo;
 }`,
 		},
@@ -49,6 +60,18 @@ function example( number ) {
 		return number + 1;
 	}
 
+	return number + foo;
+}`,
+			errors: [ { message: 'Variables should not be assigned until just prior its first reference. An early return statement may leave this variable unused.' } ],
+		},
+		{
+			code: `
+function example() {
+	const { foo } = doSomeCostlyOperation();
+	if ( number > 10 ) {
+		return number + 1;
+	}
+
 	return number + foo;
 }`,
 			errors: [ { message: 'Variables should not be assigned until just prior its first reference. An early return statement may leave this variable unused.' } ],
diff --git a/packages/eslint-plugin/rules/no-unused-vars-before-return.js b/packages/eslint-plugin/rules/no-unused-vars-before-return.js
index b2951c10f3bfa..fe8f20f4ded76 100644
--- a/packages/eslint-plugin/rules/no-unused-vars-before-return.js
+++ b/packages/eslint-plugin/rules/no-unused-vars-before-return.js
@@ -17,6 +17,21 @@ module.exports = {
 		const options = context.options[ 0 ] || {};
 		const { excludePattern } = options;
 
+		/**
+		 * Given an Espree VariableDeclarator node, returns true if the node
+		 * can be exempted from consideration as unused, or false otherwise. A
+		 * node can be exempt if it destructures to multiple variables, since
+		 * those other variables may be used prior to the return statement. A
+		 * future enhancement could validate that they are in-fact referenced.
+		 *
+		 * @param {Object} node Node to test.
+		 *
+		 * @return {boolean} Whether declarator is emempt from consideration.
+		 */
+		function isExemptObjectDestructureDeclarator( node ) {
+			return node.id.type === 'ObjectPattern' && node.id.properties.length > 1;
+		}
+
 		return {
 			ReturnStatement( node ) {
 				let functionScope = context.getScope();
@@ -37,7 +52,7 @@ module.exports = {
 							// Target function calls as "expensive".
 							def.node.init.type === 'CallExpression' &&
 							// Allow unused if part of an object destructuring.
-							def.node.id.type !== 'ObjectPattern' &&
+							! isExemptObjectDestructureDeclarator( def.node ) &&
 							// Only target assignments preceding `return`.
 							def.node.end < node.end
 						);
diff --git a/packages/eslint-plugin/rules/react-no-unsafe-timeout.js b/packages/eslint-plugin/rules/react-no-unsafe-timeout.js
index 64ff537199caa..f97a4d7384fe3 100644
--- a/packages/eslint-plugin/rules/react-no-unsafe-timeout.js
+++ b/packages/eslint-plugin/rules/react-no-unsafe-timeout.js
@@ -44,8 +44,6 @@ module.exports = {
 	create( context ) {
 		return {
 			'CallExpression[callee.name="setTimeout"]'( node ) {
-				const { references } = context.getScope();
-
 				// If the result of a `setTimeout` call is assigned to a
 				// variable, assume the timer ID is handled by a cancellation.
 				const hasAssignment = (
@@ -74,6 +72,7 @@ module.exports = {
 				// Consider whether `setTimeout` is a reference to the global
 				// by checking references to see if `setTimeout` resolves to a
 				// variable in scope.
+				const { references } = context.getScope();
 				const hasResolvedReference = references.some( ( reference ) => (
 					reference.identifier.name === 'setTimeout' &&
 					!! reference.resolved &&