-
-
Notifications
You must be signed in to change notification settings - Fork 808
fix(lint): enable rules via dependencies when setting domains in config #8658
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
🦋 Changeset detectedLatest commit: a77e34a 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 |
WalkthroughThis PR adds a changeset and tests, and changes the lint evaluation flow so manifest domains are consulted per-rule: rules whose domain appears in the manifest's domains list are skipped rather than gated by an empty/absent-domains check. Dependency-based enabling still applies for non-excluded domains; the "only" filter early exit remains. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-12-22T09:27:13.161ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
🧬 Code graph analysis (1)crates/biome_service/src/file_handlers/mod.rs (2)
🔇 Additional comments (1)
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 |
| let domains = self.settings.as_linter_domains(path); | ||
| if let Some(manifest) = &self.package_json { | ||
| for domain in R::METADATA.domains { | ||
| // handled in `record_rule_from_domains` |
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.
This comment doesn't seem to provide much value. Handled what?
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.
I mean, it handles the case where the condition for entering the continue branch is met. Just deleted it.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_service/src/file_handlers/mod.rs (1)
1375-1396: Hoistas_linter_domainscall to avoid repeated pattern matching per rule.The per-domain logic is sound, but
as_linter_domainsrecalculates for every rule since it iterates through all override patterns. Move this call once toLintVisitor::new()(around line 1906–1913), store the result as a field, and reuse it inrecord_rule_from_manifest.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_rules_via_dependencies_but_disable_domain_from_config.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/nice-fans-beg.mdcrates/biome_cli/tests/cases/rules_via_dependencies.rscrates/biome_service/src/file_handlers/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/nice-fans-beg.md
- crates/biome_cli/tests/cases/rules_via_dependencies.rs
🧰 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_service/src/file_handlers/mod.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: When a rule is recommended and has domains, it is enabled only when the user enables the relative domains
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Rules can optionally be assigned to domains such as `RuleDomain::Test` to indicate they belong to specific concepts like testing frameworks
📚 Learning: 2025-12-22T09:27:13.161Z
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:27:13.161Z
Learning: In crates/biome_analyze/**/*analyze/src/**/*.rs, the `fix_kind` field in `declare_lint_rule!` should only be specified when the rule implements the `action` function. Rules that only emit diagnostics without providing code fixes should not include `fix_kind` in their metadata.
Applied to files:
crates/biome_service/src/file_handlers/mod.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_service/src/file_handlers/mod.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 `no` prefix when the rule's sole intention is to forbid a single concept
Applied to files:
crates/biome_service/src/file_handlers/mod.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 : When porting rules from other linters, use `sources` metadata with `RuleSource::Eslint().same()` for identical behavior or `.inspired()` for different behavior
Applied to files:
crates/biome_service/src/file_handlers/mod.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_service/src/file_handlers/mod.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 : The first paragraph of rule documentation must be written in a single line to ensure proper rendering in the rules overview table
Applied to files:
crates/biome_service/src/file_handlers/mod.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_service/src/file_handlers/mod.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_service/src/file_handlers/mod.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 : Implement the `action` function and add `fix_kind` metadata to the rule macro if the rule provides code actions
Applied to files:
crates/biome_service/src/file_handlers/mod.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/biome_rule_options/lib/**/*.rs : Apply `#[serde(deny_unknown_fields)]` to rule option structs to enforce strict configuration validation
Applied to files:
crates/biome_service/src/file_handlers/mod.rs
🧬 Code graph analysis (1)
crates/biome_service/src/file_handlers/mod.rs (2)
crates/biome_analyze/src/rule.rs (1)
domains(649-652)crates/biome_configuration/src/analyzer/mod.rs (1)
domain(578-578)
🔇 Additional comments (1)
crates/biome_service/src/file_handlers/mod.rs (1)
1371-1373: Nice fix – the dual-gate has been properly removed.The early return now only checks for "only" filters, removing the previous gate that blocked manifest-based enabling when any domains were configured. This allows the subsequent per-domain logic to handle each domain individually.
Summary
Currently, Biome does not check the
package.jsonfile whendomainsis set in config. For example, with the following config:{ "linter": { "domains": { "project": "recommended" } } }, even if the
reactdependency exists inpackage.json, the React domain is not enabled. I think this behavior is incorrect, enabling one domain should not affect another.Also, when deriving the test case from
enables_rules_via_dependencies_but_disable_rule_from_config, I found that setting thereactdependency tolatestinpackage.jsondoes not enable the React domain. Is this the expected behavior?Test Plan
Added new test
enables_rules_via_dependencies_when_setting_domains_in_config.Docs