Conversation
Fixes #1839 Previously, the rule would flag async map callbacks even when the promise array was passed to a local wrapper around Promise.allSettled, Promise.all, Promise.race, or Promise.any. The fix adds detection for local functions that internally use these promise concurrency methods. When a mapped promise array is passed to such a wrapper, the rule now correctly recognizes that the operations are already running in parallel. Test cases cover: - Promise.allSettled wrapper (the reported case) - Promise.all wrapper - Promise.race wrapper - Negative case: wrapper without promise concurrency (still flagged) - Regression corpus entry for the exact reported pattern Co-authored-by: Skosh <skoshx@users.noreply.github.com>
The fuzz harness requires plain JavaScript syntax, so remove TypeScript generic type parameters from the regression test file. Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
commit: |
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Contributor
Interactive terminal E2ETerminal Control verified the built CLI at
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1839
Root Cause
The
async-await-in-looprule was reporting false positives when async.map()callbacks were passed to a local wrapper function aroundPromise.allSettled(),Promise.all(), or other promise concurrency methods.The rule correctly identified that promises from
.map()were being awaited, but it didn't recognize that those promises were passed to a local function that internally uses promise concurrency, meaning the operations were actually running in parallel.Fix
Added detection for local functions that internally use promise concurrency methods (
Promise.all,Promise.allSettled,Promise.race,Promise.any). When a mapped promise array is passed to such a wrapper, the rule now correctly recognizes that the operations are already running in parallel and doesn't flag the code.Implementation
The fix adds:
doesLocalFunctionUsePromiseConcurrency()- helper function that walks a local function's AST to detect if it uses any promise concurrency methodsisWrappedInPromiseConcurrency()to resolve local function calls and check them with the new helperScope Decision
What was fixed: Detection of direct local wrapper functions that internally use promise concurrency.
What was NOT fixed: Higher-order functions where the wrapper is passed as a parameter (e.g.,
helper(items, waitAll)). This would require more complex analysis tracking function parameters through call chains. The reported issue specifically concerns direct wrapper calls, not parameterized combinators.Test Coverage
Parity Check
Ready to Review
This PR is ready for code review. All tests pass, the fix is well-scoped and focused on the reported case, and the reproduction example no longer produces a false positive.