ESLint Plugin: no-unused-vars-before-return: Exempt destructuring onl…

…y 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

@@ -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

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.' } ],

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
 						);

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 &&