Skip to content

fix(linter/unicorn): fix ASI hazard in prefer-spread rule fixer#16440

Merged
graphite-app[bot] merged 1 commit intomainfrom
c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer
Dec 9, 2025
Merged

fix(linter/unicorn): fix ASI hazard in prefer-spread rule fixer#16440
graphite-app[bot] merged 1 commit intomainfrom
c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer

Conversation

@camc314
Copy link
Contributor

@camc314 camc314 commented Dec 3, 2025

🤖 generated with help from Claude Opus 4.5

@github-actions github-actions bot added A-linter Area - Linter C-bug Category - Bug labels Dec 3, 2025
Copy link
Contributor Author

camc314 commented Dec 3, 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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@camc314 camc314 self-assigned this Dec 3, 2025
@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch 2 times, most recently from aa56e91 to 091d331 Compare December 3, 2025 11:50
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 3, 2025

CodSpeed Performance Report

Merging #16440 will not alter performance

Comparing c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer (b98310b) with main (5b10825)

Summary

✅ 4 untouched
⏩ 41 skipped1

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.

@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from 091d331 to 4147346 Compare December 3, 2025 13:11
@camc314 camc314 force-pushed the c/12-03-feat_semantic_expose_get_comment_at_method branch 2 times, most recently from b404099 to e68269a Compare December 3, 2025 13:27
@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from 4147346 to fd61773 Compare December 3, 2025 13:27
@camc314 camc314 marked this pull request as ready for review December 3, 2025 13:33
@camc314 camc314 requested a review from overlookmotel December 3, 2025 13:33
@camc314
Copy link
Contributor Author

camc314 commented Dec 3, 2025

@overlookmotel assigned this to you to review as I'd appreciate another set of eyes. All good if you don't have time though - thanks

@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from fd61773 to 21c674f Compare December 3, 2025 13:47
@camc314 camc314 force-pushed the c/12-03-feat_semantic_expose_get_comment_at_method branch 2 times, most recently from e7f423e to 206868c Compare December 3, 2025 13:48
@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from 21c674f to 4867ee2 Compare December 3, 2025 13:48
@graphite-app graphite-app bot changed the base branch from c/12-03-feat_semantic_expose_get_comment_at_method to graphite-base/16440 December 3, 2025 14:21
@camc314 camc314 force-pushed the graphite-base/16440 branch from 206868c to 8274f63 Compare December 3, 2025 14:22
@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from 4867ee2 to 801fc7e Compare December 3, 2025 14:22
@camc314 camc314 changed the base branch from graphite-base/16440 to c/12-03-feat_semantic_expose_get_comment_at_method December 3, 2025 14:22
@graphite-app graphite-app bot changed the base branch from c/12-03-feat_semantic_expose_get_comment_at_method to graphite-base/16440 December 3, 2025 14:29
@graphite-app graphite-app bot force-pushed the graphite-base/16440 branch from 8274f63 to 8c10694 Compare December 3, 2025 14:36
@graphite-app graphite-app bot force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from 801fc7e to ee8ab4a Compare December 3, 2025 14:36
@graphite-app graphite-app bot changed the base branch from graphite-base/16440 to main December 3, 2025 14:36
@graphite-app graphite-app bot force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from ee8ab4a to cda3bce Compare December 3, 2025 14:37
Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

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

My god this is tricky. It would be so much simpler and less error-prone if we had tokens to look at.

A couple more chars need adding to the check, I think, and a few notes on perf.

I've only nitpicked on perf because presumably this function is going to need to be used in lots of other rules too.

Copilot AI review requested due to automatic review settings December 9, 2025 10:44
@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from cda3bce to bf593c7 Compare December 9, 2025 10:44
@camc314 camc314 force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from 19f94eb to b98310b Compare December 9, 2025 10:46
Copy link
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 fixes an ASI (Automatic Semicolon Insertion) hazard in the prefer-spread linter rule's auto-fixer. The fixer now detects when replacing code like Array.from(set) with [...set] could cause parsing issues due to JavaScript's ASI rules, and adds a semicolon prefix when needed (e.g., foo()\nArray.from(set) becomes foo()\n;[...set]).

Key Changes:

  • Added could_be_asi_hazard() function to detect when a semicolon is needed before inserting code that starts with [
  • Updated prefer-spread rule fixer to check for ASI hazards and add semicolons when necessary
  • Added comprehensive test coverage for various ASI hazard scenarios including Unicode identifiers, comments, and irregular whitespace

Reviewed changes

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

File Description
crates/oxc_linter/src/ast_util.rs Implements ASI hazard detection with could_be_asi_hazard() and helper function find_last_meaningful_char() to scan backwards through source text, skipping whitespace and comments
crates/oxc_linter/src/rules/unicorn/prefer_spread.rs Threads node parameter through to fixer functions and adds ASI hazard checks to all auto-fixes for Array.from(), array.slice(), array.toSpliced(), and string.split()

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

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 9, 2025
Copy link
Contributor Author

camc314 commented Dec 9, 2025

Merge activity

@graphite-app graphite-app bot force-pushed the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch from b98310b to 678e43b Compare December 9, 2025 11:12
@graphite-app graphite-app bot merged commit 678e43b into main Dec 9, 2025
20 checks passed
@graphite-app graphite-app bot deleted the c/12-03-fix_linter_unicorn_fix_asi_hazard_in_prefer-spread_rule_fixer branch December 9, 2025 11:17
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 9, 2025
Copilot AI pushed a commit that referenced this pull request Dec 10, 2025
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants