diff --git a/README.md b/README.md index 89390df..4c54902 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ const webRules = getRulesForPlatform('web'); const backendRules = getRulesForPlatform('backend'); ``` -## Available Rules (50 total) +## Available Rules (51 total) ### Expo Router Rules @@ -222,6 +222,7 @@ const backendRules = getRulesForPlatform('backend'); | `no-manual-retry-loop` | warning | universal | Use a retry library instead of manual retry/polling loops | | `no-emoji-icons` | warning | universal | Use icons from lucide-react instead of emoji characters | | `prefer-named-params` | warning | universal | Use object destructuring instead of positional parameters | +| `prefer-promise-all` | warning | universal | Use Promise.all instead of sequential await in for...of loops | ### General Rules diff --git a/src/rules/index.ts b/src/rules/index.ts index dd01b2a..aacf174 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -49,6 +49,7 @@ import { noSyncFs } from './no-sync-fs'; import { preferNamedParams } from './prefer-named-params'; import { requireUseClient } from './require-use-client'; import { noServerImportInClient } from './no-server-import-in-client'; +import { preferPromiseAll } from './prefer-promise-all'; export const rules: Record = { 'no-relative-paths': noRelativePaths, @@ -101,4 +102,5 @@ export const rules: Record = { 'prefer-named-params': preferNamedParams, 'require-use-client': requireUseClient, 'no-server-import-in-client': noServerImportInClient, + '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..8a28fa2 --- /dev/null +++ b/src/rules/prefer-promise-all.ts @@ -0,0 +1,169 @@ +import traverse from '@babel/traverse'; +import * as t from '@babel/types'; +import type { File } 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) { + // 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: any) { + innerPath.skip(); + }, + + // Don't descend into nested loops + 'ForStatement|ForOfStatement|ForInStatement|WhileStatement|DoWhileStatement'( + innerPath: any, + ) { + innerPath.skip(); + }, + + AwaitExpression(_awaitPath: any) { + hasAwait = true; + }, + + CallExpression(callPath: any) { + 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: any) { + // Check this break targets the outer for...of (not a nested switch/loop) + let parent = 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: any) { + // Check this return is in the function containing the loop, not a nested fn + let parent = 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: any): boolean { + // Collect identifiers that are ASSIGNED inside the loop body + const assignedInLoop = new Set(); + + loopPath.traverse({ + 'FunctionDeclaration|FunctionExpression|ArrowFunctionExpression'(innerPath: any) { + innerPath.skip(); + }, + + AssignmentExpression(assignPath: any) { + const left = assignPath.node.left; + if (t.isIdentifier(left)) { + assignedInLoop.add(left.name); + } + }, + + UpdateExpression(updatePath: any) { + const arg = updatePath.node.argument; + if (t.isIdentifier(arg)) { + assignedInLoop.add(arg.name); + } + }, + }); + + 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 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; +} + +function isInsidePath(inner: any, outer: any): boolean { + let current = 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 112bd5e..81092d3 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(50); + expect(ruleNames.length).toBe(51); }); }); }); diff --git a/tests/prefer-promise-all.test.ts b/tests/prefer-promise-all.test.ts new file mode 100644 index 0000000..2647dbc --- /dev/null +++ b/tests/prefer-promise-all.test.ts @@ -0,0 +1,206 @@ +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); + }); +});