Skip to content

feat(no-single-promise-in-promise-methods): add no-single-promise-in-promise-methods rule - #1524

Open
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-no-single-promise-in-promise-methods
Open

bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-no-single-promise-in-promise-methods

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

This ports the oxlint unicorn/no-single-promise-in-promise-methods rule to
deno_lint as a native Rust rule.

The rule flags calls to Promise.all([x]), Promise.any([x]), and
Promise.race([x]) where the sole argument is an array literal containing
exactly one element and that element is neither a spread nor an elision. Wrapping
a single value in such a call is pointless: the value can be used directly, or
Promise.resolve(...) can be used instead. The argument array is unwrapped
through parentheses and TypeScript type-modifying expressions (as,
satisfies, as const, non-null assertions, type assertions, instantiation
expressions) before inspection, and optional-chaining/computed forms such as
Promise?.race([x]), Promise.race?.([x]), and Promise["race"]([x]) are not
matched. Promise.allSettled([x]) is intentionally not flagged, matching the
oxc reference.

Unlike oxc, which ships a conditional fixer, this implementation is
diagnostic-only (no autofix), consistent with the rest of deno_lint. The
diagnostic message and help text, and the complete pass/fail fixture sets, are
ported from the oxc reference:
https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/rules/unicorn/no_single_promise_in_promise_methods.rs

Tagged RECOMMENDED (on by default) — flagging for maintainer sign-off.

@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: LGTM — correct and carefully handled. Recommend merge after the RECOMMENDED sign-off you flagged + rebase. Not auto-merging (new rule).

Faithful, thorough port. I traced the tricky cases:

Correctness — verified

  • Callee must be a non-optional static Promise.<method>: optional chaining (Promise?.race(...), Promise.race?.(...)) is an OptChainExpr and never reaches call_expr, and computed access (Promise["race"]) fails the MemberProp::Ident check. Both correctly excluded.
  • Method set all | any | race with allSettled intentionally excluded. ✓
  • unwrap_expr unwraps parens + all the TS type-modifying wrappers (as, satisfies, as const, !, <T>, instantiation) before checking for an array literal — matches the description and reference.
  • Single-element checks are careful: array.elems.len() == 1, then let Some(elem) = array.elems[0] correctly skips an elision (Promise.all([,])elems[0] is None → not flagged), and elem.spread().is_some() skips a spread element (Promise.all([...x])). Arg-level spread (Promise.all(...x)) is also excluded.
  • args[0] indexing is guarded by the prior args.len() == 1 check — no panic.

Minor / non-blocking

  • Name-based obj.sym() == "Promise" would also match a shadowed local Promise — consistent with oxc's approach; fine.
  • Unnecessary(String) allocates the method name per diagnostic; negligible since diagnostics are rare.

Perf: trivial (per call_expr).

Housekeeping: RECOMMENDED sign-off (you flagged it); mergeable: UNKNOWN → rebase; docs .md not in diff.

No code changes requested — the edge-case handling (elision, spread, TS wrappers, optional/computed, allSettled) is exactly right. Just the tagging decision + rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant