-
-
Notifications
You must be signed in to change notification settings - Fork 791
fix(noUnusedImports): parse also linkcode, and not only link #8088
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
Conversation
🦋 Changeset detectedLatest commit: 9fe4427 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 |
WalkthroughThe JSDoc/TSDoc inline-tag regex in the noUnusedImports lint was extended to recognise the inline link variants { Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/type_info.rs : Add new TypeScript type support by extending the TypeData enum rather than introducing parallel structures.
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/{type_info,local_inference,resolver,flattening}.rs : Avoid recursive type structures and cross-module Arcs; represent links between types using TypeReference and TypeData::Reference.
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/collector.rs : Thin (module-level) inference should resolve local and global bindings to TypeReference::Resolved, mark imported bindings as TypeReference::Import, and fall back to TypeReference::Unknown if unresolved.
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code; if a mandatory token/node is missing, return `None` instead
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import and use the `FormatNode` trait for AST nodes
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
Outdated
Show resolved
Hide resolved
ematipico
left a 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.
The template of the issue should contain all the information to create a PR. I'll sum it up here:
- we need a changeset that explains the fix to end-users
- we need new tests to verify that the fix works
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
Outdated
Show resolved
Hide resolved
a9c94af to
ff28068
Compare
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: 2
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (1)
167-168: Add test coverage for{@see ...}inline tag.The regex includes
seein the alternation, but neither test file exercises this pattern. Consider adding a test case using{@see SomeType}to verify the regex correctly prevents unused import warnings for this tag variant as well.Based on the existing test file structure, you could add:
import type LinkOnSeeTag from "mod"; /** * {@see LinkOnSeeTag} */ function testSeeTag() { }
📜 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/correctness/noUnusedImports/issue_7876_tsdoc_linkcode.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkplain.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkcode.ts(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkplain.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/type_info.rs : Add new TypeScript type support by extending the TypeData enum rather than introducing parallel structures.
Applied to files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkplain.tscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:25:05.698Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:25:05.698Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in ../biome_lsp/src/server.tests.rs
Applied to files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkplain.ts
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/{type_info,local_inference,resolver,flattening}.rs : Avoid recursive type structures and cross-module Arcs; represent links between types using TypeReference and TypeData::Reference.
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code; if a mandatory token/node is missing, return `None` instead
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import and use the `FormatNode` trait for AST nodes
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:22:15.851Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:15.851Z
Learning: Applies to crates/biome_formatter/src/**/*.rs : After generation, remove usages of `format_verbatim_node` and implement real formatting with biome_formatter utilities
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/collector.rs : Thin (module-level) inference should resolve local and global bindings to TypeReference::Resolved, mark imported bindings as TypeReference::Import, and fall back to TypeReference::Unknown if unresolved.
Applied to files:
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (1)
168-168: Regex pattern correctly expanded to supportlinkcodeandlinkplaintags.The addition of
linkcodeandlinkplainto the inline tag regex properly addresses issue #7876 and aligns with the JSDoc specification for inline link tags.
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkcode.ts
Show resolved
Hide resolved
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/issue_7876_tsdoc_linkplain.ts
Show resolved
Hide resolved
ff28068 to
131cfc6
Compare
|
@db295 we still need a changeset |
CodSpeed Performance ReportMerging #8088 will not alter performanceComparing Summary
Footnotes
|
45c39e7 to
d81cb50
Compare
d81cb50 to
394b855
Compare
|
@db295 it seems that you still miss something. You added new tests, but they are usually accompanied with snapshots being added or updated, this isn't the case. I don't want to sound like a broken a record, but the contribution guide actually explains this part. |
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)
.changeset/forty-hornets-burn.md (1)
5-16: Documentation is clear; consider showing both new tags in the example.The description accurately documents the fix and correctly references issue #7876. The code example effectively demonstrates @linkcode usage, but since both @linkcode and @linkplain are now supported, you might add a brief note or second example showing @linkplain for symmetry—though this is optional.
Also, ensure the test snapshots for the new test files (mentioned in your earlier changes) are generated and committed, as noted in prior feedback.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/forty-hornets-burn.md(1 hunks)
🔇 Additional comments (1)
.changeset/forty-hornets-burn.md (1)
1-3: Changeset format is correct.The YAML frontmatter properly specifies the package name and patch-level bump.
ematipico
left a 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.
The snapshot is there! I thought it was a typescript file 😓 thank you @db295 for the fix!
…#8088) Co-authored-by: D'or Banoun <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Summary
Fixes #7876
Test Plan
Docs