-
-
Notifications
You must be signed in to change notification settings - Fork 805
fix(linter): differentiate static/instance methods in useAdjacentOverloadSignatures #8670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…OverloadSignatures Fixed biomejs#8345: useAdjacentOverloadSignatures no longer reports false positives for static and instance methods with the same name. Static methods and instance methods are now treated as separate overload groups.
🦋 Changeset detectedLatest commit: 014b48e 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 |
CodSpeed Performance ReportMerging #8670 will not alter performanceComparing Summary
Footnotes
|
- Keep static/instance in key without ignoring interleaving members - Preserve private names (#f) vs public names - Add invalid coverage + update snapshots
WalkthroughThis PR fixes a false-positive in the Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs (1)
193-203: Consider adding rustdoc forClassMethodKey.The struct is well-designed, but a brief rustdoc comment would clarify its purpose: "Represents a class method identity including both its name and whether it's static. Used to distinguish static and instance methods with the same name as separate overload groups."
🔎 Suggested rustdoc
+/// Key for tracking class method overloads. +/// +/// Combines the method name with a static flag to ensure static and instance +/// methods with the same name are treated as separate overload groups. #[derive(Clone, Eq, PartialEq, Hash)] struct ClassMethodKey { name: ClassMemberName, is_static: bool, }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/fix-adjacent-overload-static.mdcrates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rscrates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/invalid.tscrates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/valid.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
🧠 Learnings (17)
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/js_module_info/collector.rs : Implement module-level (thin) inference to resolve `TypeReference::Qualifier` variants by looking up declarations in module scopes and handling import statements
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_node_union!` macro to query multiple node types together to avoid redundant traversal passes
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Avoid string allocations by using `&str` or `TokenText` instead of `to_string()`
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use the `Semantic<T>` query type to access semantic information about bindings, references, and scope within a rule
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `Markup!` macro for diagnostic messages and code action descriptions to ensure proper formatting
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_lint_rule!` macro with a `version` field set to `next` for new rules
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Check if a variable is global using the semantic model before reporting diagnostics for rules that ban global functions or variables
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Deprecated rules must include a `deprecated` field in the `declare_lint_rule!` macro with an explanation of what rule to use instead
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Invalid code snippets in rule documentation must emit exactly one diagnostic
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Rule names should use the `use` prefix when the rule's sole intention is to mandate a single concept
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Code blocks in rule documentation must specify a language identifier and be tagged with `expect_diagnostic` for invalid examples or remain untagged for valid examples
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `rule_category!()` macro to refer to the diagnostic category instead of dynamically parsing its string name
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Mark rules with `issue_number` in the `declare_lint_rule!` macro if they are work-in-progress to indicate missing features
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
📚 Learning: 2025-12-22T09:26:56.943Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:26:56.943Z
Learning: When defining lint rules (declare_lint_rule!), only specify fix_kind if the rule implements an action(...) function. Rules that only emit diagnostics without a code fix should omit fix_kind. This applies to all Rust lint rule definitions under crates/.../src/lint (e.g., crates/biome_js_analyze/src/lint/...).
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/invalid.ts (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
F(143-143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test Node.js API
🔇 Additional comments (5)
crates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/invalid.ts (1)
55-67: Excellent test coverage for the fix.Classes E and F verify that the rule still correctly flags non-adjacent overloads within the same group (instance or static) whilst treating static and instance as separate groups. Class E validates instance overload violations, Class F validates static overload violations.
crates/biome_js_analyze/src/lint/suspicious/use_adjacent_overload_signatures.rs (2)
213-251: Well-implemented static/instance distinction.Both
JsMethodClassMemberandTsMethodSignatureClassMembercode paths correctly extract the static modifier usingBitFlags<Modifier>and constructClassMethodKeywith the appropriate flag. The consistent handling across both member types ensures comprehensive coverage.
317-334: Elegant generic solution.Making
check_methodgeneric overT: Clone + Eq + Hash + Into<TokenText>is clever—it allows the function to work seamlessly with both plainTokenText(for interfaces, types) andClassMethodKey(for classes) without code duplication. TheInto<TokenText>bound ensures diagnostics can still display just the method name.crates/biome_js_analyze/tests/specs/suspicious/useAdjacentOverloadSignatures/valid.ts (1)
74-102: Comprehensive test coverage with clear documentation.The four new classes thoroughly validate the fix:
- D & E: Basic static/instance separation in different orders
- F: Static overloads remain valid when adjacent
- G: Instance overloads valid with static method following
The comments clearly link to issue #8345 and explain the expected behaviour.
.changeset/fix-adjacent-overload-static.md (1)
1-13: Clear and well-documented changelog entry.The changeset succinctly explains the fix with a practical example. The reference to issue #8345 and the rule documentation link provide useful context for users.
Summary
Fixed #8345:
useAdjacentOverloadSignaturesno longer reports false positives for static and instance methods with the same name.The rule was incorrectly treating static methods and instance methods with the same name as belonging to the same overload group:
Static and instance methods are now tracked separately, so they're treated as independent overload groups.
Test plan
valid.tscargo test -p biome_js_analyze use_adjacent_overload_signaturesAI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.