consistent-function-scoping: Fix inconsistent behavior (#2748)

Author: SashankBhamidi

docs/rules/consistent-function-scoping.md

@@ -20,6 +20,24 @@ export function doFoo(foo) {
 	return doBar;
 }
 
+function doFoo(foo) {
+	const doBar = bar => {
+		return bar === 'bar';
+	};
+}
+
+function doFoo() {
+	// Does not capture anything from the scope, can be moved to the outer scope
+	return bar => bar === 'bar';
+}
+
+// Arrow functions in return statements are now also flagged
+export function someAction() {
+	return dispatch => dispatch({type: 'SOME_TYPE'});
+}
+```
+
+```js
 // ✅
 function doBar(bar) {
 	return bar === 'bar';
@@ -55,6 +73,12 @@ export function doFoo(foo) {
 
 	return doBar;
 }
+
+const doBar = bar => bar === 'bar';
+
+export function doFoo() {
+	return doBar;
+}
 ```
 
 ## Options

rules/consistent-function-scoping.js

@@ -108,6 +108,43 @@ const isIife = node =>
 	&& node.parent.type === 'CallExpression'
 	&& node.parent.callee === node;
 
+// Helper to walk up the chain to find the first non-arrow ancestor
+function getNonArrowAncestor(node) {
+	let ancestor = node;
+	while (ancestor && ancestor.type === 'ArrowFunctionExpression') {
+		ancestor = ancestor.parent;
+	}
+
+	return ancestor;
+}
+
+// Helper to skip over a chain of ArrowFunctionExpression nodes
+function skipArrowFunctionChain(node) {
+	let current = node;
+	while (current.type === 'ArrowFunctionExpression') {
+		current = current.parent;
+	}
+
+	return current;
+}
+
+function handleNestedArrowFunctions(parentNode, node) {
+	// Skip over arrow function expressions when they are parents and we came from a ReturnStatement
+	// This handles nested arrow functions: return next => action => { ... }
+	// But only when we're in a return statement context
+	if (parentNode.type === 'ArrowFunctionExpression' && node.type === 'ArrowFunctionExpression') {
+		const ancestor = getNonArrowAncestor(parentNode);
+		if (ancestor && ancestor.type === 'ReturnStatement') {
+			parentNode = skipArrowFunctionChain(parentNode);
+			if (parentNode.type === 'ReturnStatement') {
+				parentNode = parentNode.parent;
+			}
+		}
+	}
+
+	return parentNode;
+}
+
 function checkNode(node, scopeManager) {
 	const scope = scopeManager.acquire(node);
 
@@ -128,7 +165,15 @@ function checkNode(node, scopeManager) {
 		parentNode = parentNode.parent;
 	}
 
-	if (parentNode.type === 'BlockStatement') {
+	// Only skip ReturnStatement for arrow functions
+	// Regular function expressions have different semantics and shouldn't be moved
+	if (parentNode?.type === 'ReturnStatement' && node.type === 'ArrowFunctionExpression') {
+		parentNode = parentNode.parent;
+	}
+
+	parentNode = handleNestedArrowFunctions(parentNode, node);
+
+	if (parentNode?.type === 'BlockStatement') {
 		parentNode = parentNode.parent;
 	}
 

test/consistent-function-scoping.js

@@ -137,16 +137,6 @@ test({
 				bar =>
 				foo + bar;
 		`,
-		outdent`
-			const doFoo = () => {
-				return bar => bar;
-			}
-		`,
-		outdent`
-			function doFoo() {
-				return bar => bar;
-			}
-		`,
 		outdent`
 			const doFoo = foo => {
 				const doBar = bar => {
@@ -380,6 +370,27 @@ test({
 				inner();
 			}
 		`,
+		// #931 - valid cases: arrow functions that capture variables from outer scope
+		outdent`
+			function createHandler(config) {
+				return (event) => {
+					return config.handle(event);
+				};
+			}
+		`,
+		outdent`
+			export function middleware(store) {
+				return next => action => {
+					store.dispatch(action);
+					return next(action);
+				};
+			}
+		`,
+		outdent`
+			function createThunk(type) {
+				return (dispatch) => dispatch({ type });
+			}
+		`,
 		// Should ignore functions inside arrow functions
 		{
 			code: outdent`
@@ -807,6 +818,91 @@ test({
 			errors: [createError('function \'inner\'')],
 			options: [{checkArrowFunctions: false}],
 		},
+		// #931 - arrow functions returned from function declarations should be flagged
+		{
+			code: outdent`
+				export function someAction() {
+					return (dispatch) => dispatch({ type: 'SOME_TYPE' });
+				}
+			`,
+			errors: [createError('arrow function')],
+		},
+		{
+			code: outdent`
+				function outer() {
+					return bar => bar;
+				}
+			`,
+			errors: [createError('arrow function')],
+		},
+		{
+			code: outdent`
+				const outer = function() {
+					return bar => bar;
+				};
+			`,
+			errors: [createError('arrow function')],
+		},
+		{
+			code: outdent`
+				export function thunk() {
+					return async (dispatch, getState) => {
+						return dispatch({ type: 'ACTION' });
+					};
+				}
+			`,
+			errors: [createError('async arrow function')],
+		},
+		{
+			code: outdent`
+				function middleware() {
+					return next => action => {
+						return next(action);
+					};
+				}
+			`,
+			errors: [
+				createError('arrow function'),
+				createError('arrow function'),
+			],
+		},
+		{
+			code: outdent`
+				const doFoo = () => {
+					return bar => bar;
+				}
+			`,
+			errors: [createError('arrow function')],
+		},
+		// Additional test cases for edge cases
+		{
+			code: outdent`
+				class TestClass {
+					method() {
+						return x => x * 2;
+					}
+				}
+			`,
+			errors: [createError('arrow function')],
+		},
+		{
+			code: outdent`
+				const obj = {
+					method() {
+						return y => y + 1;
+					}
+				};
+			`,
+			errors: [createError('arrow function')],
+		},
+		{
+			code: outdent`
+				export default function() {
+					return z => z.toString();
+				}
+			`,
+			errors: [createError('arrow function')],
+		},
 	],
 });