fix(lint): fix false positive in noMisleadingReturnType for nested object widening#10006
Conversation
🦋 Changeset detectedLatest commit: 952eef2 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 |
github-actions
bot
added
A-Linter
minseong0324
force-pushed
the
fix/no-misleading-return-type-nested-object-widening
branch
3 times, most recently
from
dbb2fd6 to
5b79e61
Compare
|
|
||
| while let Some((ann, inf)) = stack.pop() { | ||
| iterations += 1; | ||
| if iterations > 50 { |
There was a problem hiding this comment.
Same iteration guard as is_nonunion_wider. Prevents infinite loops on cyclic types like interface Node { parent: Node }. 50 is a generous ceiling - real code never nests anywhere near that deep.
There was a problem hiding this comment.
Don't use magic numbers though. Create a constant and document
There was a problem hiding this comment.
is_nonunion_wider already uses the same 50 for the same iteration guard. Extract both into one shared constant in this PR, or split it into a separate cleanup PR?
There was a problem hiding this comment.
@ematipico 952eef2 extracts both into a shared constant. Let me know if you'd rather have a separate PR for it.
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
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:
WalkthroughRefactors 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/tidy-planes-stay.md:
- Line 5: Update the changeset description in .changeset/tidy-planes-stay.md to
include a link to the referenced issue (`#9810`); edit the single-line summary
("Fixed `noMisleadingReturnType` incorrectly flagging nested object literals
with widened properties.") or the body to append the issue URL (e.g. "See:
https://github.com/<org>/<repo>/issues/9810" or the appropriate repo link) so
the bug-fix changeset contains the issue reference per guidelines.
🪄 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: 20347756-606a-44d8-a0de-25b550290520
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/tidy-planes-stay.mdcrates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rscrates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts
minseong0324
force-pushed
the
fix/no-misleading-return-type-nested-object-widening
branch
from
5b79e61 to
65f01b5
Compare
minseong0324
force-pushed
the
fix/no-misleading-return-type-nested-object-widening
branch
from
65f01b5 to
e79b09d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rs (1)
467-495: Optional: index-signature branch doesn't recurse into nested value types.Unlike the named-member branch below (which pushes mismatched pairs onto the stack), the index-signature path only accepts matches via
types_matchoris_base_type_of_literal. So an annotation likeRecord<string, { x: string }>against an inferred{ a: { x: "y" } }would bail out here rather than being recognised as pure literal widening. Not a regression introduced by this PR and arguably outside its scope — just flagging for a potential follow-up so the two branches stay symmetrical.🤖 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 467 - 495, The index-signature branch (starting at annotated_index_signature and using index_signature_value_type) currently only accepts direct matches via types_match or is_base_type_of_literal and therefore doesn't recurse into nested value types; update this branch to mirror the named-member logic by, when a mismatch is found between index_signature_value_type and an inferred_type (and it isn't a simple base/literal match), push the pair (index_signature_value_type, inferred_type) onto the same comparison stack (or invoke the same recursion routine used by the named-member branch) so nested object literal widening (e.g., Record<string, { x: string }> vs { a: { x: "y" } }) is recognized; keep the existing index_signature_has_widening handling for base-of-literal cases and only return false if the pushed comparisons ultimately fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rs`:
- Around line 467-495: The index-signature branch (starting at
annotated_index_signature and using index_signature_value_type) currently only
accepts direct matches via types_match or is_base_type_of_literal and therefore
doesn't recurse into nested value types; update this branch to mirror the
named-member logic by, when a mismatch is found between
index_signature_value_type and an inferred_type (and it isn't a simple
base/literal match), push the pair (index_signature_value_type, inferred_type)
onto the same comparison stack (or invoke the same recursion routine used by the
named-member branch) so nested object literal widening (e.g., Record<string, {
x: string }> vs { a: { x: "y" } }) is recognized; keep the existing
index_signature_has_widening handling for base-of-literal cases and only return
false if the pushed comparisons ultimately fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aee22f9e-80cb-49ac-8c81-f9330b5245c2
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/tidy-planes-stay.mdcrates/biome_js_analyze/src/lint/nursery/no_misleading_return_type.rscrates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts
✅ Files skipped from review due to trivial changes (2)
- .changeset/tidy-planes-stay.md
- crates/biome_js_analyze/tests/specs/nursery/noMisleadingReturnType/valid.ts
minseong0324
force-pushed
the
fix/no-misleading-return-type-nested-object-widening
branch
from
e79b09d to
9425f15
Compare
minseong0324
force-pushed
the
fix/no-misleading-return-type-nested-object-widening
branch
from
99f712b to
38a6526
Compare
2 participants
I used Claude Code to assist with this implementation.
Summary
Addresses items from #9810: Nested object widening
Test Plan
Snapshot tests.