diff --git a/README.md b/README.md index 39ade81..93891fa 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ const webRules = getRulesForPlatform('web'); const backendRules = getRulesForPlatform('backend'); ``` -## Available Rules (53 total) +## Available Rules (54 total) ### Expo Router Rules @@ -241,6 +241,7 @@ const backendRules = getRulesForPlatform('backend'); | `sql-no-nested-calls` | error | backend | Don't nest sql template tags | | `no-sync-fs` | error | backend | Use fs.promises or fs/promises instead of sync fs methods | | `no-unrestricted-loop-in-serverless` | error | backend | Unbounded loops (while(true), for(;;)) cause serverless timeouts | +| `prefer-promise-all` | warning | universal | Use Promise.all instead of sequential await in for...of loops | --- diff --git a/src/rules/index.ts b/src/rules/index.ts index 035b630..e803798 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -53,6 +53,7 @@ import { ssrBrowserApiGuard } from './ssr-browser-api-guard'; import { noReactNativeInWeb } from './no-react-native-in-web'; import { noModuleLevelNew } from './no-module-level-new'; import { noUnrestrictedLoopInServerless } from './no-unrestricted-loop-in-serverless'; +import { preferPromiseAll } from './prefer-promise-all'; export const rules: Record = { 'no-relative-paths': noRelativePaths, @@ -109,4 +110,5 @@ export const rules: Record = { 'no-react-native-in-web': noReactNativeInWeb, 'no-module-level-new': noModuleLevelNew, 'no-unrestricted-loop-in-serverless': noUnrestrictedLoopInServerless, + 'prefer-promise-all': preferPromiseAll, }; diff --git a/src/rules/prefer-promise-all.ts b/src/rules/prefer-promise-all.ts new file mode 100644 index 0000000..4a9f6cf --- /dev/null +++ b/src/rules/prefer-promise-all.ts @@ -0,0 +1,244 @@ +import traverse from '@babel/traverse'; +import type { NodePath } from '@babel/traverse'; +import * as t from '@babel/types'; +import type { + AssignmentExpression, + AwaitExpression, + BreakStatement, + CallExpression, + File, + ForOfStatement, + ReturnStatement, + UpdateExpression, +} from '@babel/types'; +import type { LintResult } from '../types'; + +const RULE_NAME = 'prefer-promise-all'; + +export function preferPromiseAll(ast: File, _code: string): LintResult[] { + const results: LintResult[] = []; + + traverse(ast, { + ForOfStatement(loopPath: NodePath) { + // Skip `for await...of` — already an async iterator pattern + if (loopPath.node.await) return; + + let hasAwait = false; + let hasOrderDependentPattern = false; + + loopPath.traverse({ + // Don't descend into nested functions — their awaits are independent + 'FunctionDeclaration|FunctionExpression|ArrowFunctionExpression'(innerPath: NodePath) { + innerPath.skip(); + }, + + // Don't descend into nested loops + 'ForStatement|ForOfStatement|ForInStatement|WhileStatement|DoWhileStatement'( + innerPath: NodePath, + ) { + innerPath.skip(); + }, + + AwaitExpression(_awaitPath: NodePath) { + hasAwait = true; + }, + + CallExpression(callPath: NodePath) { + const { callee } = callPath.node; + + // Detect .push() calls — order-dependent array accumulation + if ( + t.isMemberExpression(callee) && + t.isIdentifier(callee.property) && + callee.property.name === 'push' + ) { + hasOrderDependentPattern = true; + callPath.stop(); + } + + // Detect .splice() / .unshift() — order-dependent + if ( + t.isMemberExpression(callee) && + t.isIdentifier(callee.property) && + (callee.property.name === 'splice' || callee.property.name === 'unshift') + ) { + hasOrderDependentPattern = true; + callPath.stop(); + } + }, + + // Detect break/return conditioned on await result — sequential early-exit + BreakStatement(breakPath: NodePath) { + // Check this break targets the outer for...of (not a nested switch/loop) + let parent: NodePath | null = breakPath.parentPath; + while (parent && parent !== loopPath) { + const type = parent.node.type; + if ( + type === 'ForStatement' || + type === 'WhileStatement' || + type === 'DoWhileStatement' || + type === 'ForOfStatement' || + type === 'ForInStatement' || + type === 'SwitchStatement' + ) { + return; // break belongs to nested construct + } + parent = parent.parentPath; + } + hasOrderDependentPattern = true; + }, + + ReturnStatement(returnPath: NodePath) { + // Check this return is in the function containing the loop, not a nested fn + let parent: NodePath | null = returnPath.parentPath; + while (parent && parent !== loopPath) { + if ( + parent.node.type === 'FunctionDeclaration' || + parent.node.type === 'FunctionExpression' || + parent.node.type === 'ArrowFunctionExpression' + ) { + return; // return belongs to nested function + } + parent = parent.parentPath; + } + hasOrderDependentPattern = true; + }, + }); + + if (hasOrderDependentPattern) return; + + // Check for cross-iteration data dependencies: + // A variable declared BEFORE the loop that is both read and written inside the loop body + if (hasCrossIterationDependency(loopPath)) return; + + if (hasAwait) { + const { loc } = loopPath.node; + results.push({ + rule: RULE_NAME, + message: + 'Sequential await in for...of loop can likely be parallelized with Promise.all(items.map(async (item) => ...)) for better performance', + line: loc?.start.line ?? 0, + column: loc?.start.column ?? 0, + severity: 'warning', + }); + } + }, + }); + + return results; +} + +function hasCrossIterationDependency(loopPath: NodePath): boolean { + // Collect identifiers that are ASSIGNED inside the loop body + const assignedInLoop = new Set(); + + loopPath.traverse({ + 'FunctionDeclaration|FunctionExpression|ArrowFunctionExpression'(innerPath: NodePath) { + innerPath.skip(); + }, + + AssignmentExpression(assignPath: NodePath) { + collectAssignedNames(assignPath.node.left, assignedInLoop); + }, + + UpdateExpression(updatePath: NodePath) { + const arg = updatePath.node.argument; + if (t.isIdentifier(arg)) { + assignedInLoop.add(arg.name); + } else if (t.isMemberExpression(arg)) { + const root = getMemberRootName(arg); + if (root) assignedInLoop.add(root); + } + }, + }); + + if (assignedInLoop.size === 0) return false; + + // Check if any of those assigned variables are bound OUTSIDE the loop + for (const name of assignedInLoop) { + const binding = loopPath.scope.getBinding(name); + if (!binding) continue; // global or unresolved — assume no dependency + + // If the variable is declared outside the loop, it's a cross-iteration dependency + const declPath = binding.path; + if (!isInsidePath(declPath, loopPath)) { + return true; + } + } + + return false; +} + +// Walk a left-hand side and collect every identifier name it ultimately writes +// to. Handles Identifier, MemberExpression (root object), ObjectPattern, +// ArrayPattern, RestElement, AssignmentPattern (default-value destructure), +// and TS-only wrappers like `as` / `!` casts. +function collectAssignedNames(node: t.Node, out: Set): void { + if (t.isIdentifier(node)) { + out.add(node.name); + return; + } + + if (t.isMemberExpression(node) || t.isOptionalMemberExpression(node)) { + // For `state.prev = ...` or `obj.a.b = ...`, treat the root identifier + // (e.g. `state`, `obj`) as the dependency surface. If that root is bound + // outside the loop, mutations to it carry across iterations. + const root = getMemberRootName(node); + if (root) out.add(root); + return; + } + + if (t.isObjectPattern(node)) { + for (const prop of node.properties) { + if (t.isObjectProperty(prop)) { + // value is the binding target (e.g. `{ a: x }` → `x`; `{ a }` → `a`) + collectAssignedNames(prop.value, out); + } else if (t.isRestElement(prop)) { + collectAssignedNames(prop.argument, out); + } + } + return; + } + + if (t.isArrayPattern(node)) { + for (const el of node.elements) { + if (el === null) continue; + collectAssignedNames(el, out); + } + return; + } + + if (t.isRestElement(node)) { + collectAssignedNames(node.argument, out); + return; + } + + if (t.isAssignmentPattern(node)) { + collectAssignedNames(node.left, out); + return; + } + + if (t.isTSNonNullExpression(node) || t.isTSAsExpression(node)) { + collectAssignedNames(node.expression, out); + return; + } +} + +function getMemberRootName(node: t.MemberExpression | t.OptionalMemberExpression): string | null { + let current: t.Expression | t.Super = node.object; + while (t.isMemberExpression(current) || t.isOptionalMemberExpression(current)) { + current = current.object; + } + if (t.isIdentifier(current)) return current.name; + if (t.isThisExpression(current)) return 'this'; + return null; +} + +function isInsidePath(inner: NodePath, outer: NodePath): boolean { + let current: NodePath | null = inner; + while (current) { + if (current === outer) return true; + current = current.parentPath; + } + return false; +} diff --git a/tests/config-modes.test.ts b/tests/config-modes.test.ts index d8bbfed..ff43264 100644 --- a/tests/config-modes.test.ts +++ b/tests/config-modes.test.ts @@ -123,7 +123,7 @@ describe('config modes', () => { expect(ruleNames).toContain('no-relative-paths'); expect(ruleNames).toContain('expo-image-import'); expect(ruleNames).toContain('no-stylesheet-create'); - expect(ruleNames.length).toBe(54); + expect(ruleNames.length).toBe(55); }); }); }); diff --git a/tests/prefer-promise-all.test.ts b/tests/prefer-promise-all.test.ts new file mode 100644 index 0000000..ada5462 --- /dev/null +++ b/tests/prefer-promise-all.test.ts @@ -0,0 +1,296 @@ +import { describe, it, expect } from 'vitest'; +import { lintJsxCode } from '../src'; + +const config = { rules: ['prefer-promise-all'] }; + +describe('prefer-promise-all rule', () => { + it('should flag sequential await in for...of with bare await', () => { + const code = ` + async function processAll(items) { + for (const item of items) { + await processItem(item); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(1); + expect(results[0].rule).toBe('prefer-promise-all'); + expect(results[0].message).toContain('Promise.all'); + expect(results[0].severity).toBe('warning'); + }); + + it('should flag when result is added to a Set', () => { + const code = ` + async function fetchAll(urls) { + const results = new Set(); + for (const url of urls) { + const data = await fetch(url); + results.add(data); + } + return results; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(1); + }); + + it('should flag when result is stored in a Map', () => { + const code = ` + async function fetchAll(items) { + const cache = new Map(); + for (const item of items) { + const result = await fetchItem(item.id); + cache.set(item.id, result); + } + return cache; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(1); + }); + + it('should flag when await result is assigned to a local variable only', () => { + const code = ` + async function processAll(items) { + for (const item of items) { + const result = await doWork(item); + console.log(result); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(1); + }); + + it('should NOT flag when result is pushed to an array', () => { + const code = ` + async function fetchAll(urls) { + const results = []; + for (const url of urls) { + const data = await fetch(url); + results.push(data); + } + return results; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag when loop has break', () => { + const code = ` + async function findFirst(items) { + for (const item of items) { + const result = await check(item); + if (result.found) break; + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag when loop has return', () => { + const code = ` + async function findFirst(items) { + for (const item of items) { + const result = await check(item); + if (result.found) return result; + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag cross-iteration dependency', () => { + const code = ` + async function chain(items) { + let prev = null; + for (const item of items) { + prev = await process(prev, item); + } + return prev; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag cross-iteration accumulator', () => { + const code = ` + async function sum(items) { + let total = 0; + for (const item of items) { + const value = await getValue(item); + total += value; + } + return total; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag for await...of', () => { + const code = ` + async function consume(stream) { + for await (const chunk of stream) { + await processChunk(chunk); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag loops without await', () => { + const code = ` + function processAll(items) { + for (const item of items) { + processItem(item); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag when await is inside a nested function (e.g. .map callback)', () => { + const code = ` + function processAll(items) { + for (const group of groups) { + const results = group.items.map(async (item) => { + return await fetchItem(item); + }); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag break inside a switch statement (not targeting outer loop)', () => { + const code = ` + async function processAll(items) { + for (const item of items) { + const result = await fetchItem(item); + switch (result.type) { + case 'a': + handleA(result); + break; + default: + handleDefault(result); + break; + } + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(1); + }); + + it('should NOT flag unshift to array', () => { + const code = ` + async function fetchAll(urls) { + const results = []; + for (const url of urls) { + const data = await fetch(url); + results.unshift(data); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + // Cross-iteration dependency via member-expression assignment. + // Pre-fix this would false-positive because hasCrossIterationDependency only + // looked at Identifier lefts. dx5v review on PR #57. + it('should NOT flag cross-iteration dependency through member expression LHS', () => { + const code = ` + async function chain(items) { + const state = { prev: null }; + for (const item of items) { + state.prev = await process(state.prev, item); + } + return state.prev; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + it('should NOT flag cross-iteration dependency through nested member expression LHS', () => { + const code = ` + async function chain(items) { + const state = { meta: { prev: null } }; + for (const item of items) { + state.meta.prev = await process(state.meta.prev, item); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + // Cross-iteration dependency via object-pattern assignment (re-bind). + // dx5v review on PR #57. + it('should NOT flag cross-iteration dependency through object-pattern LHS', () => { + const code = ` + async function chain(items) { + let next = null; + for (const item of items) { + ({ next } = await process(next, item)); + } + return next; + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + // Cross-iteration dependency via aliased object-pattern (e.g. `{ a: x }`). + it('should NOT flag cross-iteration dependency through aliased object-pattern LHS', () => { + const code = ` + async function chain(items) { + let prev = null; + for (const item of items) { + ({ next: prev } = await process(prev, item)); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + // Cross-iteration dependency via array-pattern assignment. + it('should NOT flag cross-iteration dependency through array-pattern LHS', () => { + const code = ` + async function chain(items) { + let prev = null; + let count = 0; + for (const item of items) { + [prev, count] = await process(prev, item); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(0); + }); + + // Member writes targeting a fresh object declared inside the loop should + // still flag — the root binding is loop-local, not cross-iteration. + it('should still flag member-expression assignment to a loop-local object', () => { + const code = ` + async function processAll(items) { + for (const item of items) { + const local = {}; + local.value = await fetchItem(item); + } + } + `; + const results = lintJsxCode(code, config); + expect(results).toHaveLength(1); + }); +});