fix: correct error message in useIndexOf rule#7234
Conversation
Fixes biomejs#7233 The error message was incorrectly suggesting to prefer findIndex() over indexOf(), when it should suggest the opposite. Updated the diagnostic message to correctly recommend using indexOf()/lastIndexOf() over findIndex()/findLastIndex().
🦋 Changeset detectedLatest commit: 9d78a13 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. WalkthroughThe PR updates the diagnostic text in crates/biome_js_analyze/src/lint/complexity/use_index_of.rs to state a preference for Array#indexOf/lastIndexOf over Array#findIndex/findLastIndex when looking for the index of an item. Only the diagnostic wording and punctuation were changed; no logic, control flow or public signatures were modified. Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
A-Linter
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/complexity/use_index_of.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Format code (Rust + TOML) using
just formatFormat Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/complexity/use_index_of.rs
crates/biome_*/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Core crates must be located in
/crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/use_index_of.rs
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/complexity/use_index_of.rs (1)
178-182: Diagnostic wording now matches rule intent (indexOf over findIndex).Swapping
new_member_name/old_member_namefixes the message to preferindexOf/lastIndexOfoverfindIndex/findLastIndex. The note text and the action (Lines 214–215) remain consistent with this change. Docs at Lines 17–23 also align. Nice tidy fix.
| "Prefer "<Emphasis>"Array#"{new_member_name}"()"</Emphasis>" over "<Emphasis>"Array#"{old_member_name}"()"</Emphasis>" when looking for the index of an item." | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify snapshots and references to the old phrasing.
To be safe, scan for any stale snapshots or docs still asserting the old message text and update them if needed. Also, please run the formatter as per repo conventions.
Run this from the repo root:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for old, now-incorrect diagnostics…"
rg -n -C2 -P 'Prefer\s+Array#find(Index|LastIndex)\(\)\s+over\s+Array#(last)?indexOf\(\)'
echo
echo "Searching for the new, correct diagnostics…"
rg -n -C2 -P 'Prefer\s+Array#(last)?indexOf\(\)\s+over\s+Array#find(Last)?Index\(\)'
echo
echo "Reminder: format Rust/TOML before commit:"
echo " just format"Length of output: 3517
Update stale snapshots & run formatter
The snapshots under crates/biome_js_analyze/tests/specs/complexity/useIndexOf/invalid.js.snap still contain the old message:
i Prefer Array#findIndex() over Array#indexOf() when looking for the index of an item.
Please update those lines to:
i Prefer Array#indexOf() over Array#findIndex() when looking for the index of an item.
Then run:
just format
🤖 Prompt for AI Agents
In crates/biome_js_analyze/src/lint/complexity/use_index_of.rs around lines
178-179, the emitted message text was inverted; update the snapshot(s) under
crates/biome_js_analyze/tests/specs/complexity/useIndexOf/invalid.js.snap to
replace the old line "i Prefer Array#findIndex() over Array#indexOf() when
looking for the index of an item." with "i Prefer Array#indexOf() over
Array#findIndex() when looking for the index of an item." and then run the
formatter with the command `just format`.
|
Thank you @JeetuSuthar This change will trigger the update of testing snapshots. Can you please update them? The contribution guide explains how |
Updated test snapshots to reflect the corrected error message that now properly suggests using indexOf()/lastIndexOf() over findIndex()/findLastIndex().
|
@ematipico Thank you for the guidance! I've updated the test snapshots in the latest commit. The snapshots now reflect the corrected error message that properly suggests using Changes made:
|
CodSpeed Performance ReportMerging #7234 will not alter performanceComparing Summary
Footnotes |
Remove extra backticks at the end of error messages in the test snapshots to match the exact format expected by the test runner.
|
Thank you. One last thing is the changelog, as the bot showed. The contribution guide shows everything |
|
@ematipico |
|
@ematipico Should I proceed without a changeset, or would you prefer one for this fix? |
|
@JeetuSuthar Fixing an error message is still an user-facing change, as users can observe the diagnostic message has been changed. So we would prefer adding a changeset. |
|
@siketyan I apologize for the confusion earlier. I've now added the changeset as requested. |
3 participants
Fixes #7233
The error message was incorrectly suggesting to prefer findIndex() over indexOf(), when it should suggest the opposite. Updated the diagnostic message to correctly recommend using indexOf()/lastIndexOf() over findIndex()/findLastIndex().
Summary
Fixed incorrect error message in the
useIndexOflint rule. The diagnostic was showing "Prefer Array#findIndex() over Array#indexOf()" when it should show "Prefer Array#indexOf() over Array#findIndex()". This was a simple variable swap in the diagnostic message template.Test Plan
The fix swaps two variables in the diagnostic message string. The existing test snapshots will catch this change and verify the message is now correct. GitHub CI will validate the build and tests.