-
-
Notifications
You must be signed in to change notification settings - Fork 805
chore: use tombi #8534
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
chore: use tombi #8534
Conversation
|
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
WalkthroughThis PR performs a comprehensive tooling and configuration update centred on migrating TOML formatting from Taplo CLI to Tombi. It updates GitHub workflows and documentation to reflect the change, replaces Taplo configuration with a Tombi configuration file, and reorganises Cargo manifests across the entire workspace. The root Cargo.toml is restructured to centralise workspace dependencies, package metadata, and lint configurations. Additionally, ~80 individual crate manifests are updated with explicit package metadata, workspace references, and feature alignment. The workspace resolver strategy is updated to version 2, and build scripts (justfile, package.json) are modified accordingly. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/biome_markup/Cargo.toml (1)
20-20: Use workspace dependency forquote.Other crates (e.g.,
biome_deserialize_macros) referencequote = { workspace = true }. This hardcoded version breaks consistency. Based on learnings, use workspace dependencies for crates defined in rootCargo.toml.Proposed fix
-quote = "1.0.14" +quote = { workspace = true }crates/biome_html_formatter/Cargo.toml (1)
29-41:tests_macrosshould use a path dependency.Line 41 uses
workspace = truefortests_macros, but other crates in this PR (e.g.,biome_tailwind_parser) usepath = "../tests_macros". Per coding guidelines, dev-dependencies for internal crates should use path dependencies to avoid requiring published versions. Based on learnings, this applies totests_macros.🔎 Proposed fix
-tests_macros = { workspace = true } +tests_macros = { path = "../tests_macros" }crates/biome_js_parser/Cargo.toml (1)
40-51: Inconsistent dev-dependency pattern for internal crates.Several internal biome crates (biome_configuration, biome_deserialize, biome_fs, biome_service, biome_test_utils) use
workspace = true, whilsttests_macroscorrectly uses a path dependency. For consistency withcrates/biome_css_formatter/Cargo.tomland the coding guidelines, all internal crates in dev-dependencies should use path dependencies.🔎 Suggested fix
[dev-dependencies] -biome_configuration = { workspace = true } -biome_deserialize = { workspace = true } -biome_fs = { workspace = true } -biome_service = { workspace = true } -biome_test_utils = { workspace = true } +biome_configuration = { path = "../biome_configuration" } +biome_deserialize = { path = "../biome_deserialize" } +biome_fs = { path = "../biome_fs" } +biome_service = { path = "../biome_service" } +biome_test_utils = { path = "../biome_test_utils" } camino = { workspace = true } criterion = { package = "codspeed-criterion-compat", version = "=3.0.5" } insta = { workspace = true } quickcheck = { workspace = true } quickcheck_macros = { workspace = true } tests_macros = { path = "../tests_macros" }Based on learnings: Use path dependencies for internal crates in dev-dependencies to avoid requiring published versions.
🧹 Nitpick comments (10)
crates/biome_js_analyze/Cargo.toml (1)
58-66: Consider using path dependencies for internal dev-dependencies.Per coding guidelines, dev-dependencies for internal crates should use path dependencies to avoid requiring published versions. Lines 59, 61, and 62 use
workspace = trueforbiome_fs,biome_plugin_loader, andbiome_service.Based on learnings, you may want to align these with the pattern used for
biome_js_parser,biome_test_utils, andtests_macros.🔎 Suggested change
[dev-dependencies] -biome_fs = { workspace = true } +biome_fs = { path = "../biome_fs" } biome_js_parser = { path = "../biome_js_parser", features = ["tests"] } -biome_plugin_loader = { workspace = true } -biome_service = { workspace = true } +biome_plugin_loader = { path = "../biome_plugin_loader" } +biome_service = { path = "../biome_service" } biome_test_utils = { path = "../biome_test_utils" }crates/biome_grit_syntax/Cargo.toml (1)
14-22: Minor redundancy in feature enablement.Line 15 already enables
biome_rowanwithfeatures = ["serde"], but theschemafeature on line 22 also enablesbiome_rowan/serde. This is harmless but redundant—Cargo deduplicates features anyway..github/workflows/autofix.yml (1)
55-59: Same pinning consideration applies here.For consistency with other actions in the repository, consider pinning
tombi-toml/setup-tombito a specific commit hash. Thetombi formatcommand (without--check) is correct for an autofix workflow.crates/biome_tailwind_syntax/Cargo.toml (1)
18-19: Consider removing commented-out dependencies.These commented-out dependency lines appear to be leftovers. If
biome_string_caseandcaminoare genuinely not needed, remove them entirely. If they're planned for future use, a TODO comment explaining the intent would be preferable to silent commented code.🔎 Proposed fix
[dependencies] biome_rowan = { workspace = true, features = ["serde"] } -# biome_string_case = { workspace = true } -# camino = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, features = ["derive"] }crates/biome_graphql_parser/Cargo.toml (1)
31-37: Dev-dependencies for internal crates should use path references.As per coding guidelines, internal crates in
dev-dependenciesshould use path dependencies to avoid requiring published versions. Consider changingbiome_test_utilsandtests_macrosto path-based references:-biome_test_utils = { workspace = true } +biome_test_utils = { path = "../biome_test_utils" } criterion = { package = "codspeed-criterion-compat", version = "=3.0.5" } insta = { workspace = true } quickcheck = { workspace = true } quickcheck_macros = { workspace = true } -tests_macros = { workspace = true } +tests_macros = { path = "../tests_macros" }Based on learnings, dev-dependencies should use path dependencies for internal crates.
crates/biome_html_parser/Cargo.toml (1)
28-35: Inconsistent dev-dependency handling forbiome_test_utils.
tests_macroscorrectly uses a path dependency, butbiome_test_utilsusesworkspace = true. For consistency and per coding guidelines, consider using a path reference:-biome_test_utils = { workspace = true } +biome_test_utils = { path = "../biome_test_utils" }Based on learnings, dev-dependencies for internal crates should use path dependencies.
crates/biome_deserialize/Cargo.toml (1)
30-31: Dev-dependency should use path reference.Per coding guidelines, internal crates in dev-dependencies should use path dependencies:
[dev-dependencies] -biome_deserialize_macros = { workspace = true } +biome_deserialize_macros = { path = "../biome_deserialize_macros" }Based on learnings, this avoids requiring published versions for development.
crates/biome_json_parser/Cargo.toml (1)
34-34: Consider using a path dependency forbiome_test_utils.Per the coding guidelines, dev-dependencies for internal crates should use path dependencies to avoid requiring published versions. Other crates in this PR (e.g.,
biome_css_parser,biome_json_formatter) usepath = "../biome_test_utils"for this dependency.Suggested fix
-biome_test_utils = { workspace = true } +biome_test_utils = { path = "../biome_test_utils" }crates/biome_css_analyze/Cargo.toml (1)
41-42: Consider using path dependencies for internal dev-dependencies.Per the coding guidelines, dev-dependencies for internal crates should use path dependencies.
biome_fsandbiome_plugin_loaderare internal crates but currently useworkspace = true.Suggested fix
-biome_fs = { workspace = true } -biome_plugin_loader = { workspace = true } +biome_fs = { path = "../biome_fs" } +biome_plugin_loader = { path = "../biome_plugin_loader" }crates/biome_formatter/Cargo.toml (1)
30-30: Consider consolidatingdrop_bombas a workspace dependency.The version
"0.1.5"is duplicated across three crates (biome_parser, biome_js_parser, and biome_formatter). Defining it in the rootCargo.tomlwould keep dependency versions centralised and prevent drift.
| "rand", | ||
| // Needs to be updated manually | ||
| "wasm-bindgen-cli" |
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 wasm-bindgen-cli ignore is appropriate—consider documenting the version sync requirement.
wasm-bindgen-cli's bindgen format is unstable enough that its version must exactly match the wasm-bindgen library dependency, making manual updates necessary. Since Biome's build process depends on WebAssembly bindings, this change correctly prevents Renovate from auto-updating a package that could break the build if misaligned.
If this fix is tangential to your PR's primary objective (Taplo → Tombi migration), consider whether it belongs in a separate PR for clearer review scope. Otherwise, the comment could reference why version sync matters for future maintainers.
🤖 Prompt for AI Agents
.github/renovate.json5 lines 46-48: the current Renovate ignore for
"wasm-bindgen-cli" is fine but lacks documentation; add a brief comment in the
file explaining that wasm-bindgen-cli must be manually updated because its
version must exactly match the wasm-bindgen library to avoid breaking
WebAssembly bindings, and note that this change may be split into a separate PR
if it’s unrelated to the Taplo→Tombi migration to keep scope clear.
| - uses: tombi-toml/setup-tombi@v1 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Format TOML files | ||
| run: tombi format --check |
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.
🧩 Analysis chain
🏁 Script executed:
find . -name "main.yml" -path "*/.github/workflows/*" | head -20Repository: biomejs/biome
Length of output: 85
🏁 Script executed:
cat .github/workflows/main.yml | head -100Repository: biomejs/biome
Length of output: 3151
🏁 Script executed:
grep -n "uses:" .github/workflows/main.ymlRepository: biomejs/biome
Length of output: 1040
Pin the action to a specific commit hash for consistency and supply chain security.
Third-party actions should be pinned to specific commit hash SHAs for maximum control and immutability. Other actions in this workflow use commit hash pinning (e.g., actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8), but tombi-toml/setup-tombi uses @v1. Version tags can be moved or deleted if a bad actor gains access to the repository storing the action—commit hashes prevent this.
🤖 Prompt for AI Agents
In .github/workflows/main.yml around lines 38 to 42, the tombi-toml/setup-tombi
action is pinned to a mutable tag (@v1); replace that tag with a specific commit
SHA from the tombi-toml/setup-tombi repository (e.g., @<commit-sha>) to ensure
immutability and supply-chain security — locate the desired release commit on
the action repo, copy its full commit hash, and update the workflow step to use
that SHA while leaving the env and run lines unchanged.
| [target.'cfg(unix)'.dependencies] | ||
| libc = "0.2.177" | ||
| tokio = { workspace = true, features = ["process"] } |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n Cargo.toml | grep -A 50 "\[workspace.dependencies\]"Repository: biomejs/biome
Length of output: 4617
🏁 Script executed:
rg -n 'libc' Cargo.tomlRepository: biomejs/biome
Length of output: 101
🏁 Script executed:
sed -n '100,135p' Cargo.toml | cat -nRepository: biomejs/biome
Length of output: 2286
🏁 Script executed:
sed -n '16,130p' Cargo.toml | grep -E '^\[|^libc'Repository: biomejs/biome
Length of output: 122
🏁 Script executed:
# Also check if libc appears elsewhere in crates/biome_cli/Cargo.toml with different versions
cat -n crates/biome_cli/Cargo.toml | grep -i libcRepository: biomejs/biome
Length of output: 81
🏁 Script executed:
# Check how libc is used in other crates
fd 'Cargo.toml' crates/ -x grep -l 'libc' {} \;Repository: biomejs/biome
Length of output: 122
🏁 Script executed:
# Check if other platform-specific dependencies use workspace = true
grep -n "target.*dependencies" crates/biome_cli/Cargo.toml -A 5Repository: biomejs/biome
Length of output: 506
🏁 Script executed:
# Check how libc is specified in biome_plugin_loader
grep -n 'libc' crates/biome_plugin_loader/Cargo.toml -B 2 -A 2Repository: biomejs/biome
Length of output: 345
🏁 Script executed:
# Verify the workspace guideline applies here - check if this is an internal-only dependency
grep -n 'libc' crates/*/Cargo.toml | head -20Repository: biomejs/biome
Length of output: 314
Use workspace dependency for libc to match the pattern used in other crates.
libc is defined in root workspace dependencies as 0.2.178, but biome_cli pins it to 0.2.177 directly. Switch to { workspace = true } like biome_plugin_loader does in its platform-specific dependencies section.
🤖 Prompt for AI Agents
In crates/biome_cli/Cargo.toml around lines 93 to 95, the unix-target dependency
pins libc = "0.2.177" while the workspace declares libc = "0.2.178"; replace the
explicit version with the workspace reference by changing the libc entry to use
{ workspace = true } so the crate uses the workspace-managed libc version
(matching biome_plugin_loader's pattern) and avoid version drift.
| [dev-dependencies] | ||
| criterion = { package = "codspeed-criterion-compat", version = "=3.0.5" } | ||
| insta = { workspace = true } | ||
| tests_macros = { workspace = true } |
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.
Inconsistent dev-dependency pattern for internal crate.
tests_macros uses workspace = true here, whilst crates/biome_css_formatter/Cargo.toml uses { path = "../tests_macros" } for the same crate. As per coding guidelines, dev-dependencies for internal crates should use path dependencies to avoid requiring published versions.
🔎 Suggested fix
[dev-dependencies]
criterion = { package = "codspeed-criterion-compat", version = "=3.0.5" }
insta = { workspace = true }
-tests_macros = { workspace = true }
+tests_macros = { path = "../tests_macros" }Based on learnings: Use path dependencies for internal crates in dev-dependencies.
🤖 Prompt for AI Agents
In crates/biome_configuration/Cargo.toml around lines 50 to 53, the
dev-dependency for the internal crate tests_macros uses workspace = true which
is inconsistent with other crates; change it to a path dependency (e.g.,
tests_macros = { path = "../tests_macros" }) so the dev-dependency references
the local crate source instead of relying on workspace/published versions,
matching the pattern used in crates/biome_css_formatter.
| categories.workspace = true | ||
| description = "<DESCRIPTION>" | ||
| edition.workspace = true | ||
| description = "<DESCRIPTION>" |
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.
Placeholder description needs updating.
The description field contains <DESCRIPTION> which should be replaced with an actual crate description before merging.
🤖 Prompt for AI Agents
crates/biome_glob/Cargo.toml around line 6: the description field is still the
placeholder "<DESCRIPTION>"; replace it with a concise, accurate crate
description (e.g., "Glob pattern utilities for Biome project" or similar) that
summarizes the crate's purpose, ensuring it is a short one-line string and
properly quoted for Cargo.toml.
| [dev-dependencies] | ||
| biome_json_parser = { workspace = true } |
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.
Dev-dependency should use path reference.
As per coding guidelines, dev-dependencies for internal crates should use path dependencies to avoid requiring published versions. Other crates in this PR (e.g., biome_graphql_semantic, biome_js_semantic) follow this pattern.
Suggested fix
[dev-dependencies]
-biome_json_parser = { workspace = true }
+biome_json_parser = { path = "../biome_json_parser" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [dev-dependencies] | |
| biome_json_parser = { workspace = true } | |
| [dev-dependencies] | |
| biome_json_parser = { path = "../biome_json_parser" } |
🤖 Prompt for AI Agents
In crates/biome_json_value/Cargo.toml around lines 23-24, the dev-dependency
declaration uses { workspace = true } but should be a path dependency for an
internal crate; change the dev-dependency to use a relative path to the local
biome_json_parser crate (e.g., path = "../biome_json_parser") so it references
the workspace crate by filesystem path instead of workspace=true, and ensure the
syntax matches other crates' dev-dependency entries.
| [[bench]] | ||
| harness = false | ||
| name = "mutation" | ||
| serde = ["biome_text_size/serde", "biome_text_size/serde", "dep:serde"] |
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.
Duplicate feature entry detected.
biome_text_size/serde appears twice in the serde feature list. One should be removed.
🔎 Proposed fix
-serde = ["biome_text_size/serde", "biome_text_size/serde", "dep:serde"]
+serde = ["biome_text_size/serde", "dep:serde"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| serde = ["biome_text_size/serde", "biome_text_size/serde", "dep:serde"] | |
| serde = ["biome_text_size/serde", "dep:serde"] |
🤖 Prompt for AI Agents
In crates/biome_rowan/Cargo.toml around line 36, the serde feature list contains
a duplicate entry "biome_text_size/serde"; remove the duplicated entry so each
feature appears only once (leave one "biome_text_size/serde" and the other
entries unchanged) and save the file.
| [dev-dependencies] | ||
| biome_test_utils = { workspace = true } | ||
| insta = { workspace = true } | ||
| quickcheck = { workspace = true } | ||
| quickcheck_macros = { workspace = true } | ||
| tests_macros = { workspace = true } |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check dev-dependencies patterns in parser crates
echo "=== biome_html_parser dev-dependencies ==="
rg -A 10 '^\[dev-dependencies\]' crates/biome_html_parser/Cargo.toml || echo "File not found"
echo -e "\n=== biome_json_parser dev-dependencies ==="
rg -A 10 '^\[dev-dependencies\]' crates/biome_json_parser/Cargo.toml || echo "File not found"
echo -e "\n=== biome_css_parser dev-dependencies ==="
rg -A 10 '^\[dev-dependencies\]' crates/biome_css_parser/Cargo.toml || echo "File not found"
echo -e "\n=== biome_yaml_parser dev-dependencies (for comparison) ==="
rg -A 10 '^\[dev-dependencies\]' crates/biome_yaml_parser/Cargo.toml || echo "File not found"Repository: biomejs/biome
Length of output: 2024
Use path dependencies for internal dev-dependencies.
Dev-dependencies like tests_macros should use path references (e.g., { path = "../tests_macros" }) rather than workspace = true, consistent with the coding guideline and other parser crates such as biome_html_parser and biome_json_parser. This avoids the need for published versions in dev-only contexts.
🤖 Prompt for AI Agents
crates/biome_yaml_parser/Cargo.toml lines 24-29: internal dev-dependencies are
using `workspace = true`; change these to explicit path dependencies for local
crates (e.g., `tests_macros = { path = "../tests_macros" }`) consistent with
other parser crates. Update any other internal dev-deps here (biome_test_utils,
insta, quickcheck, quickcheck_macros, tests_macros) to use the appropriate `path
= "../<crate_name>"` entries instead of `workspace = true`, ensuring relative
paths match the workspace layout.
| biome_json_parser = { path = "../crates/biome_json_parser" } | ||
| biome_service = { path = "../crates/biome_service" } | ||
| libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer", default-features = false } | ||
| similar = { workspace = true } |
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.
workspace = true won't resolve in an isolated workspace.
This crate declares its own workspace at line 12-13 with only itself as a member. Using similar = { workspace = true } requires similar to be defined in this crate's [workspace.dependencies], which it isn't. This will cause a build failure.
🔎 Proposed fix
-similar = { workspace = true }
+similar = "2.7.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| similar = { workspace = true } | |
| similar = "2.7.0" |
🤖 Prompt for AI Agents
fuzz/Cargo.toml lines 93-93: The dependency entry `similar = { workspace = true
}` is invalid for this isolated workspace; either add `similar` to this
Cargo.toml's [workspace.dependencies] or change the entry to a normal dependency
(e.g. `similar = "x.y"` or `similar = { path = "../path/to/similar" }`). Update
the file by either (A) adding a [workspace.dependencies] section at the top that
includes `similar = "version"` or (B) replacing the current line with a concrete
version or path-based dependency so Cargo can resolve it during build.
Summary
Unfortunately, taplo, the tool we use for formatting TOML, isn't maintained anymore (tamasfe/taplo#715).
This PR moves us to tombi: https://tombi-toml.github.io/tombi/docs/reference/difference-taplo/
Test Plan
Green CI
Docs