feat(linter): no useless assignment

[zzt1224]

No description provided.

[graphite-app]

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.

[Copilot]

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.

[zzt1224]

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]

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]

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

[zzt1224]

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]

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 :)

[zzt1224]

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.

[camc314]

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]

CodSpeed Performance Report

Merging #15466 will degrade performances by 5.65%

_{Comparing zzt1224:feat/no-useless-assignment (86ede81) with main (adbdc62)}

Summary

❌ 2 regressions
✅ 2 untouched
⏩ 41 skipped¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Mode	Benchmark	BASE	HEAD	Change
❌	Simulation	linter[cal.com.tsx]	474.7 ms	495.4 ms	-4.18%
❌	Simulation	linter[react.development.js]	19.6 ms	20.8 ms	-5.65%

Footnotes

1. 41 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. ↩

[zzt1224]

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]

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]

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]

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.

[overlookmotel]

I've only reviewed the changes to oxc_allocator crate - that's my domain. A couple of comments below.

In answer to your question, yes it's totally "allowed" to use the BitSet type. Everything in oxc_allocator crate is intended to be reused wherever it's useful.

I'm not actually sure if it's the best data structure to use here - might be better to use a heap-allocated bitset rather than an arena-allocated one (there are crates for that).

But the additions you've made to BitSet type are good regardless. It'd be ideal (if you have time) to make the changes to oxc_allocator in a separate PR. I'd like to get that part merged ASAP, while the rest is waiting on review. Promise to merge it very swiftly!

[zzt1224]

I've only reviewed the changes to oxc_allocator crate - that's my domain. A couple of comments below.

In answer to your question, yes it's totally "allowed" to use the BitSet type. Everything in oxc_allocator crate is intended to be reused wherever it's useful.

I'm not actually sure if it's the best data structure to use here - might be better to use a heap-allocated bitset rather than an arena-allocated one (there are crates for that).

But the additions you've made to BitSet type are good regardless. It'd be ideal (if you have time) to make the changes to oxc_allocator in a separate PR. I'd like to get that part merged ASAP, while the rest is waiting on review. Promise to merge it very swiftly!

Yep, on it! Separate PR coming.

[overlookmotel]

Yep, on it! Separate PR coming.

Thank you. Please ping me here when you've posted PR, or assign me as reviewer on the PR. My GitHub notifications are a snowdrift! Doing either of those 2 will help it cut through the snow.