feat(organizeImports): add kind field to import matcher#10074
Conversation
Introduce a `:BARE:` predefined group matcher for the `organizeImports`
assist action. It selects bare (side-effect) imports such as
`import "polyfill"` and composes with the existing `groups` option and
`sortBareImports: true` to express orderings such as "place all
side-effect imports at the end of the import list":
{
"sortBareImports": true,
"groups": [
["**", "!:BARE:"],
":BLANK_LINE:",
[":BARE:"]
]
}
It can also be combined with other predefined matchers (`:NODE:`,
`:PACKAGE:`, `:STYLE:`, etc.) to express richer orderings in a single
`groups` array, without introducing a dedicated top-level option.
Implementation mirrors the recently-added `:STYLE:` matcher: a new
`PredefinedSourceMatcher::Bare` variant plus an `is_bare` flag threaded
through `ImportSourceCandidate`. Bare imports only participate in group
matching when `sortBareImports` is enabled, preserving today's default
chunk-preserving behavior.
🦋 Changeset detectedLatest commit: 92e7f51 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 extends the Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_rule_options/src/no_restricted_imports.rs (1)
250-255:⚠️ Potential issue | 🟠 MajorPreserve
:BARE:semantics for restricted-import groups.Line 254 always passes
false, sopatterns[].group: [":BARE:"]can never match a real static side-effect import innoRestrictedImports, even though this path uses the sharedSourcesMatcher. Please either derive bare-ness here, or reject:BARE:for this rule if it is meant to stay organise-imports-only.🐛 Proposed fix
if let Some(group) = &self.group { let source = ImportSource::from(ComparableToken { token: import_source_text.clone(), }); - let candidate = ImportSourceCandidate::new(&source, false); + let is_bare = matches!( + node, + AnyJsImportLike::JsModuleSource(module_source_node) + if matches!( + module_source_node.syntax().parent().map(|clause| clause.kind()), + Some(JsSyntaxKind::JS_IMPORT_BARE_CLAUSE) + ) + ); + let candidate = ImportSourceCandidate::new(&source, is_bare); if group.is_match(&candidate) {Please add a
noRestrictedImportsfixture coveringgroup: [":BARE:"]as well; the organise-import snapshots will not catch this shared matcher path. As per coding guidelines, “All code changes MUST include appropriate tests”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_rule_options/src/no_restricted_imports.rs` around lines 250 - 255, The call to ImportSourceCandidate::new(&source, false) always sets the "bare" flag to false so groups containing ":BARE:" can never match; change this to derive the bare-ness from the import token (e.g. detect a side-effect/bare import from ComparableToken/import_source_text or existing helpers) and pass the correct boolean into ImportSourceCandidate::new (or alternatively explicitly reject ":BARE:" for this rule if intended), and add a new noRestrictedImports fixture that includes group: [":BARE:"] to cover this path; update references: ImportSource, ComparableToken, ImportSourceCandidate, and group.is_match accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/biome_rule_options/src/no_restricted_imports.rs`:
- Around line 250-255: The call to ImportSourceCandidate::new(&source, false)
always sets the "bare" flag to false so groups containing ":BARE:" can never
match; change this to derive the bare-ness from the import token (e.g. detect a
side-effect/bare import from ComparableToken/import_source_text or existing
helpers) and pass the correct boolean into ImportSourceCandidate::new (or
alternatively explicitly reject ":BARE:" for this rule if intended), and add a
new noRestrictedImports fixture that includes group: [":BARE:"] to cover this
path; update references: ImportSource, ComparableToken, ImportSourceCandidate,
and group.is_match accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e98fa38d-92bd-4608-b03d-3cd3d1a91073
⛔ Files ignored due to path filters (5)
crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-only.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-predefined.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (11)
.changeset/organize-imports-bare-matcher.mdcrates/biome_js_analyze/src/assist/source/organize_imports.rscrates/biome_js_analyze/src/assist/source/organize_imports/import_key.rscrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-only.jscrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-only.options.jsoncrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-predefined.jscrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-predefined.options.jsoncrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping.jscrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping.options.jsoncrates/biome_rule_options/src/no_restricted_imports.rscrates/biome_rule_options/src/organize_imports/import_groups.rs
|
It seems that OP isn't a bot, despite the heavy use of AI technologies. @Conaclos what do think of this proposal? |
|
Hi @georgephillips, thanks for your interest for improving Biome! I see an issue in the current implementation : {
"sortBareImports": true,
"groups": [
{ "source": ":BARE:" }
]
}This doesn't make sense to me. Actually, I wonder if we should extend the {
"sortBareImports": true,
"groups": [
{ "kind": "bare", "source": "**/*.css" }
]
}What do you think? Your example could so be written as: {
"sortBareImports": true,
"groups": [
{ "kind": "!bare" },
":BLANK_LINE:",
{ "kind": "bare" }
]
} |
Added a `kind` field to the `ImportMatcher` in the `organizeImports` assist action, allowing users to filter imports by their syntactic kind. The new matcher supports `bare` imports (e.g., `import "polyfill"`) and can be negated with `!bare`. This enhancement enables more flexible grouping configurations, such as combining with `source` and `type` fields for complex patterns. Updated documentation and tests to reflect these changes.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_rule_options/src/organize_imports/import_groups.rs (1)
196-317: Consider collapsing theNegatable…boilerplate.
NegatableImportKindMatchermirrorsNegatablePredefinedSourceMatcheralmost line-for-line: same{ is_negated, inner }shape, sameDisplay/Debug/From<_> for String/TryFrom<String>/FromStrscaffolding, same!-prefixed JsonSchema trick. A genericNegatable<T: Display + FromStr>(or a smallimpl_negatable!macro) would pay for itself the next time a kind matcher is added — and there's clearly appetite to add more (type, etc.).Non-blocking; happy to see it landed as-is and cleaned up in a follow-up, since it matches the pattern already in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_rule_options/src/organize_imports/import_groups.rs` around lines 196 - 317, NegatableImportKindMatcher duplicates NegatablePredefinedSourceMatcher boilerplate; refactor by introducing a generic Negatable<T> (or impl_negatable! macro) that wraps T with is_negated and implements Display, Debug, FromStr/TryFrom<String>, From<Negatable<T>> for String, biome_deserialize::Deserializable, and the schemars json_schema enum-extension; then replace NegatableImportKindMatcher with a type alias or newtype using Negatable<ImportKindMatcher> and keep ImportKindMatcher unchanged (look for NegatableImportKindMatcher, NegatablePredefinedSourceMatcher, ImportKindMatcher, the Deserializable impl, Display/Debug impls, TryFrom<String>, FromStr, and the #[cfg(feature = "schema")] json_schema method to update).
🤖 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_rule_options/src/organize_imports/import_groups.rs`:
- Around line 196-317: NegatableImportKindMatcher duplicates
NegatablePredefinedSourceMatcher boilerplate; refactor by introducing a generic
Negatable<T> (or impl_negatable! macro) that wraps T with is_negated and
implements Display, Debug, FromStr/TryFrom<String>, From<Negatable<T>> for
String, biome_deserialize::Deserializable, and the schemars json_schema
enum-extension; then replace NegatableImportKindMatcher with a type alias or
newtype using Negatable<ImportKindMatcher> and keep ImportKindMatcher unchanged
(look for NegatableImportKindMatcher, NegatablePredefinedSourceMatcher,
ImportKindMatcher, the Deserializable impl, Display/Debug impls,
TryFrom<String>, FromStr, and the #[cfg(feature = "schema")] json_schema method
to update).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 75b555bb-9ce5-480f-a318-5e6697ff1b54
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-source.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (9)
.changeset/organize-imports-bare-matcher.mdcrates/biome_js_analyze/src/assist/source/organize_imports.rscrates/biome_js_analyze/src/assist/source/organize_imports/import_key.rscrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-only.options.jsoncrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-predefined.options.jsoncrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-source.jscrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-source.options.jsoncrates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping.options.jsoncrates/biome_rule_options/src/organize_imports/import_groups.rs
✅ Files skipped from review due to trivial changes (5)
- crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-source.js
- crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-only.options.json
- crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-source.options.json
- crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping.options.json
- crates/biome_js_analyze/tests/specs/source/organizeImports/bare-grouping-with-predefined.options.json
Conaclos
left a comment
There was a problem hiding this comment.
Thanks for your changes! Looks awesome to me.
kind field to import matcher
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
@Conaclos thanks for merging! Any idea when this will be released? |
Summary
Adds a
:BARE:predefined group matcher to theorganizeImportsassist action. It selects bare (side-effect) imports such asimport "polyfill"and composes with the existinggroupsoption andsortBareImports: trueintroduced in #9384 and discussed at #7811.I originally started with a top-level option that would skip the chunking of the imports entirely. This seemed easier but then I came across the sortBareImports on the current branch and found the discussion about adding the BARE matcher. This will be useful for our lit.dev project which doesn't care too much about the order of the imports.
{ "sortBareImports": true, "groups": [ ["**", "!:BARE:"], ":BLANK_LINE:", [":BARE:"] ] }Hope this matches what you were planning to add for this. Thanks for building a great tool.
Test Plan
crates/biome_js_analyze/tests/specs/source/organizeImports/:bare-grouping.js— mixed bare/binding imports, bare group placed last with a blank-line separator.bare-grouping-only.js— file containing only bare imports, sorted naturally within the bare group.bare-grouping-with-predefined.js—:BARE:combined with:NODE:and the default package group.organizeImportssnapshots unchanged:cargo test -p biome_js_analyze --test spec_tests -- organize_imports→ 61 passed.cargo test -p biome_js_analyze --test spec_tests→ 2477 passed.cargo fmt,cargo run -p rules_check,cargo codegen-schema, and bindings regen are all clean.Docs
Documentation is inline in the
declare_source_rule!macro forOrganizeImportsincrates/biome_js_analyze/src/assist/source/organize_imports.rs::BARE:bullet in the predefined-groups list under### groups.### Place side-effect imports lastworked example under## Common configurations.:BARE:in the### Chunksnote explaining howsortBareImports+:BARE:interact.AI assistance disclosure: I used Claude Code to explore the codebase and scaffold the tests; the design and the final diff were chosen and reviewed by me.