Skip to content

perf(linter/promise/always-return): narrow to function node types#23878

Merged
camc314 merged 3 commits into
mainfrom
perf/promise-always-return-node-types
Jun 28, 2026
Merged

perf(linter/promise/always-return): narrow to function node types#23878
camc314 merged 3 commits into
mainfrom
perf/promise-always-return-node-types

Conversation

@connorshea

@connorshea connorshea commented Jun 28, 2026

Copy link
Copy Markdown
Member

What

promise/always-return had NODE_TYPES = None, so the linter dispatched its run method on every AST node and relied on an early-return inside a helper to filter out non-functions. Because the node-kind check lived inside is_inline_then_function_expression, the linter codegen couldn't infer the rule's node types.

This PR leads run with a match node.kind() that returns early for non-function nodes so codegen picks it up.

This results in a behavior change (which is covered by the new tests), and that behavior change is a match for the original rule from eslint-plugin-promise and is therefore correct (assuming we did not diverge intentionally from the original rule).

This was generated with Claude Code, reviewed and tested by me.

Perf

Measured on VS Code (11674 files) with only this rule enabled:

Calls Time
Before 15,251,059 ~238ms
After 236,173 ~13ms

Behavior

The warning count shifts slightly (1189 → 1187). This is an intentional consequence of the change and actually brings the rule closer to the original ESLint rule's behavior.

Verification

  • cargo test -p oxc_linter always_return — rule pass/fail snapshot suite passes
  • test_rule_runner_impls passes (added a manual assert_rule_runs_on_node_types entry locking the bitset in both directions)
  • Built target/release/oxlint and smoke-tested the rule fires/stays-silent correctly

🤖 Generated with Claude Code

`promise/always-return` had `NODE_TYPES = None`, so the linter dispatched
its `run` method on every AST node and relied on an early-return inside a
helper to filter. The node-kind check lived in a helper, so the codegen
could not infer the rule's node types.

Lead `run` with a `match node.kind()` that returns early for non-function
nodes, which the codegen's `EarlyDivergeDetector` recognizes. The rule now
generates `NODE_TYPES = {Function, ArrowFunctionExpression}`, letting the
linter skip dispatch on other node kinds.

On 11674 files the rule's calls drop from 15,251,059 to 236,173 and its
time from ~238ms to ~13ms. The small violation-count change (1189 -> 1187
warnings) is an intended consequence that brings behavior closer to the
original rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the A-linter Area - Linter label Jun 28, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@connorshea connorshea marked this pull request as ready for review June 28, 2026 05:31
@connorshea connorshea requested a review from camc314 as a code owner June 28, 2026 05:31
@codspeed-hq

codspeed-hq Bot commented Jun 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 66 skipped benchmarks1


Comparing perf/promise-always-return-node-types (c7b08fa) with main (aa1ad74)

Open in CodSpeed

Footnotes

  1. 66 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.

Comment thread crates/oxc_linter/src/rules/promise/always_return.rs Outdated
Signed-off-by: Cameron <cameron.clark@hey.com>
@camc314 camc314 changed the title perf(linter): narrow promise/always-return to function node types perf(linter/promise/always-return): narrow to function node types Jun 28, 2026
@camc314 camc314 self-assigned this Jun 28, 2026
@camc314 camc314 merged commit 3e55c21 into main Jun 28, 2026
30 checks passed
@camc314 camc314 deleted the perf/promise-always-return-node-types branch June 28, 2026 10:42
camc314 added a commit that referenced this pull request Jun 29, 2026
# Oxlint
### 💥 BREAKING CHANGES

- 88f4455 str: [**BREAKING**] `Str` and `Ident` methods take
`&GetAllocator` (#23781) (overlookmotel)

### 🚀 Features

- f2091b3 ast: Unify old and new `AstBuilder`s (#23875) (overlookmotel)
- 1c8f50c linter: Add schema for `eslint/no-restricted-import` (#23642)
(Sysix)

### 🐛 Bug Fixes

- 7cb85c4 linter/eslint/no-negated-condition: Add autofix for negated
conditions (#23825) (Yagiz Nizipli)
- f7d1f50 oxlint, oxfmt: Enable `disable_old_builder` Cargo feature for
`oxc_ast` crate (#23886) (overlookmotel)
- d891990 linter/jsx-a11y/role-supports-aria-props: Ignore nullish prop
values (#23865) (Mikhail Baev)
- 94b6599 linter: Deduplicate missing plugin errors (#23853) (camc314)
- eff3eff linter/oxc/branches-sharing-code: Avoid else-if false
positives (#23843) (camc314)
- 2a2d3b9 linter/eslint/prefer-destructuring: Skip
`AssignmentExpression` autofixes (#23818) (camc314)
- ddc24ae linter/eslint/id-length: Respect checkGeneric for mapped type
keys (#23802) (bab)
- cd89202 linter/react/exhaustive-deps: Skip wrapper expression when
analyzing hook initializers (#23793) (camc314)
- 20e8285 linter/unicorn/prefer-native-coercion-function: Allow ts type
predicates (#23774) (camc314)
- d86f60b lsp: Normalize user config path to watch pattern (#23723)
(Sysix)
- 52032cf linter: Newline-terminate tsgolint errors (#23762) (Mikhail
Baev)
- 368fda7 linter/eslint/no-warning-comments: Avoid dropping generated
regex patterns (#23741) (camc314)
- ce44fbd linter/valid-title: Escape disallowed words regex (#23742)
(camc314)
- 3100d11 linter/prefer-called-exactly-once-with: Avoid out-of-bounds
slice panic at end of file (#23625) (Jerry Zhao)
- 742be36 refactor/node/handle-callback-err: Reject invalid regex config
(#23740) (camc314)
- d7be179 linter/eslint/no-restricted-globals: Handle shadowed locals
(#23736) (camc314)
- b3b1ff8 linter/vitest/expect-expect: Handle global vitest detection
correctly (#23734) (camc314)

### ⚡ Performance

- 68f9472 linter/jsx-a11y: Skip lowercasing non-aria attribute names
(#23906) (Lawrence Lin)
- b9312b4 linter/unicorn/prefer-export-from: Use keyed binding lookup
(#23893) (Marius Schulz)
- cd5204e linter/typescript/no-unsafe-declaration-merging: Use keyed
binding lookup (#23894) (Marius Schulz)
- e948498 linter/eslint/prefer-named-capture-group: Only dispatch for
relevant node types (#23868) (Connor Shea)
- 4ac7a8e linter/eslint/max-depth: Derive node types (#23896) (Connor
Shea)
- daeed09 linter/eslint/no-restricted-globals: Only scan unresolved
references (#23890) (camc314)
- e808514 linter/jest-vitest: Speed up no-standalone-expect (#23883)
(camc314)
- 8b165e5 linter/react/exhaustive-deps: Skip non-reactive calls early
(#23882) (camc314)
- 54005e7 linter/eslint/no-unused-vars: Precompute exported bindings
(#23881) (camc314)
- 9bc2f8c linter/unicorn/prefer-number-properties: Speed up global
checks (#23880) (camc314)
- 4ff104f linter: Optimize `require-hook` and `prefer-mock-*` rules to
run on specific node types (#23871) (Connor Shea)
- cc2213b linter: Run `no-underscore-dangle` only when relevant node
types are present (#23867) (Connor Shea)
- 3e55c21 linter/promise/always-return: Narrow to function node types
(#23878) (Connor Shea)
- 7136182 linter/jest-vitest: Speed up no-commented-out-tests (#23864)
(camc314)
- f138264 linter/eslint/no-script-url: Match javascript: prefix without
allocating (#23861) (Lawrence Lin)
- 7ef6895 linter/react/no-array-index-key: Delay index symbol lookup
(#23857) (camc314)
- 26bc171 linter/react/no-array-index-key: Match callback methods
directly (#23856) (camc314)
- 44fbbda linter/jsx-a11y/interactive-supports-focus: Check cheap
conditions first (#23854) (camc314)
- 84a5aa3 linter/eslint/no-extend-native: Skip lowercase references
early (#23851) (camc314)
- 88a74b2 linter/eslint/no-nonoctal-decimal-escape: Scan decimal escapes
as bytes (#23850) (camc314)
- fca69a8 linter: Skip traversal without this expressions (#23845)
(camc314)
- 838fd63 linter: Reduce preallocation for per-file diagnostics `Vec`
(#23705) (Marius Schulz)
- 417b506 linter/typescript/array-type: Remove full source text clone
(#23751) (Marius Schulz)

### 📚 Documentation

- 57e4469 linter/unicorn: Update prefer-dom-node-text-content rationale
(#23933) (Mikhail Baev)
- 3d61dea all: Correct capitalization in comments (#23887)
(overlookmotel)

### 🛡️ Security

- 3cdd18f deps: Update npm packages (#23690) (renovate[bot])
# Oxfmt
### 💥 BREAKING CHANGES

- 259e0cd oxfmt,formatter_graphql: [**BREAKING**] Support draft syntax
with removing prettier fallback (#23326) (leaysgur)
- accbc49 oxfmt: [**BREAKING**] Format `parser:css,less,scss` files +
css-in-js by `oxc_formatter_css` (#23321) (leaysgur)

### 🚀 Features

- dffa4b3 formatter_css: Implement `oxc_formatter_css` (#23320)
(leaysgur)
- 01de9ec oxfmt: Format `parser:graphql` files by
`oxc_formatter_graphql` (#23318) (leaysgur)
- 4e66212 formatter_graphql: Implement oxc_formatter_graphql (#23317)
(leaysgur)

### 🐛 Bug Fixes

- 67325ae formatter_css: Handle frontmatter language (#23819) (leaysgur)
- 3f355e5 formatter_graphql: Improve major prettier diffs (#23419)
(leaysgur)
- 48e2d78 formatter_css: Improve major prettier diffs (#23327)
(leaysgur)
- 8c07cad all: Enable `disable_old_builder` Cargo feature for `oxc_ast`
crate in tests (#23888) (overlookmotel)
- f7d1f50 oxlint, oxfmt: Enable `disable_old_builder` Cargo feature for
`oxc_ast` crate (#23886) (overlookmotel)
- d86f60b lsp: Normalize user config path to watch pattern (#23723)
(Sysix)

### ⚡ Performance

- 4ddcba0 formatter_core: Add printable-ASCII fast path to TextWidth
(#23913) (Lawrence Lin)

### 📚 Documentation

- b4d0dc9 oxfmt,formatter,formatter_css,formatter_core: Update AGENTS.md
(#23814) (leaysgur)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Co-authored-by: Cameron <cameron.clark@hey.com>
camc314 added a commit that referenced this pull request Jul 3, 2026
…3878)

## What

`promise/always-return` had `NODE_TYPES = None`, so the linter
dispatched its `run` method on **every** AST node and relied on an
early-return inside a helper to filter out non-functions. Because the
node-kind check lived inside `is_inline_then_function_expression`, the
linter codegen couldn't infer the rule's node types.

This PR leads `run` with a `match node.kind()` that returns early for
non-function nodes so codegen picks it up.

This results in a behavior change (which is covered by the new tests),
and that behavior change is a match for the original rule from
eslint-plugin-promise and is therefore correct (assuming we did not
diverge intentionally from the original rule).

This was generated with Claude Code, reviewed and tested by me.

## Perf

Measured on VS Code (11674 files) with only this rule enabled:

| | Calls | Time |
|---|---|---|
| Before | 15,251,059 | ~238ms |
| After | 236,173 | ~13ms |

## Behavior

The warning count shifts slightly (1189 → 1187). This is an
**intentional** consequence of the change and actually brings the rule
closer to the original ESLint rule's behavior.

## Verification

- `cargo test -p oxc_linter always_return` — rule pass/fail snapshot
suite passes
- `test_rule_runner_impls` passes (added a manual
`assert_rule_runs_on_node_types` entry locking the bitset in both
directions)
- Built `target/release/oxlint` and smoke-tested the rule
fires/stays-silent correctly

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Cameron <cameron.clark@hey.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Cameron <cameron.clark@hey.com>
camc314 added a commit that referenced this pull request Jul 3, 2026
# Oxlint
### 💥 BREAKING CHANGES

- 88f4455 str: [**BREAKING**] `Str` and `Ident` methods take
`&GetAllocator` (#23781) (overlookmotel)

### 🚀 Features

- f2091b3 ast: Unify old and new `AstBuilder`s (#23875) (overlookmotel)
- 1c8f50c linter: Add schema for `eslint/no-restricted-import` (#23642)
(Sysix)

### 🐛 Bug Fixes

- 7cb85c4 linter/eslint/no-negated-condition: Add autofix for negated
conditions (#23825) (Yagiz Nizipli)
- f7d1f50 oxlint, oxfmt: Enable `disable_old_builder` Cargo feature for
`oxc_ast` crate (#23886) (overlookmotel)
- d891990 linter/jsx-a11y/role-supports-aria-props: Ignore nullish prop
values (#23865) (Mikhail Baev)
- 94b6599 linter: Deduplicate missing plugin errors (#23853) (camc314)
- eff3eff linter/oxc/branches-sharing-code: Avoid else-if false
positives (#23843) (camc314)
- 2a2d3b9 linter/eslint/prefer-destructuring: Skip
`AssignmentExpression` autofixes (#23818) (camc314)
- ddc24ae linter/eslint/id-length: Respect checkGeneric for mapped type
keys (#23802) (bab)
- cd89202 linter/react/exhaustive-deps: Skip wrapper expression when
analyzing hook initializers (#23793) (camc314)
- 20e8285 linter/unicorn/prefer-native-coercion-function: Allow ts type
predicates (#23774) (camc314)
- d86f60b lsp: Normalize user config path to watch pattern (#23723)
(Sysix)
- 52032cf linter: Newline-terminate tsgolint errors (#23762) (Mikhail
Baev)
- 368fda7 linter/eslint/no-warning-comments: Avoid dropping generated
regex patterns (#23741) (camc314)
- ce44fbd linter/valid-title: Escape disallowed words regex (#23742)
(camc314)
- 3100d11 linter/prefer-called-exactly-once-with: Avoid out-of-bounds
slice panic at end of file (#23625) (Jerry Zhao)
- 742be36 refactor/node/handle-callback-err: Reject invalid regex config
(#23740) (camc314)
- d7be179 linter/eslint/no-restricted-globals: Handle shadowed locals
(#23736) (camc314)
- b3b1ff8 linter/vitest/expect-expect: Handle global vitest detection
correctly (#23734) (camc314)

### ⚡ Performance

- 68f9472 linter/jsx-a11y: Skip lowercasing non-aria attribute names
(#23906) (Lawrence Lin)
- b9312b4 linter/unicorn/prefer-export-from: Use keyed binding lookup
(#23893) (Marius Schulz)
- cd5204e linter/typescript/no-unsafe-declaration-merging: Use keyed
binding lookup (#23894) (Marius Schulz)
- e948498 linter/eslint/prefer-named-capture-group: Only dispatch for
relevant node types (#23868) (Connor Shea)
- 4ac7a8e linter/eslint/max-depth: Derive node types (#23896) (Connor
Shea)
- daeed09 linter/eslint/no-restricted-globals: Only scan unresolved
references (#23890) (camc314)
- e808514 linter/jest-vitest: Speed up no-standalone-expect (#23883)
(camc314)
- 8b165e5 linter/react/exhaustive-deps: Skip non-reactive calls early
(#23882) (camc314)
- 54005e7 linter/eslint/no-unused-vars: Precompute exported bindings
(#23881) (camc314)
- 9bc2f8c linter/unicorn/prefer-number-properties: Speed up global
checks (#23880) (camc314)
- 4ff104f linter: Optimize `require-hook` and `prefer-mock-*` rules to
run on specific node types (#23871) (Connor Shea)
- cc2213b linter: Run `no-underscore-dangle` only when relevant node
types are present (#23867) (Connor Shea)
- 3e55c21 linter/promise/always-return: Narrow to function node types
(#23878) (Connor Shea)
- 7136182 linter/jest-vitest: Speed up no-commented-out-tests (#23864)
(camc314)
- f138264 linter/eslint/no-script-url: Match javascript: prefix without
allocating (#23861) (Lawrence Lin)
- 7ef6895 linter/react/no-array-index-key: Delay index symbol lookup
(#23857) (camc314)
- 26bc171 linter/react/no-array-index-key: Match callback methods
directly (#23856) (camc314)
- 44fbbda linter/jsx-a11y/interactive-supports-focus: Check cheap
conditions first (#23854) (camc314)
- 84a5aa3 linter/eslint/no-extend-native: Skip lowercase references
early (#23851) (camc314)
- 88a74b2 linter/eslint/no-nonoctal-decimal-escape: Scan decimal escapes
as bytes (#23850) (camc314)
- fca69a8 linter: Skip traversal without this expressions (#23845)
(camc314)
- 838fd63 linter: Reduce preallocation for per-file diagnostics `Vec`
(#23705) (Marius Schulz)
- 417b506 linter/typescript/array-type: Remove full source text clone
(#23751) (Marius Schulz)

### 📚 Documentation

- 57e4469 linter/unicorn: Update prefer-dom-node-text-content rationale
(#23933) (Mikhail Baev)
- 3d61dea all: Correct capitalization in comments (#23887)
(overlookmotel)

### 🛡️ Security

- 3cdd18f deps: Update npm packages (#23690) (renovate[bot])
# Oxfmt
### 💥 BREAKING CHANGES

- 259e0cd oxfmt,formatter_graphql: [**BREAKING**] Support draft syntax
with removing prettier fallback (#23326) (leaysgur)
- accbc49 oxfmt: [**BREAKING**] Format `parser:css,less,scss` files +
css-in-js by `oxc_formatter_css` (#23321) (leaysgur)

### 🚀 Features

- dffa4b3 formatter_css: Implement `oxc_formatter_css` (#23320)
(leaysgur)
- 01de9ec oxfmt: Format `parser:graphql` files by
`oxc_formatter_graphql` (#23318) (leaysgur)
- 4e66212 formatter_graphql: Implement oxc_formatter_graphql (#23317)
(leaysgur)

### 🐛 Bug Fixes

- 67325ae formatter_css: Handle frontmatter language (#23819) (leaysgur)
- 3f355e5 formatter_graphql: Improve major prettier diffs (#23419)
(leaysgur)
- 48e2d78 formatter_css: Improve major prettier diffs (#23327)
(leaysgur)
- 8c07cad all: Enable `disable_old_builder` Cargo feature for `oxc_ast`
crate in tests (#23888) (overlookmotel)
- f7d1f50 oxlint, oxfmt: Enable `disable_old_builder` Cargo feature for
`oxc_ast` crate (#23886) (overlookmotel)
- d86f60b lsp: Normalize user config path to watch pattern (#23723)
(Sysix)

### ⚡ Performance

- 4ddcba0 formatter_core: Add printable-ASCII fast path to TextWidth
(#23913) (Lawrence Lin)

### 📚 Documentation

- b4d0dc9 oxfmt,formatter,formatter_css,formatter_core: Update AGENTS.md
(#23814) (leaysgur)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Co-authored-by: Cameron <cameron.clark@hey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants