Skip to content

feat(linter): implement no-useless-assignment#15466

Merged
camc314 merged 12 commits intooxc-project:mainfrom
zzt1224:feat/no-useless-assignment
Mar 30, 2026
Merged

feat(linter): implement no-useless-assignment#15466
camc314 merged 12 commits intooxc-project:mainfrom
zzt1224:feat/no-useless-assignment

Conversation

@zzt1224
Copy link
Copy Markdown
Contributor

@zzt1224 zzt1224 commented Nov 8, 2025

No description provided.

Copilot AI review requested due to automatic review settings November 8, 2025 08:11
@zzt1224 zzt1224 requested a review from camc314 as a code owner November 8, 2025 08:11
@graphite-app
Copy link
Copy Markdown
Contributor

graphite-app Bot commented Nov 8, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions Bot added the A-linter Area - Linter label Nov 8, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the no-useless-assignment ESLint rule, which detects and flags assignments where the newly assigned value is never read afterward. The implementation uses control flow graph analysis to track read and write operations across different code paths.

Key changes:

  • Added new linter rule for detecting useless assignments (dead stores)
  • Implemented CFG-based analysis to track symbol operations across control flow paths
  • Added comprehensive test coverage with 70+ test cases

Reviewed Changes

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

File Description
crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Core implementation of the no-useless-assignment rule with CFG analysis
crates/oxc_linter/src/snapshots/eslint_no_useless_assignment.snap Test snapshots showing diagnostic output for failing test cases
crates/oxc_linter/src/rules.rs Registration of the new rule in the linter module system

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Nov 8, 2025

Hi @camc314, my PR didn’t pass just r. When I run the local CI check, it generates a bunch of snapshot updates. I’m not sure if that’s expected.

@connorshea
Copy link
Copy Markdown
Member

connorshea commented Nov 8, 2025

Hi @camc314, my PR didn’t pass just r. When I run the local CI check, it generates a bunch of snapshot updates. I’m not sure if that’s expected.

It'd depend on the specific updates to the snapshots, but that's probably fine as long as the only snapshot it's trying to update is the one for your particular rule.

@connorshea
Copy link
Copy Markdown
Member

You'll also want to run just fmt to format the files, otherwise the CI checks will fail on that

@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Nov 8, 2025

You'll also want to run just fmt to format the files, otherwise the CI checks will fail on that

Thanks for the tip!

@connorshea
Copy link
Copy Markdown
Member

connorshea commented Nov 8, 2025

If you use VS Code or similar, you can change the rust-analyzer extension's checker setting to "clippy" to get the full linting (or just running cargo clippy in a terminal should perform the lint checks that CI is failing on).

Unfortunately it's also a lot slower, but that should help you fix the lint errors at least. I'll let cam review further as he sees fit :)

@github-actions github-actions Bot added A-cli Area - CLI A-codegen Area - Code Generation labels Nov 8, 2025
@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Nov 8, 2025

If you use VS Code or similar, you can change the rust-analyzer extension's checker setting to "clippy" to get the full linting (or just running cargo clippy in a terminal should perform the lint checks that CI is failing on).

Unfortunately it's also a lot slower, but that should help you fix the lint errors at least. I'll let cam review further as he sees fit :)

Thanks for the quick review! I’m new to Rust and OXC, so I didn’t really have that in mind when implementing this. I understand that performance is critical for the project — is there any way I can check benchmarks for my implementation? I’d love the chance to improve the performance.

@zzt1224 zzt1224 changed the title Feat/no useless assignment feat/no useless assignment Nov 8, 2025
@zzt1224 zzt1224 changed the title feat/no useless assignment feat(linter)/no useless assignment Nov 8, 2025
@zzt1224 zzt1224 changed the title feat(linter)/no useless assignment feat(linter): no useless assignment Nov 8, 2025
@github-actions github-actions Bot added the C-enhancement Category - New feature or request label Nov 8, 2025
@camc314
Copy link
Copy Markdown
Contributor

camc314 commented Nov 8, 2025

Amazing work on this!

Out of interest, did you use AI? if so how did you use it and which models did you use?

I understand that performance is critical for the project — is there any way I can check benchmarks for my implementation? I’d love the chance to improve the performance.

I just approced the CI workflows to run, that will give us a codspeed benchmark which should highlight any performance regressions.

Looks like this rule is really struggling on the react benchmark.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Nov 8, 2025

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 52 skipped benchmarks1


Comparing zzt1224:feat/no-useless-assignment (bf11fc6) with main (0f01fbd)2

Open in CodSpeed

Footnotes

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

  2. No successful run was found on main (22ce6af) during the generation of this report, so 0f01fbd was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Nov 8, 2025

Amazing work on this!

Out of interest, did you use AI? if so how did you use it and which models did you use?

I understand that performance is critical for the project — is there any way I can check benchmarks for my implementation? I’d love the chance to improve the performance.

I just approced the CI workflows to run, that will give us a codspeed benchmark which should highlight any performance regressions.

Looks like this rule is really struggling on the react benchmark.

I’ll see if I can work something out to improve the performance on the React benchmark.

I do use AI — mainly Deepwiki, which Boshen recommended on Discord. I ask all kinds of codebase-related questions, like “How can I detect a loop in a CFG?” or “If I have a node ID, how can I get its CFG block?” It doesn’t give me direct copy-and-paste answers every single time, but it points me to the right parts of the code I can study.

For example, I asked Deepwiki, “How do I get the variable scope of a symbol?” and it gave me your code:

fn get_parent_variable_scope(&self, scope_id: ScopeId) -> ScopeId {
    self.scoping()
        .scope_ancestors(scope_id)
        .find_or_last(|scope_id| self.scoping().scope_flags(*scope_id).is_var())
        .expect("scope iterator will always contain at least one element")
}

@camc314
Copy link
Copy Markdown
Contributor

camc314 commented Nov 8, 2025

I’ll see if I can work something out to improve the performance on the React benchmark.

yeahh, we may need a different algorithm. admittedly, this was in debug modes but running this rule on react.development.js was taking 1+ minutes.

i had a local version of this working somewhere that used fixed point iteration instead which maybe a better approach.

Thanks for sharing how you used AI, i do find it interesting how others are using it.

@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Nov 9, 2025

I’ll see if I can work something out to improve the performance on the React benchmark.

yeahh, we may need a different algorithm. admittedly, this was in debug modes but running this rule on react.development.js was taking 1+ minutes.

i had a local version of this working somewhere that used fixed point iteration instead which maybe a better approach.

Thanks for sharing how you used AI, i do find it interesting how others are using it.

I’ve pinpointed that the part slowing everything down is moving things in and out of the backtracking state. I probably need to refactor the whole thing to get rid of that. I’ll try a new approach to this, so you can close this PR for now. Would you mind sharing your solution and elaborating on what “fixed-point iteration” means? Or, if you’re already working on this, I can move on to something else.

To be honest, I thought this one was labeled “good first issue,” so I expected it to be an easy one.

@zzt1224 zzt1224 force-pushed the feat/no-useless-assignment branch from 177df6c to b317e90 Compare November 25, 2025 06:24
@zzt1224 zzt1224 requested a review from Dunqing as a code owner November 25, 2025 06:24
@github-actions github-actions Bot added the A-transformer Area - Transformer / Transpiler label Nov 25, 2025
@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Nov 25, 2025

Hi @camc314, I’ve improved the performance of my previous approach — it should be much faster than before. Could you give it another shot and see if the performance looks acceptable now? Thanks!

I used a bitset data structure from oxc/allocator; I’m not sure if that’s allowed. I also didn’t use a fix-point iteration to resolve the loop issue. Instead, I run a separate analysis on the loop to obtain its live-ins, which I believe should be faster.

@zzt1224 zzt1224 force-pushed the feat/no-useless-assignment branch 2 times, most recently from 793be5f to 2db92d3 Compare November 26, 2025 05:52
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs Outdated
@zzt1224 zzt1224 force-pushed the feat/no-useless-assignment branch from 0e53ac4 to 86ede81 Compare December 19, 2025 08:13
@overlookmotel overlookmotel removed A-transformer Area - Transformer / Transpiler A-codegen Area - Code Generation labels Dec 19, 2025
@zzt1224 zzt1224 force-pushed the feat/no-useless-assignment branch 2 times, most recently from 8d5f263 to 2ca49d5 Compare March 7, 2026 05:17
@zzt1224 zzt1224 requested a review from camc314 March 7, 2026 05:26
@zzt1224
Copy link
Copy Markdown
Contributor Author

zzt1224 commented Mar 9, 2026

Hi @camc314 , I’ve resolved the performance regression. Could you review it?

@camc314 camc314 force-pushed the feat/no-useless-assignment branch from 2ca49d5 to 94530c7 Compare March 30, 2026 17:56
Comment thread crates/oxc_linter/src/rules/eslint/no_useless_assignment.rs
camc314 added 3 commits March 30, 2026 20:48
This reverts commit 11db72a.
This reverts commit ced4d4b.
This reverts commit 96bb109.
Copy link
Copy Markdown
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! - Amazing work on this one!

Apologies for the delay in getting it merged!

@camc314 camc314 changed the title feat(linter): no useless assignment feat(linter): implement no-useless-assignment Mar 30, 2026
@camc314 camc314 merged commit 871f9d9 into oxc-project:main Mar 30, 2026
27 checks passed
leaysgur pushed a commit that referenced this pull request Apr 7, 2026
# Oxlint
### 💥 BREAKING CHANGES

- 22ce6af oxlint/lsp: [**BREAKING**] Show/fix safe suggestions by
default (#19816) (Sysix)

### 🚀 Features

- 7a7b7b8 oxlint/lsp: Add source.fixAllDangerous.oxc code action kind
(#20526) (bab)
- 9cfe57e linter/unicorn: Implement prefer-import-meta-properties rule
(#20662) (Irfan - ئىرفان)
- 1edb391 linter/eslint: Implement `no-restricted-exports` rule (#20592)
(Nicolas Le Cam)
- 0f12bcd linter/react: Implement `hook-use-state` rule (#20986) (Khaled
Labeb)
- 1513a9f oxlint/lsp: Show note field for lsp diagnostic (#20983)
(Sysix)
- 7fdf722 linter/unicorn: Implement `no-useless-iterator-to-array` rule
(#20945) (Mikhail Baev)
- 39c8f2c linter/jest: Implement padding-around-after-all-blocks
(#21034) (Sapphire)
- ac39e51 linter/eslint-vitest-plugin: Prefer importing vitest globals
(#20960) (Said Atrahouch)
- 0b84de1 oxlint: Support allow option for prefer-promise-reject-errors
(#20934) (camc314)
- 23db851 linter/consistent-return: Move rule from nursery to suspicious
(#20920) (camc314)
- 9a27e32 linter/no-unnecessary-type-conversion: Move rule from nursery
to suspicious (#20919) (camc314)
- 1ca7b58 linter/dot-notation: Move rule from nursery to style (#20918)
(camc314)
- 73ba81a linter/consistent-type-exports: Move rule from nursery to
style (#20917) (camc314)
- b9199b1 linter/unicorn: Implement switch-case-break-position (#20872)
(Mikhail Baev)
- 3435ff8 linter: Implements `prefer-snapshot-hint` rule in Jest and
Vitest (#20870) (Said Atrahouch)
- 98510d2 linter: Implement react/prefer-function-component (#19652)
(Connor Shea)
- 871f9d9 linter: Implement no-useless-assignment (#15466) (Zhaoting
Zhou)
- 0f01fbd linter: Implement eslint/object-shorthand (#17688) (yue)

### 🐛 Bug Fixes

- dd2df87 npm: Export package.json for oxlint and oxfmt (#20784) (kazuya
kawaguchi)
- 9bc77dd linter/no-unused-private-class-members: False positive with
await expr (#21067) (camc314)
- 60a57cd linter/const-comparisons: Detect equality contradictions
(#21065) (camc314)
- 2bb2be2 linter/no-array-index-key: False positive when index is passed
as function argument (#21012) (bab)
- 6492953 linter/no-this-in-sfc: Only flag `this` used as member
expression object (#20961) (bab)
- 9446dcc oxlint/lsp: Skip `node_modules` in oxlint config walker
(#21004) (copilot-swe-agent)
- af89923 linter/no-namespace: Support glob pattern matching against
basename (#21031) (bab)
- 64a1a7e oxlint: Don't search for nested config outside base config
(#21051) (Sysix)
- 3b953bc linter/button-has-type: Ignore `document.createElement` calls
(#21008) (Said Atrahouch)
- 8c36070 linter/unicorn: Add support for `Array.from()` for
`prefer-set-size` rule (#21016) (Mikhail Baev)
- c1a48f0 linter: Detect vitest import from vite-plus/test (#20976)
(Said Atrahouch)
- 5c32fd1 lsp: Prevent corrupted autofix output from overlapping text
edits (#19793) (Peter Wagenet)
- ca79960 linter/no-array-index-key: Move span to `key` property
(#20947) (camc314)
- 2098274 linter: Add suggestion for `jest/prefer-equality-matcher`
(#20925) (eryue0220)
- 6eb77ec linter: Allow default-import barrels in import/named (#20757)
(Bazyli Brzóska)
- 9c218ef linter/eslint-vitest-plugin: Remove pending fix status for
require-local-test-context-for-concurrent-snapshot (#20890) (Said
Atrahouch)

### ⚡ Performance

- fb52383 napi/parser, linter/plugins: Clear buffers and source texts
earlier (#21025) (overlookmotel)
- 3b7dec4 napi/parser, linter/plugins: Use `utf8Slice` for decoding
UTF-8 strings (#21022) (overlookmotel)
- 012c924 napi/parser, linter/plugins: Speed up decoding strings in raw
transfer (#21021) (overlookmotel)
- 55e1e9b napi/parser, linter/plugins: Initialize vars as 0 (#21020)
(overlookmotel)
- c25ef02 napi/parser, linter/plugins: Simplify branch condition in
`deserializeStr` (#21019) (overlookmotel)
- 9f494c3 napi/parser, linter/plugins: Raw transfer use
`String.fromCharCode` in string decoding (#21018) (overlookmotel)
- 0503a78 napi/parser, linter/plugins: Faster deserialization of `raw`
fields (#20923) (overlookmotel)
- a24f75e napi/parser: Optimize string deserialization for non-ASCII
sources (#20834) (Joshua Tuddenham)

### 📚 Documentation

- af72b80 oxlint: Fix typo for --tsconfig (#20889) (leaysgur)
- 70c53b1 linter: Highlight that tsconfig is not respected in type aware
linting (#20884) (camc314)
# Oxfmt
### 🚀 Features

- 35cf6e8 oxfmt: Add node version hint for ts config import failures
(#21046) (camc314)

### 🐛 Bug Fixes

- dd2df87 npm: Export package.json for oxlint and oxfmt (#20784) (kazuya
kawaguchi)
- 9d45511 oxfmt: Propagate file write errors instead of panicking
(#20997) (leaysgur)
- 139ddd9 formatter: Handle leading comment after array elision (#20987)
(leaysgur)
- 4216380 oxfmt: Support `.editorconfig` `tab_width` fallback (#20988)
(leaysgur)
- d10df39 formatter: Resolve pending space in fits measurer before
expanded-mode early exit (#20954) (Dunqing)
- f9ef1bd formatter: Avoid breaking after `=>` when arrow body has JSDoc
type cast (#20857) (bab)

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

Labels

A-cli Area - CLI A-linter Area - Linter C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants