Conversation
🦋 Changeset detectedLatest commit: 46e3636 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 |
Merging this PR will not alter performance
Comparing Footnotes
|
6cc3176 to
5c6f564
Compare
c6082cc to
328c0e5
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughA new nursery lint rule Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
328c0e5 to
0abca04
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.changeset/rude-apples-prove.md (1)
5-7: Changeset description is clear and user-focused.The description accurately conveys the rule's purpose and its type-aware behaviour. A minor clarity improvement: consider adding a comma after "strings" to make the compound structure clearer.
📝 Optional clarity tweak
-The rule uses type information, so it only reports on strings and skips array lookups such as `items[0] === "a"`. +The rule uses type information, so it only reports on strings, and skips array lookups such as `items[0] === "a"`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/rude-apples-prove.md around lines 5 - 7, The changeset sentence describing the rule's type-aware behavior needs a comma for clarity: update the sentence that mentions the rule "uses type information" so that the clause reads "so it only reports on strings, and skips array lookups such as `items[0] === \"a\"`" — modify the text referring to useStringStartsEndsWith to insert the comma after "strings" to improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.changeset/rude-apples-prove.md:
- Around line 5-7: The changeset sentence describing the rule's type-aware
behavior needs a comma for clarity: update the sentence that mentions the rule
"uses type information" so that the clause reads "so it only reports on strings,
and skips array lookups such as `items[0] === \"a\"`" — modify the text
referring to useStringStartsEndsWith to insert the comma after "strings" to
improve readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16cea3ab-836c-48e7-b2c0-84534420d57f
⛔ Files ignored due to path filters (14)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/domain_selector.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidCharAt.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidIndex.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidNoFix.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidRegex.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSearch.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSlice.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/valid.ts.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (11)
.changeset/rude-apples-prove.mdcrates/biome_js_analyze/src/lint/nursery/use_string_starts_ends_with.rscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidCharAt.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidIndex.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidNoFix.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidRegex.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSearch.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSlice.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/valid.tscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_string_starts_ends_with.rs
✅ Files skipped from review due to trivial changes (9)
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSlice.ts
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSearch.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidRegex.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidNoFix.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidIndex.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidCharAt.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/valid.ts
- crates/biome_rule_options/src/use_string_starts_ends_with.rs
ematipico
left a comment
There was a problem hiding this comment.
It's a very big rule with a lot of methods, so I am going to approve with some loose suggestions:
- Let's try to docstrings all methods (some aren't, even though they might seem straightforward)
- Let's try to frame/comment on weird logics that aren't framed in the docstrings (I left one comment)
- Let's try using plain language (less technical) in the diagnostics
- Remember to add https://github.com/You-saku as a co-author, because they added the first PR in the first place
0abca04 to
c09b088
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_string_starts_ends_with.rs`:
- Around line 668-687: normalize_binary_expression currently returns
BinaryComparison preserving original left/right; to handle commuted equality
checks, detect when left is a literal/null/number/string or a simple numeric
literal expression (e.g., JsLiteral, JsNullLiteral, numeric/string literal) and
right is a non-literal/complex expression, then swap left and right before
constructing BinaryComparison so the "interesting" expression is always on left;
preserve the negated flag for Inequality/StrictInequality cases and keep using
JsBinaryExpression/operator to decide negation; update
normalize_binary_expression to perform this swap (referencing
normalize_binary_expression, BinaryComparison, JsBinaryExpression, operator,
left, right).
- Around line 361-385: The matcher allows indexOf/lastIndexOf calls with extra
arguments which change semantics; in match_index_of_expression check that
call.arguments list has exactly one argument (i.e., bail out unless the call has
length == 1) before creating the FixPlan, and make the identical change in the
corresponding matcher for lastIndexOf (the other function around lines
~394–421), ensuring you inspect the call variable returned by string_method_call
and return None if the argument count is not exactly one.
- Around line 1109-1138: The EndsWith rewrite is incorrectly matching slice
calls that provide an explicit end bound; update the two EndsWith branches to
require the slice's second argument to be either absent or equal to the string
length. Concretely, in the branch that inspects the minus binary (the one
calling matches_length_minus_value and checking second.is_none()), and in the
later branch that matches left/right against length (the one using
ensure_expression_match and matches_length_expression), add a guard that second
is None OR matches the length member (use
matches_length_expression/ensure_expression_match against
length_member(object.clone())). This ensures the transformation to
PreferredMethod::EndsWith only happens when the slice end is the full string
length (or unspecified).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 227041e3-c380-4624-a79b-235ef072a2f6
⛔ Files ignored due to path filters (14)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/domain_selector.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidCharAt.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidIndex.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidNoFix.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidRegex.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSearch.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSlice.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/valid.ts.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (11)
.changeset/rude-apples-prove.mdcrates/biome_js_analyze/src/lint/nursery/use_string_starts_ends_with.rscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidCharAt.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidIndex.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidNoFix.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidRegex.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSearch.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSlice.tscrates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/valid.tscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_string_starts_ends_with.rs
✅ Files skipped from review due to trivial changes (9)
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSearch.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidRegex.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidNoFix.ts
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidSlice.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/valid.ts
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidIndex.ts
- crates/biome_rule_options/src/use_string_starts_ends_with.rs
- crates/biome_js_analyze/tests/specs/nursery/useStringStartsEndsWith/invalidCharAt.ts
Co-authored-by: You-saku <[email protected]>
c09b088 to
46e3636
Compare
Summary
This adds
useStringStartsEndsWith, which is a port of https://typescript-eslint.io/rules/prefer-string-starts-ends-with/I chose to use type info (like the source rule) because my hunch is that there would be way too many false positives otherwise. Also, I chose to split the invalid tests into a bunch of files because the rule is very complex, and it makes it easier to spot whether diagnostics are getting emitted.
generated by gpt 5.4, but heavily guided to stop it from doing dumb things.
closes #7653
supersedes and closes #8582
Test Plan
all the tests are from the source rule, excluding the ones depending on the source rule's option
Docs