feat(lint): detect tuple widening and inferred unions in noMisleadingReturnType#9843
Conversation
|
Merging this PR will not alter performance
Comparing Footnotes
|
|
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 selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR extends the Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.changeset/afraid-items-begin.md (1)
5-5: Minor: sentence should end with a full stop.Per coding guidelines, changeset descriptions should end every sentence with a full stop. Also consider linking to the rule documentation for discoverability.
Suggested fix
-`noMisleadingReturnType` now detects tuple element widening and ternary-inferred unions. +[`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) now detects tuple element widening and ternary-inferred unions.As per coding guidelines: "End every sentence in changeset descriptions with a full stop (period)" and "for rule references, include links to the rule documentation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/afraid-items-begin.md at line 5, Update the changeset sentence for `noMisleadingReturnType` so it ends with a full stop and add a discoverability link to the rule documentation; specifically, edit the line containing "`noMisleadingReturnType` now detects tuple element widening and ternary-inferred unions." to append a period and include a short link/reference to the `noMisleadingReturnType` rule docs (e.g., in parentheses) immediately after the sentence.crates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rs (1)
811-823: Tuple comparison ignoresis_optionalandis_restflags.Per
TupleElementTypeincrates/biome_js_type_info/src/type_data.rs:915–918, tuple elements haveis_optionalandis_restfields. The current pairwise comparison only checks element types and length, so[string?, number]and[string, number]would be treated as structurally compatible. For theas constwidening use case this is likely acceptable (const tuples don't typically use optional/rest elements), but worth noting if the rule scope expands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rs` around lines 811 - 823, The tuple comparison in the TypeData::Tuple arm only compares element types and lengths, ignoring TupleElementType flags is_optional and is_rest; update the loop that iterates ann_tuple.elements() and inf_tuple.elements() to also check these flags for each paired element (e.g., require ann_e.is_optional == inf_e.is_optional and ann_e.is_rest == inf_e.is_rest, or implement the intended compatibility logic) and return false on mismatch before resolving types and pushing to stack so tuples like [string?, number] are not treated as identical to [string, number].
🤖 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/afraid-items-begin.md:
- Line 5: Update the changeset sentence for `noMisleadingReturnType` so it ends
with a full stop and add a discoverability link to the rule documentation;
specifically, edit the line containing "`noMisleadingReturnType` now detects
tuple element widening and ternary-inferred unions." to append a period and
include a short link/reference to the `noMisleadingReturnType` rule docs (e.g.,
in parentheses) immediately after the sentence.
In `@crates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rs`:
- Around line 811-823: The tuple comparison in the TypeData::Tuple arm only
compares element types and lengths, ignoring TupleElementType flags is_optional
and is_rest; update the loop that iterates ann_tuple.elements() and
inf_tuple.elements() to also check these flags for each paired element (e.g.,
require ann_e.is_optional == inf_e.is_optional and ann_e.is_rest ==
inf_e.is_rest, or implement the intended compatibility logic) and return false
on mismatch before resolving types and pushing to stack so tuples like [string?,
number] are not treated as identical to [string, number].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a841b04-4578-44b7-b22b-1b73f17ca45d
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/afraid-items-begin.mdcrates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rscrates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/invalid.tscrates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts
0014498 to
c83e54f
Compare
There was a problem hiding this comment.
no changeset needed, we haven't released the rule
I used Claude Code to assist with this implementation.
Summary
Addresses items from #9810: adds tuple element widening and inferred-union handling to
noMisleadingReturnType.Test Plan
Snapshot tests.
Docs