fix(linter/promise/no-return-wrap): detect Promise calls in all branches by zennnnnnn11 · Pull Request #22474 · oxc-project/oxc

zennnnnnn11 · 2026-05-15T22:22:54Z

Summary

Replace manual if/return traversal in check_first_return_statement with a Visit-based ReturnWrapFinder that walks the entire callback body
Stop at nested function boundaries to avoid false positives
Delete check_first_return_statement entirely

Before this fix

The rule only examined the first return or if statement in the callback body. A copy-paste bug also caused the "look in else" fallback to search the if consequent a second time, never actually checking alternate. This meant:

// None of these were caught:
doThing().then(val => { if (val) { return val } else { return Promise.resolve(val) } })
doThing().then(val => { if (val > 10) { if (val > 20) { return Promise.resolve(val) } } })
doThing().then(val => { switch(val) { case 1: return Promise.resolve(val) } })
doThing().then(val => { try { return Promise.resolve(val) } catch(e) {} })

After this fix

All return Promise.resolve() / return Promise.reject() statements inside .then(), .catch(), and .finally() callbacks are checked, regardless of nesting depth in if/else, switch, try/catch, etc. The visitor stops at nested function/arrow boundaries to avoid crossing scope.

Scope note

The existing inside_promise_cb gate treats any CallExpression nested inside a promise chain as a candidate for argument checking. This is a pre-existing design decision (see the fn3 test case) and is not changed by this PR. Expression-body arrows with conditional expressions (e.g., x => x ? Promise.resolve(x) : x) are also not covered, consistent with the original eslint-plugin-promise behavior — both could be addressed as follow-ups.

Test plan

Added 9 new test cases covering: else branches (resolve + reject), nested if, else-if, switch, try/catch, arrow block body, and allowReject option
Updated insta snapshot
All existing tests continue to pass
cargo test -p oxc_linter -- rules::promise::no_return_wrap passes
Follows existing Visit pattern from no_promise_executor_return.rs

🤖 Generated with Claude Code

Replace manual AST traversal in `check_first_return_statement` with a `Visit`-based approach that walks all branches of control flow (if/else, nested conditionals, else-if chains) to find `return Promise.resolve()` and `return Promise.reject()` wrapping. Previously the rule only checked the first return in the `if` consequent block, missing returns in else branches, nested ifs, and else-if chains. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…urn-wrap Cover additional control flow paths exercised by the Visit-based traversal: switch/case, try/catch, and allowReject option interaction with else branches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

camc314

thank you!

Copilot

Pull request overview

This PR fixes promise/no-return-wrap so it detects return Promise.resolve(...) / return Promise.reject(...) inside promise handler callbacks even when the return is nested under control-flow constructs (e.g. if/else, nested if, switch, try/catch). It replaces a shallow/manual traversal (that also had an else-branch bug) with a visitor that walks the full callback body while stopping at nested function boundaries.

Changes:

Replace the old “first return/if only” scanning with a Visit-based ReturnWrapFinder that traverses the entire callback body and checks all return statements.
Prevent false positives by stopping traversal at nested function / arrow boundaries.
Add/extend test cases and update the insta snapshot to cover nested branches and the allowReject option.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
crates/oxc_linter/src/rules/promise/no_return_wrap.rs	Reworks callback-body scanning to visitor-based traversal that finds wrapped returns in all nested branches while skipping nested functions.
crates/oxc_linter/src/snapshots/promise_no_return_wrap.snap	Updates snapshot output to include newly-detected violations from the added/expanded tests.

codspeed-hq · 2026-05-18T08:58:38Z

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 47 skipped benchmarks¹

_{Comparing zennnnnnn11:fix/promise-no-return-wrap-nested-conditionals (0c55867) with main (c27a8cf)}

47 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

# Oxlint ### 🚀 Features - 1ae291e linter/no-underscore-dangle: Add `allowInUsingDeclarations` option (#22483) (吴杨帆) - 0440b0f linter/eslint: Implement `id-match` rule (#22379) (Vladislav Sayapin) - 65bf119 linter: Implement react no-object-type-as-default-prop (#22481) (uhyo) - 2a6ddce linter/eslint: Implement `no-implied-eval` rule (#22391) (Vladislav Sayapin) - d3a3c1d linter: Auto detect agents from CLI and transition to the agent output format (#22068) (Jovi De Croock) - 625758a linter/vitest: Implement padding-around-after-all-blocks rule (#21788) (kapobajza) - 37680b0 linter: Implement react no-unstable-nested-components (#22248) (Jovi De Croock) - d8d9c74 linter: Implement import/newline-after-import rule (#19142) (Ryuya Yanagi) ### 🐛 Bug Fixes - 3f59e03 linter: Only call rayon/miette/tracing inits once (#21899) (Matiss Janis Aboltins) - 602dfd6 linter/promise/no-return-wrap: Detect Promise calls in all branches (#22474) (zennnnnnn11) - e182aee linter: Allow dialogs and popovers for no_autofocus (#22289) (mehm8128) - 7ffb710 linter/jest/vitest: Jest/no-standalone-expect ignores additionalTestBlockFunctions option for jest/vitest hooks (#22477) (kapobajza) - c6f2d3f linter: Add more expression support for iframe-has-title (#22460) (mehm8128) - 5747ff1 linter: Avoid enabling jest with vitest plugin (#22499) (camc314) - 863984f linter/no-find-dom-node: Run on all files (#22479) (bab) ### ⚡ Performance - 2afef79 linter: Optimize `no-loop-func` (#22491) (camchenry) - 4c9ca72 oxlint: Align walker thread count with rayon pool (#22494) (Boshen) ### 📚 Documentation - f7967c7 linter/id-match: Clarify `onlyDeclarations` config docs (#22523) (camc314) - 1e0c97f linter: Fix closing code block in documentation for `padding-around-after-all-blocks` rule. (#22513) (connorshea) - a9049fd linter: Exclude directly provide autoFocus to dialog pattern (#22510) (mehm8128) # Oxfmt ### 🐛 Bug Fixes - 8ee946f formatter/sort_imports: Use label to classify lines (#22512) (leaysgur) - 8c1da44 formatter: Normalize destructuring keys in DCR (#22478) (camc314)

zennnnq and others added 2 commits May 16, 2026 05:54

zennnnnnn11 requested a review from camc314 as a code owner May 15, 2026 22:22

camc314 added the A-linter Area - Linter label May 18, 2026

camc314 requested a review from Copilot May 18, 2026 08:45

Copilot started reviewing on behalf of camc314 May 18, 2026 08:45 View session

u

f0aaa94

camc314 approved these changes May 18, 2026

View reviewed changes

camc314 changed the title ~~fix(linter): detect Promise.resolve/reject in all branches of promise/no-return-wrap~~ fix(linter/promise/no-return-wrap): detect Promise calls in all branches May 18, 2026

camc314 self-assigned this May 18, 2026

Merge branch 'main' into fix/promise-no-return-wrap-nested-conditionals

a7bb7be

Copilot AI reviewed May 18, 2026

View reviewed changes

lint

0c55867

camc314 merged commit 602dfd6 into oxc-project:main May 18, 2026
28 checks passed

oxc-guard Bot mentioned this pull request May 18, 2026

release(apps): oxlint v1.66.0 && oxfmt v0.51.0 #22528

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(linter/promise/no-return-wrap): detect Promise calls in all branches#22474

fix(linter/promise/no-return-wrap): detect Promise calls in all branches#22474
camc314 merged 5 commits into
oxc-project:mainfrom
zennnnnnn11:fix/promise-no-return-wrap-nested-conditionals

zennnnnnn11 commented May 15, 2026 •

edited

Loading

Uh oh!

camc314 left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codspeed-hq Bot commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

zennnnnnn11 commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before this fix

After this fix

Scope note

Test plan

Uh oh!

camc314 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codspeed-hq Bot commented May 18, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zennnnnnn11 commented May 15, 2026 •

edited

Loading