feat(plugin): add WASM Component Model plugin system#9399
Open
Hideart wants to merge 18 commits intobiomejs:nextfrom
Open
feat(plugin): add WASM Component Model plugin system#9399Hideart wants to merge 18 commits intobiomejs:nextfrom
Hideart wants to merge 18 commits intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 3d2ac29 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 |
github-actions
bot
added
A-CLI
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces a comprehensive WebAssembly-based plugin system for Biome behind the Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
crates/biome_plugin_loader/src/configuration.rs-200-216 (1)
200-216:⚠️ Potential issue | 🟡 MinorUnknown severity levels silently return
None.When a user provides an invalid severity like
"warning"(typo for"warn"),deserializereturnsNonewithout emitting a diagnostic. This may cause confusion as the rule configuration is silently dropped.Consider reporting an error via
ctx.report_error()or similar for unrecognised values.🛠️ Proposed improvement
impl Deserializable for PluginRulePlainConfiguration { fn deserialize( ctx: &mut impl DeserializationContext, value: &impl DeserializableValue, name: &str, ) -> Option<Self> { let text: Text = Deserializable::deserialize(ctx, value, name)?; match text.text() { "off" => Some(Self::Off), "on" => Some(Self::On), "info" => Some(Self::Info), "warn" => Some(Self::Warn), "error" => Some(Self::Error), - _ => None, + other => { + ctx.report_unknown_value(value.range(), &["off", "on", "info", "warn", "error"]); + None + } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/configuration.rs` around lines 200 - 216, The deserialize implementation for PluginRulePlainConfiguration currently swallows unknown severity strings; update the match's fallback branch in Deserializable::deserialize to call ctx.report_error (or the appropriate method on DeserializationContext) with a clear message that includes the invalid value (from text.text()) and the configuration key/name, then return None (or a fallback) — i.e., in the "_" arm of PluginRulePlainConfiguration::deserialize report the unrecognised severity via ctx.report_error including the provided name and value so the user gets a diagnostic.crates/biome_js_analyze/tests/wasm_plugin_tests.rs-1-5 (1)
1-5:⚠️ Potential issue | 🟡 MinorAdd
#![cfg(feature = "wasm_plugin")]for consistency with the parallel test inbiome_plugin_loader.The test file mirrors the structure of
biome_plugin_loader/tests/wasm_plugin_integration.rs, which gates itself with this feature. Add it here for clarity and consistency across the codebase.🛠️ Suggested change
+#![cfg(feature = "wasm_plugin")] + //! Analyzer pipeline integration tests for the booleanNaming WASM plugin. //! //! These tests load the compiled `.wasm` plugin fixture, run it through the //! full `biome_js_analyze::analyze` pipeline, and verify diagnostics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/tests/wasm_plugin_tests.rs` around lines 1 - 5, Add the crate-level attribute to gate these tests behind the same feature as the parallel test: insert #![cfg(feature = "wasm_plugin")] at the top of the wasm_plugin_tests.rs file (above the existing module doc comment) so the Analyzer pipeline integration tests run only when the "wasm_plugin" feature is enabled; mirror the gating used by biome_plugin_loader/tests/wasm_plugin_integration.rs for consistency.crates/biome_plugin_loader/src/plugin_cache.rs-38-42 (1)
38-42:⚠️ Potential issue | 🟡 MinorAdd test coverage for the suffix matching fallback to prevent false positives.
The fallback suffix match with
ends_with()can produce false positives when two plugins share the same filename in different directories. Add a test confirming the fallback works for relative vs absolute path mismatches (the intended use case) without incorrectly matching unintended plugins.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/plugin_cache.rs` around lines 38 - 42, Add a unit test in plugin_cache.rs that exercises the fallback suffix match logic around the found variable (the map.get(&path_buf).or_else(...).find(...).map(...)) to ensure ends_with() does not produce false positives when two plugins share the same filename in different directories; specifically construct a map with two distinct keys that have the same file name in different parent directories and assert that looking up a relative vs absolute path_buf returns only the intended plugin (and not the other), and include a negative case where a different directory with same filename must not match. Ensure the test targets the suffix-matching branch so we catch regressions in the fallback behavior.crates/biome_service/src/file_handlers/json.rs-723-737 (1)
723-737:⚠️ Potential issue | 🟡 MinorDerive parser options from
file_sourcein reparse callback.Line 734 uses
JsonParserOptions::default(), which loses theallow_commentsandallow_trailing_commassettings from the originalfile_source. This causes parse errors during fix-all for JSONC files or JSON files with trailing commas.Build
JsonParserOptionsusingfile_source.allow_comments()andfile_source.allow_trailing_commas()instead of defaults—file_sourceis already available in scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/file_handlers/json.rs` around lines 723 - 737, The reparse callback currently uses JsonParserOptions::default() when calling biome_json_parser::parse_json inside process_fix_all.process_action_with_reparse, which drops file_source-specific settings; replace the default with a JsonParserOptions built from file_source.allow_comments() and file_source.allow_trailing_commas() (e.g., construct JsonParserOptions { allow_comments: file_source.allow_comments(), allow_trailing_commas: file_source.allow_trailing_commas(), ..Default::default() }) so the parser honors JSONC/trailing-comma settings during fixes.e2e-tests/wasm-plugins/test.sh-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd a shebang line.
The script uses
set -eubut lacks a shebang, which means the shell interpreter is undefined. This can cause portability issues.Proposed fix
+#!/bin/bash set -eu🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e-tests/wasm-plugins/test.sh` at line 1, Add a POSIX-compatible shebang as the first line of the script so the interpreter is defined (e.g. use "/usr/bin/env bash"), placing it before the existing "set -eu" line in e2e-tests/wasm-plugins/test.sh; ensure the shebang is the very first byte of the file and that "set -eu" remains immediately after it.crates/biome_plugin_loader/src/analyzer_wasm_plugin.rs-265-272 (1)
265-272:⚠️ Potential issue | 🟡 MinorSession cached even when
check_currentfails.If
check_currentreturns an error, the session is still inserted into the map (line 268). This differs from the error path at lines 289-293 which removes the session. Consider whether a failed initial check should also prevent caching.🔧 Suggested fix to avoid caching on initial failure
match self.engine.create_session( node, self.target_language, semantic_model, module_resolver, file_path, ) { Ok(mut session) => { let result = session.check_current(&self.rule_name, self.options_json.as_deref()); - map.insert(self.plugin_id, session); - result + if result.is_ok() { + map.insert(self.plugin_id, session); + } + result } Err(e) => Err(e), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/analyzer_wasm_plugin.rs` around lines 265 - 272, The code currently inserts the session into the map before calling session.check_current, which caches sessions even when check_current returns Err; move the map.insert(self.plugin_id, session) so it only runs after check_current returns Ok (or alternatively call check_current first and insert the session on success), ensuring the initial failure path does not cache the session; reference the session variable, check_current method, map.insert call, and self.plugin_id when making the change.
🧹 Nitpick comments (14)
crates/biome_plugin_loader/src/diagnostics.rs (1)
162-166: Prefer a small constructor over widening these fields.
pub(crate)works, but it also makes ad-hocCompileDiagnosticconstruction possible anywhere in the crate. A tinyCompileDiagnostic::new(message, source)keeps that wiring in one place and leaves you a bit more room for invariants later. Tiny guard-rail, small payoff.Possible refactor
#[derive(Debug, Serialize, Deserialize, Diagnostic)] #[diagnostic( category = "plugin", severity = Error, )] pub struct CompileDiagnostic { #[message] #[description] - pub(crate) message: MessageAndDescription, + message: MessageAndDescription, #[serde(skip)] #[source] - pub(crate) source: Option<Error>, + source: Option<Error>, +} + +impl CompileDiagnostic { + pub(crate) fn new(message: MessageAndDescription, source: Option<Error>) -> Self { + Self { message, source } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/diagnostics.rs` around lines 162 - 166, Introduce a small constructor to avoid exposing internal fields: make the fields message: MessageAndDescription and source: Option<Error> private (remove pub(crate) on them) and add a public(crate) CompileDiagnostic::new(message: MessageAndDescription, source: Option<Error>) -> CompileDiagnostic that constructs and returns the struct; update all call sites that currently construct CompileDiagnostic with struct literal syntax to call CompileDiagnostic::new instead so construction is centralized and you can keep invariants enforced in one place.justfile (1)
286-295: Handy recipe for manual fixture builds.One consideration: this recipe hardcodes the three current plugins. If more e2e plugins are added later, this will need updating. The build.rs approach (mentioned in the comment) may be more maintainable for the long term.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 286 - 295, The justfile recipe build-test-wasm-plugins hardcodes three plugin paths, which will require maintenance as plugins are added; change the recipe to discover and build plugins dynamically by iterating over directories under e2e-tests/wasm-plugins/plugins (or delegate to the existing build.rs) so each plugin's Cargo.toml is built with cargo build --target wasm32-wasip2 --release and its produced .wasm artifact is copied into crates/biome_plugin_loader/tests/fixtures; update the recipe name build-test-wasm-plugins and the commands that reference specific plugin directories (e.g., the three cargo build and cp lines) to use a loop over plugin dirs and copy the release .wasm outputs instead of hardcoded paths.crates/biome_plugin_sdk/build.rs (2)
83-100: Consider guarding against malformed input causing underflow.If the source file is malformed with an unmatched
},depth -= 1on line 92 could underflow. Given this parses generated files, it's low risk, but a debug assertion or saturating subtraction would be defensive.🛡️ Optional: use saturating_sub
'}' => { - depth -= 1; + depth = depth.saturating_sub(1); if depth == 0 {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_sdk/build.rs` around lines 83 - 100, The loop that scans braces (variable names: body, depth, end_pos, using body.char_indices()) can underflow when encountering an unmatched '}' — change the decrement to a guarded operation: before doing depth -= 1 check if depth > 0 and handle the malformed input (return an Err, panic with a clear message, or break), or use a saturating/checked subtraction to avoid underflow; ensure you propagate or log an error when an unmatched closing brace is found instead of allowing depth to underflow and continue, and keep setting end_pos when depth reaches zero as currently implemented.
136-150: Order of explicit discriminant handling vs skip_next may cause incorrect values.When a
#[doc(hidden)]variant has an explicit discriminant (e.g.,__LAST = 500), the current logic:
- Parses and sets
discriminant = 500(lines 136-144)- Then skips the variant and increments to 501 (lines 146-150)
This means the next non-hidden variant gets 501 instead of starting fresh from whatever the enum actually defines. If hidden variants always appear at the end (like
__LAST), this is fine in practice.🔧 Safer ordering: check skip_next before parsing discriminant
- // Check for explicit discriminant assignment: `VARIANT = N,` - if let Some(eq_rest) = trimmed.strip_prefix(variant_name) { - let eq_rest = eq_rest.trim(); - if let Some(after_eq) = eq_rest.strip_prefix('=') { - let val_str = after_eq.trim().trim_end_matches(',').trim(); - if let Ok(val) = val_str.parse::<u32>() { - discriminant = val; - } - } - } - if skip_next { skip_next = false; + // Still need to parse explicit discriminant for correct sequencing + if let Some(eq_rest) = trimmed.strip_prefix(variant_name) { + let eq_rest = eq_rest.trim(); + if let Some(after_eq) = eq_rest.strip_prefix('=') { + let val_str = after_eq.trim().trim_end_matches(',').trim(); + if let Ok(val) = val_str.parse::<u32>() { + discriminant = val; + } + } + } discriminant += 1; continue; } + + // Check for explicit discriminant assignment: `VARIANT = N,` + if let Some(eq_rest) = trimmed.strip_prefix(variant_name) { + let eq_rest = eq_rest.trim(); + if let Some(after_eq) = eq_rest.strip_prefix('=') { + let val_str = after_eq.trim().trim_end_matches(',').trim(); + if let Ok(val) = val_str.parse::<u32>() { + discriminant = val; + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_sdk/build.rs` around lines 136 - 150, The parsing currently sets discriminant from explicit values (val_str -> discriminant) before honoring skip_next, causing a skipped #[doc(hidden)] variant with an explicit discriminant to incorrectly advance discriminant; change the logic in build.rs so you check skip_next first (or skip any parsing when skip_next is true) — i.e., inspect skip_next before the block that uses trimmed.strip_prefix(variant_name) and parsing val_str, and only parse/set discriminant when the variant is not being skipped; reference variables/functions: skip_next, discriminant, trimmed, variant_name, val_str.crates/biome_plugin_sdk/src/options.rs (2)
10-25: Please make this a real doctest.
ignorelets the example rot quietly. A tiny executable example with an assertion would keep this API honest.As per coding guidelines, "Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_sdk/src/options.rs` around lines 10 - 25, The example doctest is marked "ignore" and should be an executable doctest: change the code fence from ```ignore to a normal Rust doctest (remove "ignore"), construct a small JSON string (e.g. assign a literal to options_json), call the public helpers get_string, get_number, get_bool, get_string_array from options and add assert_eq! (or assert!) checks validating returned values (including unwrap_or fallback behavior where relevant) so the example compiles and runs as a doctest; reference the functions get_string, get_number, get_bool, and get_string_array in your example to keep the API exercised.
193-264: Please add regressions for nested keys and UTF-8.The current tests only cover flat ASCII objects, which is why both failure modes above would slip through unnoticed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_sdk/src/options.rs` around lines 193 - 264, Add unit tests that cover nested keys and UTF-8 to prevent regressions: create tests (e.g., test_key_nested_not_matched and test_utf8_keys_and_values) that feed JSON with nested objects like {"outer": {"key":"bad"}, "key":"good"} and assert get_string/get_number/get_bool/get_string_array only return the top-level "key" value (not the nested one), and tests that use UTF-8 in keys and values (e.g., keys/values with "π", "é", emojis) to assert get_string/get_string_array correctly return Unicode strings and get_number/get_bool behave as expected; add analogous cases for missing keys and empty arrays to ensure existing functions (get_string, get_string_array, get_number, get_bool) handle non-ASCII input and nested structures.e2e-tests/wasm-plugins/plugins/boolean-naming/src/lib.rs (2)
61-73: Rule metadata version is"0.0.0".This appears to be placeholder metadata. Consider documenting that plugin authors should update this before publishing, or defaulting to
"next"to match Biome's convention for unreleased rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e-tests/wasm-plugins/plugins/boolean-naming/src/lib.rs` around lines 61 - 73, The RuleMetadata returned by rule_metadata currently uses a placeholder version "0.0.0"; update the version field in the rule_metadata function (the RuleMetadata struct instance) to a non-placeholder value—either set it to "next" to match Biome's convention for unreleased rules or add a short comment/README note instructing plugin authors to replace "0.0.0" before publishing so RuleMetadata::version is accurate for the boolean-naming plugin.
55-59: Configuration is silently ignored after first call.
PATTERN.set()returnsErrif already set, but the result is discarded withlet _ = .... This means reconfiguration (e.g., if the host callsconfiguremultiple times for different files with different options) is silently ignored.If single-configuration-per-plugin-lifetime is intentional, a brief doc comment would clarify this.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e-tests/wasm-plugins/plugins/boolean-naming/src/lib.rs` around lines 55 - 59, The configure function currently discards PATTERN.set(...) result so reconfiguration is silently ignored; update configure (and its use of biome_plugin_sdk::options::get_string and options_json) to handle the Err case from PATTERN.set: either allow updates by replacing the existing pattern (e.g., use the container's replace/update API or get_mut and overwrite the stored value) or explicitly return/log an error/warning when set fails so callers know configuration was ignored; also consider adding a doc comment on configure/PATTERN if single-configuration-per-plugin-lifetime is intentional.crates/biome_js_analyze/tests/wasm_plugin_tests.rs (1)
140-150: The suppression test could be more explicit about expected behaviour.The test documents that suppression effectiveness isn't asserted, which is fine for a "no crash" smoke test. However, consider adding a TODO or linking to a tracking issue for when full suppression support is wired in.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/tests/wasm_plugin_tests.rs` around lines 140 - 150, The test wasm_plugin_suppression_comment_does_not_crash currently only asserts "no crash" but lacks a TODO/link to track full suppression support; update the test's comment (inside function wasm_plugin_suppression_comment_does_not_crash) to include a clear TODO or FIXME with the tracking issue ID (or internal ticket/link) and a short note about when to strengthen the assertion to check suppression effectiveness, so future authors know to revisit this behavior.crates/biome_analyze/src/analyzer_plugin.rs (2)
241-265: Domain filtering and per-rule override logic is clear.The early returns for disabled rules and domain filtering are well-documented. However, this logic is duplicated in
BatchPluginVisitor(lines 478-510).Consider extracting the filtering logic into a helper method to avoid duplication:
♻️ Suggested refactor
fn should_skip_plugin( plugin: &dyn AnalyzerPlugin, options: &AnalyzerOptions, ) -> bool { // Per-rule disable check if let Some(ovr) = options.plugin_rule_override(plugin.rule_name()) { if ovr.disabled { return true; } } // Domain filtering let plugin_domains = plugin.domains(); if !plugin_domains.is_empty() { let domain_cfg = options.linter_domains(); if !domain_cfg.is_empty() { for domain in plugin_domains { match domain_cfg.get(domain) { Some(&PluginDomainFilter::Disabled) => return true, Some(&PluginDomainFilter::Recommended) if !plugin.is_recommended() => { return true; } _ => {} } } } } false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_analyze/src/analyzer_plugin.rs` around lines 241 - 265, Extract the duplicated per-rule and domain filtering into a helper function (e.g., fn should_skip_plugin(plugin: &dyn AnalyzerPlugin, options: &AnalyzerOptions) -> bool) and replace the inline logic in AnalyzerPluginVisitor (the current block using self.plugin.rule_name(), ctx.options.plugin_rule_override(...), ctx.options.linter_domains(), PluginDomainFilter checks, and self.plugin.is_recommended()) and the duplicated block in BatchPluginVisitor with calls to this helper; ensure the helper mirrors the existing checks (returns true when override.disabled, when a domain is Disabled, or when domain is Recommended and plugin.is_recommended() is false) and keep all existing references to plugin.rule_name(), options.plugin_rule_override(...), options.linter_domains(), and PluginDomainFilter to preserve behavior.
460-466: Case-insensitive ASCII matching for triggers.The
eq_ignore_ascii_caseapproach is efficient but won't handle Unicode case folding (e.g.,"ß"vs"SS"). If trigger strings are expected to be ASCII-only (identifiers, keywords), this is fine. Otherwise, consider documenting this limitation in the WIT interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_analyze/src/analyzer_plugin.rs` around lines 460 - 466, The current closure using t.as_bytes() and w.eq_ignore_ascii_case(t_bytes) performs ASCII-only case-insensitive matching and won't handle Unicode case folding (e.g., ß vs SS); either document this ASCII-only limitation in the WIT/type docs for the triggers input, or change the comparison in the triggers.iter().any closure (the place that builds t_bytes and calls eq_ignore_ascii_case) to use Unicode-aware folding (e.g., casefold/Unicode-normalize both source and trigger strings before comparing, or use a crate that provides Unicode case-folding) so non-ASCII case mappings are handled correctly.crates/biome_plugin_loader/src/configuration.rs (1)
275-285:PluginPathWithOptions.optionsis a required field with implicit default.The struct documentation and serde derive suggest
optionsis always present, but the deserialiser defaults it to"{}"(line 356). Consider making the fieldOption<String>to be explicit, or document that an empty object is the default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/configuration.rs` around lines 275 - 285, The PluginPathWithOptions struct declares options as a required String but code elsewhere treats it as defaulting to "{}"; change the field to Option<String> (pub options: Option<String>) and update any deserialization/default logic and call sites that read PluginPathWithOptions::options to treat None as "{}" (or generate a default when constructing/serializing). Update the serde attributes or helper functions that currently inject the "{}" default so behavior is explicit, and adjust tests and uses of PluginPathWithOptions, PluginPathWithOptions::options, and any code expecting a raw JSON string accordingly.crates/biome_plugin_loader/src/analyzer_wasm_plugin.rs (1)
253-256: Nested Option extraction is a bit convoluted but correct.The double-unwrap pattern for
ModuleResolverhandles theOption<Arc<ModuleResolver>>service correctly. The code is functional, though a brief comment explaining why the service is wrapped inOptionwould help future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/analyzer_wasm_plugin.rs` around lines 253 - 256, Add a short clarifying comment above the semantic_model/module_resolver extraction explaining that ModuleResolver is registered as an Option<Arc<ModuleResolver>> to represent a possibly-missing resolver and why we call services.get_service::<Option<Arc<ModuleResolver>>>() followed by and_then(|opt| opt.as_ref().map(Arc::clone)) to safely clone the Arc only when present; reference the variables/functions involved (semantic_model, module_resolver, ModuleResolver, get_service) so future readers understand the nested Option handling and cloning intent.crates/biome_plugin_loader/src/lib.rs (1)
57-63: Minor:plugin_nameextraction could handle edge cases more gracefully.
file_stem()returnsNonefor paths like/or empty paths. The fallback to the full path string is reasonable, but consider whether this could produce confusing diagnostic names in edge cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/lib.rs` around lines 57 - 63, The current plugin_name extraction using plugin_path.file_stem().unwrap_or(plugin_path.as_str()).to_string() can yield confusing names for edge paths (e.g., "/" or empty); update the logic around plugin_path and plugin_name to prefer file_stem(), then file_name(), then a lossy string of the path, and finally a stable fallback like "<unknown-plugin>" so diagnostics never show an empty or root path string; locate the code manipulating plugin_path and plugin_name (the plugin_name assignment) and replace the single unwrap_or fallback with this prioritized sequence using file_stem(), file_name(), Path::to_string_lossy(), and a constant default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/metal-bees-float.md:
- Line 5: Revise the changelog sentence to use user-friendly, non-technical
language and clearly mark the feature as opt-in/experimental: replace the phrase
"WASM Component Model plugin system with multi-language support (JavaScript,
CSS, JSON), per-rule options, suppression actions, and rule name display in
diagnostic headers" with a short, plain-English line such as "Added experimental
support for custom lint rules via WebAssembly plugins (requires compiling with
the wasm_plugin feature flag); you can now write rules for JavaScript, CSS, and
JSON." Ensure the term "wasm_plugin" is present to indicate it is a compile-time
opt-in feature and keep the tone accessible for end users.
In `@Cargo.toml`:
- Around line 175-183: Update the outdated dependencies in Cargo.toml by bumping
the wasmtime and wit-bindgen versions: change the wasmtime dependency (and
wasmtime-wasi if you pin the same major) from "30" to "41.0.3" and update
wit-bindgen from "0.39" to "0.53.1"; then run cargo update and cargo build to
surface any API or feature changes, and adjust any code that breaks due to
upstream changes in wasmtime (e.g., feature names or API signatures) or
wit-bindgen generated bindings until the project compiles cleanly.
In `@crates/biome_analyze/src/signals.rs`:
- Around line 174-209: In apply_plugin_edits, detect overlapping plugin edits
while iterating sorted (e.g., inside the for edit in &sorted loop): if
edit.range.start() < cursor then reject the input (return None) instead of
silently composing a malformed patch; this uses the existing cursor, sorted, and
edit.range.start()/end() symbols—add the overlap check before copying the
"before" slice and building new_text so overlapping ranges are bailed out early.
- Around line 227-238: The loop that attaches code suggestions uses raw plugin
edits and applicability, bypassing the normalization done by
plugin_rule_override(...) (and its fix_kind overrides), so fixKind:
none/safe/unsafe overrides are ignored; update the loop in AnalyzerDiagnostic
construction to use the normalized actions (the same normalized
applicability/fix_kind produced when building self.actions) — i.e., ensure you
call/apply plugin_rule_override(...) or use the normalized field on each action
before creating CodeSuggestionAdvice in the for action in &self.actions loop and
skip adding suggestions when the normalized fix_kind indicates none, and set the
suggestion.applicability from that normalized value rather than the raw plugin
applicability.
In `@crates/biome_diagnostics/src/serde.rs`:
- Around line 23-25: The subcategory field (subcategory: Option<String>) is
currently serialized as null which breaks the test_compare in test_serialize();
add the serde attribute to skip emitting the field when it's None by annotating
the subcategory field with #[serde(skip_serializing_if = "Option::is_none")];
update the struct in serde.rs (the field named subcategory next to category:
Option<&'static Category> and severity: Severity) so serialized() output omits
the key instead of producing null.
In `@crates/biome_js_semantic/src/semantic_model/model.rs`:
- Around line 484-513: The three helpers (resolve_reference_node,
binding_by_node, is_binding_exported) currently key only on
text_trimmed_range().start(), so change them to verify exact node identity by
comparing the full trimmed range (start AND end) or node identity before
returning: after you look up ids via declared_at_by_start or bindings_by_start,
fetch the stored declaration range for that id and ensure it equals
node.text_trimmed_range() (or use a pointer/green-node equality check if
available) and only then construct/return the Binding (or true); otherwise
return None/false. This uses the existing symbols declared_at_by_start,
bindings_by_start, is_exported, resolve_reference_node, binding_by_node, and
is_binding_exported to narrow accepted nodes.
In `@crates/biome_plugin_loader/build.rs`:
- Around line 83-86: The current src path assumes the .wasm is under
plugin_dir/target, but workspace-member builds place artifacts in the workspace
root target dir; update the logic in build.rs where src, dst, plugin_dir,
fixtures_dir, and wasm_name are used to either (a) pass a consistent target
directory to cargo by adding --target-dir when invoking cargo for building
workspace-member plugins so the artifact lands in
plugin_dir/target/wasm32-wasip2/release/{wasm_name}.wasm, or (b) resolve the
artifact from the workspace root target directory (e.g., locate
workspace_root/target/wasm32-wasip2/release/{wasm_name}.wasm) before copying to
fixtures_dir; pick one approach and implement it for the code that sets src so
it reliably points to the actual .wasm output for workspace-member plugins.
In `@crates/biome_plugin_loader/src/analyzer_grit_plugin.rs`:
- Around line 47-49: The current rule_name() returns the literal "anonymous" for
unnamed Grit queries which causes identifier collisions; change rule_name() to
return grit_query.name if present otherwise derive a stable fallback from the
plugin's path (e.g., use self.plugin_path filename stem or a short hash of
self.plugin_path) so each unnamed query gets a unique, stable id tied to the
plugin; update rule_name() to reference
self.grit_query.name.as_deref().unwrap_or_else(|| /* derived id from
self.plugin_path */) and implement the path-derived fallback logic using the
existing plugin path field (e.g., self.plugin_path or self.path) to produce a
deterministic string.
In `@crates/biome_plugin_sdk_macros/src/lib.rs`:
- Around line 8-9: WIT_CONTENT currently uses
include_str!("../../biome_plugin_sdk/wit/biome-plugin.wit") which pulls a file
outside this crate; move or copy biome-plugin.wit into this crate (e.g.,
crates/biome_plugin_sdk_macros/wit/biome-plugin.wit) and update the WIT_CONTENT
constant to use include_str!("wit/biome-plugin.wit") (or implement a build-step
that embeds the file into the crate package) so builds of the published macro
crate don't rely on a sibling workspace path.
In `@crates/biome_plugin_sdk/Cargo.toml`:
- Around line 14-15: The Cargo.toml currently pins the internal crate
biome_plugin_sdk_macros with a path and version, which bypasses the workspace
wiring; update the dependency declaration for biome_plugin_sdk_macros in
Cargo.toml to use the workspace dependency form (workspace = true) so it uses
the shared workspace version rather than a hardcoded path/version, ensuring the
entry referencing biome_plugin_sdk_macros is replaced accordingly.
In `@crates/biome_plugin_sdk/src/options.rs`:
- Around line 97-138: The current find_value_start function matches keys inside
any nested object because it never tracks JSON depth; update find_value_start to
track object/array depth while scanning (increment on '{' or '[' and decrement
on '}' or ']' when not inside a string) and only consider a quote-starting key
match when the current depth is exactly 1 (i.e., top-level key in the outer
object); keep the existing string-escaping logic (handling backslashes) so
string boundaries remain correct, and only when you accept a match at depth==1
compute after_key/after_colon/offset as existing code does and return that
offset.
- Around line 140-170: parse_json_string currently iterates bytes and casts to
char which mangles non-ASCII UTF-8; update parse_json_string (and ensure
get_string_array uses it) to iterate over Unicode scalar values rather than raw
bytes, handle escape sequences by consuming chars (not bytes), properly decode
\uXXXX (and surrogate pairs) into valid Rust chars/strings, and append full
Unicode characters to result; ensure you don't drop or reinterpret multi-byte
UTF-8 sequences and that all match/advance logic (the loop and index/iterator
handling) uses char indices or a char iterator instead of direct byte indexing.
In `@crates/biome_service/src/file_handlers/javascript.rs`:
- Around line 1119-1131: The reparse closure inside
process_fix_all.process_action_with_reparse currently calls
biome_js_parser::parse with JsParserOptions::default(), which ignores per-file
parser flags from params.settings (e.g., jsx_everywhere); update the closure to
derive parser options from params.settings the same way the initial parse does
(use the same extraction logic used at the earlier parse site around line 565)
and pass those options into biome_js_parser::parse so reparses use the file's
actual JsParserOptions rather than the default.
In `@crates/biome_service/src/file_handlers/mod.rs`:
- Around line 719-740: The plugin error diagnostics are missing subcategory() so
the severity override path in the severity lookup (which calls
diagnostic.subcategory() in mod.rs) never matches; update the diagnostic
construction sites in analyzer_js_plugin.rs and analyzer_grit_plugin.rs (the
error handling paths that build RuleDiagnostic) to call .subcategory(rule_name)
(or the actual rule identifier variable used there) when creating the
RuleDiagnostic so the plugin rule override lookup can find the rule and apply
configured severities.
In `@crates/biome_service/src/workspace/server.rs`:
- Around line 794-815: The cache currently keys plugins only by path causing
different configurations to collide; modify the caching logic so the cache key
includes plugin options (e.g., use a key type of (Utf8PathBuf, Option<String>)
or a combined hash of path+options) when inserting and looking up plugins in
PluginCache: update the code around the BiomePlugin::load call and
plugin_cache.insert_plugin to pass the options_json alongside the path (or
compute a stable combined key) and adjust PluginCache APIs
(insert_plugin/get_plugin) to accept that composite key so each distinct
options_json produces a distinct cache entry.
In `@crates/biome_wasm_plugin/src/engine.rs`:
- Around line 402-409: The code currently uses a single boolean self.configured
to gate calling self.bindings.call_configure, causing the first check() call
(even with None options_json) to permanently skip future configure calls and to
stick configuration to the first rule_name; change the logic to track
configuration per invocation by storing and comparing the configured pair
(rule_name and options_json) or simply call configure on every check when
options_json.is_some(); specifically update the block that checks
self.configured to instead: 1) if options_json.is_some() and the stored
configured pair (e.g., self.configured_rule and self.configured_options) does
not match (rule_name, options_json), call self.bindings.call_configure(&mut
self.store, rule_name, options_json) and update the stored pair, or 2) remove
the sticky boolean and always call call_configure when options_json.is_some() so
each check() can reconfigure for the given rule_name and options_json.
In `@crates/biome_wasm_plugin/src/host_state.rs`:
- Around line 426-433: intern currently always appends a new handle so
identities are unstable; change intern (and related storage) to canonicalize
nodes by maintaining a lookup (e.g., HashMap<ConcreteNode, u32> or a
node-id-to-handle map) and return the existing handle when the node is already
interned instead of pushing a duplicate into self.nodes; keep self.nodes as the
canonical vector of nodes and update intern to consult/insert into the map
before push (also ensure any code using node_parent(), node_children(), scope_*
can rely on stable handles returned by intern).
- Around line 47-52: HostState currently has an unsafe impl Send which lets
Store<HostState> appear Send and causes WasmPluginSession to auto-derive Send,
violating the single-threaded guarantee; to fix, add an explicit negative impl
"impl !Send for WasmPluginSession" in the same module (near the
WasmPluginSession definition) so the session cannot be moved across threads,
while keeping the unsafe impl Send for HostState and the existing safety
comment; reference the types HostState, WasmPluginSession, Store<HostState>,
ConcreteNode/SyntaxNode to locate the relevant code.
In `@e2e-tests/wasm-plugins/plugins/css-style-conventions/src/lib.rs`:
- Around line 28-33: The current PATTERN static uses OnceLock which freezes the
first override and prevents later config changes; replace it with an
overwritable synchronization primitive (e.g., a Mutex<Option<String>> or
RwLock<String>) so the active pattern can be updated per-check or per-session.
Change the static PATTERN declaration from OnceLock<String> to the chosen lock
type, update get_pattern() to read the locked value (falling back to
DEFAULT_PATTERN), and add/replace any set logic (e.g., a set_pattern or update
path used where set() was called around the previous OnceLock usage) to acquire
a write lock and replace the stored pattern so subsequent checks see the new
regex. Ensure you update all uses (including the code referenced around the
previous set() calls) to use the new get/set helpers instead of OnceLock::set().
---
Minor comments:
In `@crates/biome_js_analyze/tests/wasm_plugin_tests.rs`:
- Around line 1-5: Add the crate-level attribute to gate these tests behind the
same feature as the parallel test: insert #![cfg(feature = "wasm_plugin")] at
the top of the wasm_plugin_tests.rs file (above the existing module doc comment)
so the Analyzer pipeline integration tests run only when the "wasm_plugin"
feature is enabled; mirror the gating used by
biome_plugin_loader/tests/wasm_plugin_integration.rs for consistency.
In `@crates/biome_plugin_loader/src/analyzer_wasm_plugin.rs`:
- Around line 265-272: The code currently inserts the session into the map
before calling session.check_current, which caches sessions even when
check_current returns Err; move the map.insert(self.plugin_id, session) so it
only runs after check_current returns Ok (or alternatively call check_current
first and insert the session on success), ensuring the initial failure path does
not cache the session; reference the session variable, check_current method,
map.insert call, and self.plugin_id when making the change.
In `@crates/biome_plugin_loader/src/configuration.rs`:
- Around line 200-216: The deserialize implementation for
PluginRulePlainConfiguration currently swallows unknown severity strings; update
the match's fallback branch in Deserializable::deserialize to call
ctx.report_error (or the appropriate method on DeserializationContext) with a
clear message that includes the invalid value (from text.text()) and the
configuration key/name, then return None (or a fallback) — i.e., in the "_" arm
of PluginRulePlainConfiguration::deserialize report the unrecognised severity
via ctx.report_error including the provided name and value so the user gets a
diagnostic.
In `@crates/biome_plugin_loader/src/plugin_cache.rs`:
- Around line 38-42: Add a unit test in plugin_cache.rs that exercises the
fallback suffix match logic around the found variable (the
map.get(&path_buf).or_else(...).find(...).map(...)) to ensure ends_with() does
not produce false positives when two plugins share the same filename in
different directories; specifically construct a map with two distinct keys that
have the same file name in different parent directories and assert that looking
up a relative vs absolute path_buf returns only the intended plugin (and not the
other), and include a negative case where a different directory with same
filename must not match. Ensure the test targets the suffix-matching branch so
we catch regressions in the fallback behavior.
In `@crates/biome_service/src/file_handlers/json.rs`:
- Around line 723-737: The reparse callback currently uses
JsonParserOptions::default() when calling biome_json_parser::parse_json inside
process_fix_all.process_action_with_reparse, which drops file_source-specific
settings; replace the default with a JsonParserOptions built from
file_source.allow_comments() and file_source.allow_trailing_commas() (e.g.,
construct JsonParserOptions { allow_comments: file_source.allow_comments(),
allow_trailing_commas: file_source.allow_trailing_commas(), ..Default::default()
}) so the parser honors JSONC/trailing-comma settings during fixes.
In `@e2e-tests/wasm-plugins/test.sh`:
- Line 1: Add a POSIX-compatible shebang as the first line of the script so the
interpreter is defined (e.g. use "/usr/bin/env bash"), placing it before the
existing "set -eu" line in e2e-tests/wasm-plugins/test.sh; ensure the shebang is
the very first byte of the file and that "set -eu" remains immediately after it.
---
Nitpick comments:
In `@crates/biome_analyze/src/analyzer_plugin.rs`:
- Around line 241-265: Extract the duplicated per-rule and domain filtering into
a helper function (e.g., fn should_skip_plugin(plugin: &dyn AnalyzerPlugin,
options: &AnalyzerOptions) -> bool) and replace the inline logic in
AnalyzerPluginVisitor (the current block using self.plugin.rule_name(),
ctx.options.plugin_rule_override(...), ctx.options.linter_domains(),
PluginDomainFilter checks, and self.plugin.is_recommended()) and the duplicated
block in BatchPluginVisitor with calls to this helper; ensure the helper mirrors
the existing checks (returns true when override.disabled, when a domain is
Disabled, or when domain is Recommended and plugin.is_recommended() is false)
and keep all existing references to plugin.rule_name(),
options.plugin_rule_override(...), options.linter_domains(), and
PluginDomainFilter to preserve behavior.
- Around line 460-466: The current closure using t.as_bytes() and
w.eq_ignore_ascii_case(t_bytes) performs ASCII-only case-insensitive matching
and won't handle Unicode case folding (e.g., ß vs SS); either document this
ASCII-only limitation in the WIT/type docs for the triggers input, or change the
comparison in the triggers.iter().any closure (the place that builds t_bytes and
calls eq_ignore_ascii_case) to use Unicode-aware folding (e.g.,
casefold/Unicode-normalize both source and trigger strings before comparing, or
use a crate that provides Unicode case-folding) so non-ASCII case mappings are
handled correctly.
In `@crates/biome_js_analyze/tests/wasm_plugin_tests.rs`:
- Around line 140-150: The test wasm_plugin_suppression_comment_does_not_crash
currently only asserts "no crash" but lacks a TODO/link to track full
suppression support; update the test's comment (inside function
wasm_plugin_suppression_comment_does_not_crash) to include a clear TODO or FIXME
with the tracking issue ID (or internal ticket/link) and a short note about when
to strengthen the assertion to check suppression effectiveness, so future
authors know to revisit this behavior.
In `@crates/biome_plugin_loader/src/analyzer_wasm_plugin.rs`:
- Around line 253-256: Add a short clarifying comment above the
semantic_model/module_resolver extraction explaining that ModuleResolver is
registered as an Option<Arc<ModuleResolver>> to represent a possibly-missing
resolver and why we call services.get_service::<Option<Arc<ModuleResolver>>>()
followed by and_then(|opt| opt.as_ref().map(Arc::clone)) to safely clone the Arc
only when present; reference the variables/functions involved (semantic_model,
module_resolver, ModuleResolver, get_service) so future readers understand the
nested Option handling and cloning intent.
In `@crates/biome_plugin_loader/src/configuration.rs`:
- Around line 275-285: The PluginPathWithOptions struct declares options as a
required String but code elsewhere treats it as defaulting to "{}"; change the
field to Option<String> (pub options: Option<String>) and update any
deserialization/default logic and call sites that read
PluginPathWithOptions::options to treat None as "{}" (or generate a default when
constructing/serializing). Update the serde attributes or helper functions that
currently inject the "{}" default so behavior is explicit, and adjust tests and
uses of PluginPathWithOptions, PluginPathWithOptions::options, and any code
expecting a raw JSON string accordingly.
In `@crates/biome_plugin_loader/src/diagnostics.rs`:
- Around line 162-166: Introduce a small constructor to avoid exposing internal
fields: make the fields message: MessageAndDescription and source: Option<Error>
private (remove pub(crate) on them) and add a public(crate)
CompileDiagnostic::new(message: MessageAndDescription, source: Option<Error>) ->
CompileDiagnostic that constructs and returns the struct; update all call sites
that currently construct CompileDiagnostic with struct literal syntax to call
CompileDiagnostic::new instead so construction is centralized and you can keep
invariants enforced in one place.
In `@crates/biome_plugin_loader/src/lib.rs`:
- Around line 57-63: The current plugin_name extraction using
plugin_path.file_stem().unwrap_or(plugin_path.as_str()).to_string() can yield
confusing names for edge paths (e.g., "/" or empty); update the logic around
plugin_path and plugin_name to prefer file_stem(), then file_name(), then a
lossy string of the path, and finally a stable fallback like "<unknown-plugin>"
so diagnostics never show an empty or root path string; locate the code
manipulating plugin_path and plugin_name (the plugin_name assignment) and
replace the single unwrap_or fallback with this prioritized sequence using
file_stem(), file_name(), Path::to_string_lossy(), and a constant default.
In `@crates/biome_plugin_sdk/build.rs`:
- Around line 83-100: The loop that scans braces (variable names: body, depth,
end_pos, using body.char_indices()) can underflow when encountering an unmatched
'}' — change the decrement to a guarded operation: before doing depth -= 1 check
if depth > 0 and handle the malformed input (return an Err, panic with a clear
message, or break), or use a saturating/checked subtraction to avoid underflow;
ensure you propagate or log an error when an unmatched closing brace is found
instead of allowing depth to underflow and continue, and keep setting end_pos
when depth reaches zero as currently implemented.
- Around line 136-150: The parsing currently sets discriminant from explicit
values (val_str -> discriminant) before honoring skip_next, causing a skipped
#[doc(hidden)] variant with an explicit discriminant to incorrectly advance
discriminant; change the logic in build.rs so you check skip_next first (or skip
any parsing when skip_next is true) — i.e., inspect skip_next before the block
that uses trimmed.strip_prefix(variant_name) and parsing val_str, and only
parse/set discriminant when the variant is not being skipped; reference
variables/functions: skip_next, discriminant, trimmed, variant_name, val_str.
In `@crates/biome_plugin_sdk/src/options.rs`:
- Around line 10-25: The example doctest is marked "ignore" and should be an
executable doctest: change the code fence from ```ignore to a normal Rust
doctest (remove "ignore"), construct a small JSON string (e.g. assign a literal
to options_json), call the public helpers get_string, get_number, get_bool,
get_string_array from options and add assert_eq! (or assert!) checks validating
returned values (including unwrap_or fallback behavior where relevant) so the
example compiles and runs as a doctest; reference the functions get_string,
get_number, get_bool, and get_string_array in your example to keep the API
exercised.
- Around line 193-264: Add unit tests that cover nested keys and UTF-8 to
prevent regressions: create tests (e.g., test_key_nested_not_matched and
test_utf8_keys_and_values) that feed JSON with nested objects like {"outer":
{"key":"bad"}, "key":"good"} and assert
get_string/get_number/get_bool/get_string_array only return the top-level "key"
value (not the nested one), and tests that use UTF-8 in keys and values (e.g.,
keys/values with "π", "é", emojis) to assert get_string/get_string_array
correctly return Unicode strings and get_number/get_bool behave as expected; add
analogous cases for missing keys and empty arrays to ensure existing functions
(get_string, get_string_array, get_number, get_bool) handle non-ASCII input and
nested structures.
In `@e2e-tests/wasm-plugins/plugins/boolean-naming/src/lib.rs`:
- Around line 61-73: The RuleMetadata returned by rule_metadata currently uses a
placeholder version "0.0.0"; update the version field in the rule_metadata
function (the RuleMetadata struct instance) to a non-placeholder value—either
set it to "next" to match Biome's convention for unreleased rules or add a short
comment/README note instructing plugin authors to replace "0.0.0" before
publishing so RuleMetadata::version is accurate for the boolean-naming plugin.
- Around line 55-59: The configure function currently discards PATTERN.set(...)
result so reconfiguration is silently ignored; update configure (and its use of
biome_plugin_sdk::options::get_string and options_json) to handle the Err case
from PATTERN.set: either allow updates by replacing the existing pattern (e.g.,
use the container's replace/update API or get_mut and overwrite the stored
value) or explicitly return/log an error/warning when set fails so callers know
configuration was ignored; also consider adding a doc comment on
configure/PATTERN if single-configuration-per-plugin-lifetime is intentional.
In `@justfile`:
- Around line 286-295: The justfile recipe build-test-wasm-plugins hardcodes
three plugin paths, which will require maintenance as plugins are added; change
the recipe to discover and build plugins dynamically by iterating over
directories under e2e-tests/wasm-plugins/plugins (or delegate to the existing
build.rs) so each plugin's Cargo.toml is built with cargo build --target
wasm32-wasip2 --release and its produced .wasm artifact is copied into
crates/biome_plugin_loader/tests/fixtures; update the recipe name
build-test-wasm-plugins and the commands that reference specific plugin
directories (e.g., the three cargo build and cp lines) to use a loop over plugin
dirs and copy the release .wasm outputs instead of hardcoded paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb81395f-60f7-4fe2-bd36-77285f04deaa
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockand included by**e2e-tests/wasm-plugins/plugins/boolean-naming/Cargo.lockis excluded by!**/*.lockand included by**e2e-tests/wasm-plugins/plugins/css-style-conventions/Cargo.lockis excluded by!**/*.lockand included by**e2e-tests/wasm-plugins/plugins/json-naming/Cargo.lockis excluded by!**/*.lockand 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 (79)
.changeset/metal-bees-float.md.changeset/wasm-plugin-system.md.gitignoreCargo.tomlcrates/biome_analyze/CONTRIBUTING.mdcrates/biome_analyze/Cargo.tomlcrates/biome_analyze/src/analyzer_plugin.rscrates/biome_analyze/src/diagnostics.rscrates/biome_analyze/src/lib.rscrates/biome_analyze/src/options.rscrates/biome_analyze/src/rule.rscrates/biome_analyze/src/signals.rscrates/biome_analyze/src/suppression_action.rscrates/biome_cli/Cargo.tomlcrates/biome_css_analyze/tests/spec_tests.rscrates/biome_diagnostics/CONTRIBUTING.mdcrates/biome_diagnostics/src/context.rscrates/biome_diagnostics/src/diagnostic.rscrates/biome_diagnostics/src/display.rscrates/biome_diagnostics/src/error.rscrates/biome_diagnostics/src/serde.rscrates/biome_js_analyze/Cargo.tomlcrates/biome_js_analyze/tests/spec_tests.rscrates/biome_js_analyze/tests/wasm_plugin_tests.rscrates/biome_js_semantic/src/semantic_model/model.rscrates/biome_lsp/src/utils.rscrates/biome_plugin_loader/Cargo.tomlcrates/biome_plugin_loader/README.mdcrates/biome_plugin_loader/build.rscrates/biome_plugin_loader/src/analyzer_grit_plugin.rscrates/biome_plugin_loader/src/analyzer_js_plugin.rscrates/biome_plugin_loader/src/analyzer_wasm_plugin.rscrates/biome_plugin_loader/src/configuration.rscrates/biome_plugin_loader/src/diagnostics.rscrates/biome_plugin_loader/src/lib.rscrates/biome_plugin_loader/src/plugin_cache.rscrates/biome_plugin_loader/tests/wasm_plugin_integration.rscrates/biome_plugin_sdk/Cargo.tomlcrates/biome_plugin_sdk/README.mdcrates/biome_plugin_sdk/build.rscrates/biome_plugin_sdk/src/css_kinds.rscrates/biome_plugin_sdk/src/js_kinds.rscrates/biome_plugin_sdk/src/json_kinds.rscrates/biome_plugin_sdk/src/lib.rscrates/biome_plugin_sdk/src/options.rscrates/biome_plugin_sdk/wit/biome-plugin.witcrates/biome_plugin_sdk_macros/Cargo.tomlcrates/biome_plugin_sdk_macros/README.mdcrates/biome_plugin_sdk_macros/src/lib.rscrates/biome_service/Cargo.tomlcrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/graphql.rscrates/biome_service/src/file_handlers/html.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/file_handlers/json.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/workspace/server.rscrates/biome_wasm_plugin/Cargo.tomlcrates/biome_wasm_plugin/README.mdcrates/biome_wasm_plugin/src/engine.rscrates/biome_wasm_plugin/src/host_state.rscrates/biome_wasm_plugin/src/lib.rscrates/biome_wasm_plugin/tests/engine_integration.rscrates/biome_wasm_plugin/tests/kind_constants_validation.rscrates/biome_wasm_plugin/tests/sdk_kinds_sync.rscrates/biome_wasm_plugin/wit/biome-plugin.wite2e-tests/wasm-plugins/README.mde2e-tests/wasm-plugins/biome.jsone2e-tests/wasm-plugins/demo.csse2e-tests/wasm-plugins/demo.jse2e-tests/wasm-plugins/demo.jsone2e-tests/wasm-plugins/plugins/boolean-naming/Cargo.tomle2e-tests/wasm-plugins/plugins/boolean-naming/src/lib.rse2e-tests/wasm-plugins/plugins/css-style-conventions/Cargo.tomle2e-tests/wasm-plugins/plugins/css-style-conventions/src/lib.rse2e-tests/wasm-plugins/plugins/json-naming/Cargo.tomle2e-tests/wasm-plugins/plugins/json-naming/src/lib.rse2e-tests/wasm-plugins/test.shjustfile
.changeset/metal-bees-float.md
Outdated
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Added WASM Component Model plugin system with multi-language support (JavaScript, CSS, JSON), per-rule options, suppression actions, and rule name display in diagnostic headers. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if wasm_plugin is a default feature in the main Biome package
rg -n "default\s*=" Cargo.toml | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
find . -name "Cargo.toml" -type f | head -5Repository: biomejs/biome
Length of output: 215
🏁 Script executed:
rg -A 10 "wasm_plugin" --type tomlRepository: biomejs/biome
Length of output: 4852
🏁 Script executed:
rg -A 5 "^\[features\]" ./Cargo.tomlRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
rg -A 5 "^\[features\]" ./crates/biome_cli/Cargo.tomlRepository: biomejs/biome
Length of output: 192
🏁 Script executed:
cat ./Cargo.toml | grep -A 20 "\[features\]"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
cat ./crates/biome_cli/Cargo.toml | grep -A 15 "\[features\]"Repository: biomejs/biome
Length of output: 209
Use user-friendly language and clarify this is an opt-in feature.
The description uses technical jargon ("WASM Component Model", "suppression actions", "diagnostic headers") that won't mean much to end users. Also, wasm_plugin is an optional compile-time feature, not available by default—this must be mentioned so users understand whether they can actually use it.
Try something like: "Added experimental support for custom lint rules using WebAssembly plugins (requires compiling with the wasm_plugin feature flag). You can now write your own rules for JavaScript, CSS, and JSON."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/metal-bees-float.md at line 5, Revise the changelog sentence to
use user-friendly, non-technical language and clearly mark the feature as
opt-in/experimental: replace the phrase "WASM Component Model plugin system with
multi-language support (JavaScript, CSS, JSON), per-rule options, suppression
actions, and rule name display in diagnostic headers" with a short,
plain-English line such as "Added experimental support for custom lint rules via
WebAssembly plugins (requires compiling with the wasm_plugin feature flag); you
can now write rules for JavaScript, CSS, and JSON." Ensure the term
"wasm_plugin" is present to indicate it is a compile-time opt-in feature and
keep the tone accessible for end users.
Author
There was a problem hiding this comment.
Fixed — deleted the duplicate changeset.
Comment on lines
+794
to
815
| let results: Vec<_> = plugins | ||
| .iter() | ||
| .collect::<Vec<_>>() | ||
| .par_iter() | ||
| .map(|plugin_config| { | ||
| let plugin_path = plugin_config.path(); | ||
| let options_json = plugin_config.options_json().map(String::from); | ||
| ( | ||
| plugin_path.to_owned(), | ||
| BiomePlugin::load(self.fs.clone(), plugin_path, base_path, options_json), | ||
| ) | ||
| }) | ||
| .collect(); | ||
|
|
||
| let mut diagnostics = Vec::new(); | ||
| let plugin_cache = PluginCache::default(); | ||
|
|
||
| for plugin_config in plugins.iter() { | ||
| match plugin_config { | ||
| PluginConfiguration::Path(plugin_path) => { | ||
| match BiomePlugin::load(self.fs.clone(), plugin_path, base_path) { | ||
| Ok((plugin, _)) => { | ||
| plugin_cache.insert_plugin(plugin_path.clone().into(), plugin); | ||
| } | ||
| Err(diagnostic) => diagnostics.push(diagnostic), | ||
| } | ||
| for (path, result) in results { | ||
| match result { | ||
| Ok((plugin, _)) => { | ||
| plugin_cache.insert_plugin(path.into(), plugin); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'struct PluginCache|fn insert_plugin|fn get_analyzer_plugins|options_json|path\(' crates/biome_plugin_loader crates/biome_service/src/workspace/server.rsRepository: biomejs/biome
Length of output: 50369
🏁 Script executed:
fd -type f -name "*.rs" | xargs rg -l "PluginCache" | head -5Repository: biomejs/biome
Length of output: 373
🏁 Script executed:
cat crates/biome_plugin_loader/src/plugin_cache.rsRepository: biomejs/biome
Length of output: 3589
🏁 Script executed:
rg -A 10 "fn load" crates/biome_plugin_loader/src/lib.rs | head -40Repository: biomejs/biome
Length of output: 1486
🏁 Script executed:
rg -A 50 "pub fn load" crates/biome_plugin_loader/src/lib.rs | grep -A 50 "fn load" | head -70Repository: biomejs/biome
Length of output: 1933
🏁 Script executed:
rg -A 100 "pub fn load" crates/biome_plugin_loader/src/lib.rs | grep -B 5 -A 5 "options_json"Repository: biomejs/biome
Length of output: 1354
Plugin options must be included in the cache key to prevent configuration collisions.
Lines 800–815 load each plugin with its own options_json, but cache by path only (line 811). If two plugin configurations reference the same binary with different options, the second will overwrite the first in the cache, causing both to use a single cached instance. Since AnalyzerWasmPlugin::load consumes options_json, this creates silent configuration loss.
Include options as part of the cache identity, either by using (Utf8PathBuf, Option<String>) as the key or a combined hash.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_service/src/workspace/server.rs` around lines 794 - 815, The
cache currently keys plugins only by path causing different configurations to
collide; modify the caching logic so the cache key includes plugin options
(e.g., use a key type of (Utf8PathBuf, Option<String>) or a combined hash of
path+options) when inserting and looking up plugins in PluginCache: update the
code around the BiomePlugin::load call and plugin_cache.insert_plugin to pass
the options_json alongside the path (or compute a stable combined key) and
adjust PluginCache APIs (insert_plugin/get_plugin) to accept that composite key
so each distinct options_json produces a distinct cache entry.
Author
There was a problem hiding this comment.
Not applicable — plugin options are baked into BiomePlugin at load time. The same path can't have different options in a single session. Additionally, we now call bump_wasm_generation() at the start of load_plugins(), which invalidates all cached sessions when config changes.
Comment on lines
+402
to
+409
| // Configure on first use within this session. | ||
| if !self.configured { | ||
| if let Some(options) = options_json { | ||
| self.bindings | ||
| .call_configure(&mut self.store, rule_name, options)?; | ||
| } | ||
| self.configured = true; | ||
| } |
Contributor
There was a problem hiding this comment.
Configuration gets sticky after the first check().
This session caches a single boolean. A first call with None options permanently suppresses later configure() calls, and reusing the same session for another rule keeps the first rule's configuration alive. Track the configured (rule_name, options_json) pair, or reconfigure per call.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_wasm_plugin/src/engine.rs` around lines 402 - 409, The code
currently uses a single boolean self.configured to gate calling
self.bindings.call_configure, causing the first check() call (even with None
options_json) to permanently skip future configure calls and to stick
configuration to the first rule_name; change the logic to track configuration
per invocation by storing and comparing the configured pair (rule_name and
options_json) or simply call configure on every check when
options_json.is_some(); specifically update the block that checks
self.configured to instead: 1) if options_json.is_some() and the stored
configured pair (e.g., self.configured_rule and self.configured_options) does
not match (rule_name, options_json), call self.bindings.call_configure(&mut
self.store, rule_name, options_json) and update the stored pair, or 2) remove
the sticky boolean and always call call_configure when options_json.is_some() so
each check() can reconfigure for the given rule_name and options_json.
Author
There was a problem hiding this comment.
Not applicable — configuration is not sticky. Each rule gets its own WasmPluginSession keyed by plugin_id (which includes the rule name). The configured flag is per-rule-per-file, and sessions are invalidated via generation-based cache cleanup when load_plugins() is called.
e2e-tests/wasm-plugins/plugins/css-style-conventions/src/lib.rs
Outdated
Show resolved
Hide resolved
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_plugin_loader/tests/wasm_plugin_integration.rs (1)
278-306: JSON tests lack metadata verification.CSS and JS plugins both have metadata tests verifying
language()andphase(). The JSON plugin tests skip this. Worth adding for consistency, but not blocking.🔧 Optional: Add JSON plugin metadata test
#[test] fn load_json_plugin() { let path = fixture_path("json_naming.wasm"); let plugins = AnalyzerWasmPlugin::load(path.as_ref(), "json_naming", None) .expect("should load JSON plugin"); assert_eq!(plugins.len(), 1, "Expected 1 rule from json-naming"); } +#[test] +fn json_plugin_metadata() { + let plugins = AnalyzerWasmPlugin::load( + fixture_path("json_naming.wasm").as_ref(), + "json_naming", + None, + ) + .unwrap(); + + assert_eq!(plugins[0].language(), PluginTargetLanguage::Json); +} + #[test] fn json_plugin_evaluate() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/tests/wasm_plugin_integration.rs` around lines 278 - 306, Add assertions to the JSON plugin tests to verify the plugin metadata like the other tests do: after loading the plugin via AnalyzerWasmPlugin::load in load_json_plugin and/or json_plugin_evaluate, call the plugin metadata accessors (e.g., plugins[0].metadata().language() and plugins[0].metadata().phase()) and assert they return the expected values for the JSON plugin (match the values used in the CSS/JS tests), so the tests check both language() and phase() consistency.
🤖 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_plugin_loader/tests/wasm_plugin_integration.rs`:
- Around line 278-306: Add assertions to the JSON plugin tests to verify the
plugin metadata like the other tests do: after loading the plugin via
AnalyzerWasmPlugin::load in load_json_plugin and/or json_plugin_evaluate, call
the plugin metadata accessors (e.g., plugins[0].metadata().language() and
plugins[0].metadata().phase()) and assert they return the expected values for
the JSON plugin (match the values used in the CSS/JS tests), so the tests check
both language() and phase() consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd0211b4-db1a-4ff7-8303-73abb703531d
📒 Files selected for processing (2)
crates/biome_js_analyze/tests/wasm_plugin_tests.rscrates/biome_plugin_loader/tests/wasm_plugin_integration.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/wasm_plugin_tests.rs
Hideart
added a commit
to Hideart/biome
that referenced
this pull request
Mar 9, 2026
- Upgrade wasmtime 30→42 and wit-bindgen 0.39→0.53 - Add overlapping edit guard in plugin actions - Apply fixKind override in diagnostic path - Add serde skip_serializing_if on subcategory - Set publish = false on SDK crates - Use workspace dependency convention for SDK crates - Add JSON parser depth tracking in options parser - Fix UTF-8 string parsing in options parser - Use settings-based parser options in reparse closures - Replace OnceLock with RefCell in example plugin - Add end-offset guard in semantic model binding_by_node - Derive grit plugin rule name from file stem - Add subcategory to grit plugin diagnostics - Add PhantomData<Rc<()>> for !Send on WasmPluginSession - Implement handle dedup using SyntaxElementKey - Add generation-based thread-local session cache cleanup - Deduplicate plugin configs by path in as_all_plugins - Restore reverted sortBareImports feature - Remove duplicate changeset, fix markdown tables - Remove unused dev-deps from biome_wasm_plugin - Update CLI snapshots for subcategory display - Collapse nested if for clippy in lsp utils Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
crates/biome_wasm_plugin/src/engine.rs (1)
407-414:⚠️ Potential issue | 🟠 MajorConfiguration still sticks to the first check.
A first call with
Noneoptions permanently suppresses laterconfigure()calls, and reusing the same session across rules/options keeps the first rule’s config alive. Please key this by(rule_name, options_json)or reconfigure whenever those inputs change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_wasm_plugin/src/engine.rs` around lines 407 - 414, The current boolean self.configured causes the first call (even with None) to block future configure() calls; change the gate to track the last applied config and only skip if identical: replace self.configured with something like self.last_config: Option<(String, Option<String>)> (or store rule_name and options_json serialization) and in the configure block compare (rule_name.to_string(), options_json.clone() or serialised form) against self.last_config; if different, call self.bindings.call_configure(&mut self.store, rule_name, options_json)? and then set self.last_config to the new tuple so subsequent calls with the same rule/options are skipped but different rule_name or options_json trigger reconfiguration.
🧹 Nitpick comments (2)
crates/biome_plugin_loader/src/analyzer_grit_plugin.rs (1)
92-104: Consider graceful handling if the downcast chain fails.The
root.unwrap()on line 104 will panic ifdowncast_reforas_send()returnsNone. Whilst the analyzer contract should ensure language matching, a defensive approach might return an error diagnostic instead of panicking.💡 Defensive alternative
- let parse = AnyParse::Node(NodeParse::new(root.unwrap(), vec![])); + let Some(root) = root else { + return PluginEvaluationResult::from_diagnostics(vec![ + RuleDiagnostic::new( + category!("plugin"), + None::<TextRange>, + markup!(<Emphasis>{name}</Emphasis>" received unexpected node type"), + ) + .subcategory(name.to_string()), + ]); + }; + let parse = AnyParse::Node(NodeParse::new(root, vec![]));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_plugin_loader/src/analyzer_grit_plugin.rs` around lines 92 - 104, The current code unwraps root (from the downcast chain for PluginTargetLanguage::JavaScript/Css/Json) which can panic if downcast_ref or as_send() yields None; replace the unwrap with a graceful check: after computing root (the variable built from JsSyntaxNode/CssSyntaxNode/JsonSyntaxNode), match on Option and on None return an error diagnostic or a Result::Err describing a type-mismatch/failure (or construct an AnyParse error variant) instead of calling root.unwrap(); update the call site that constructs AnyParse::Node(NodeParse::new(...)) to only run NodeParse::new when root is Some and surface a clear diagnostic including the plugin target via self.language() so failures are recoverable and non-panicking.crates/biome_diagnostics/src/serde.rs (1)
24-25: Please add one round-trip test forSome(subcategory).The current fixture only nails the omitted-field case, so a regression in serialising or deserialising a real subcategory would stroll past unnoticed.
Also applies to: 43-43, 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_diagnostics/src/serde.rs` around lines 24 - 25, Add a unit test that round-trips a value with subcategory = Some("...") through serde (serialize then deserialize) and asserts equality to catch regressions; specifically create a test (e.g., test_subcategory_round_trip) that constructs the struct containing the #[serde(default, skip_serializing_if = "Option::is_none")] subcategory: Option<String> set to Some("foo"), serialize it to the fixture format used in this crate, deserialize it back and assert the resulting struct equals the original, and add analogous round-trip tests for the other two occurrences referenced (the fields at the other locations) to ensure both serialization and deserialization preserve Some(subcategory).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/flat-beers-battle.md:
- Around line 5-6: The changeset description for the new organizeImports option
`sortBareImports` is inverted: update the prose so it states that bare imports
are sorted within other imports when `sortBareImports` is `true` (not `false`)
to match the example and implementation; specifically edit the sentence that
currently says "when set to `false`" to say "when set to `true`" and verify the
rest of the description and example reference the `sortBareImports` option
consistently with the `organizeImports` feature.
In `@crates/biome_wasm_plugin/src/engine.rs`:
- Around line 556-575: The load_from_cache function currently calls unsafe
wasmtime::component::Component::deserialize on bytes from a user-writable plugin
directory (cache_path) which is untrusted; instead prevent deserializing
untrusted bytes by either moving cache files to an app-owned,
integrity-protected directory before using Component::deserialize or, simpler
and safer, stop deserializing here and return None so the caller (e.g., the
compile path) will recompile the component; update load_from_cache to check that
cache_path is in a trusted app-owned location (or remove the unsafe deserialize
branch) and only call Component::deserialize when that trust check passes,
otherwise return None to force recompilation.
In `@crates/biome_wasm_plugin/src/host_state.rs`:
- Around line 572-578: The file_comments() implementation only walks the first
node's subtree (using self.nodes.first()), so it misses comments elsewhere
because handle 0 is reset per matched node; update file_comments in
host_state.rs to traverse the true file root instead of self.nodes.first() —
locate where root.collect_comments() is called and change it to collect comments
from the stored file root (e.g., a dedicated file_root field or the original
root node) or iterate all top-level nodes consistently to aggregate comments
across the entire file so suppression/comments are discovered globally.
---
Duplicate comments:
In `@crates/biome_wasm_plugin/src/engine.rs`:
- Around line 407-414: The current boolean self.configured causes the first call
(even with None) to block future configure() calls; change the gate to track the
last applied config and only skip if identical: replace self.configured with
something like self.last_config: Option<(String, Option<String>)> (or store
rule_name and options_json serialization) and in the configure block compare
(rule_name.to_string(), options_json.clone() or serialised form) against
self.last_config; if different, call self.bindings.call_configure(&mut
self.store, rule_name, options_json)? and then set self.last_config to the new
tuple so subsequent calls with the same rule/options are skipped but different
rule_name or options_json trigger reconfiguration.
---
Nitpick comments:
In `@crates/biome_diagnostics/src/serde.rs`:
- Around line 24-25: Add a unit test that round-trips a value with subcategory =
Some("...") through serde (serialize then deserialize) and asserts equality to
catch regressions; specifically create a test (e.g.,
test_subcategory_round_trip) that constructs the struct containing the
#[serde(default, skip_serializing_if = "Option::is_none")] subcategory:
Option<String> set to Some("foo"), serialize it to the fixture format used in
this crate, deserialize it back and assert the resulting struct equals the
original, and add analogous round-trip tests for the other two occurrences
referenced (the fields at the other locations) to ensure both serialization and
deserialization preserve Some(subcategory).
In `@crates/biome_plugin_loader/src/analyzer_grit_plugin.rs`:
- Around line 92-104: The current code unwraps root (from the downcast chain for
PluginTargetLanguage::JavaScript/Css/Json) which can panic if downcast_ref or
as_send() yields None; replace the unwrap with a graceful check: after computing
root (the variable built from JsSyntaxNode/CssSyntaxNode/JsonSyntaxNode), match
on Option and on None return an error diagnostic or a Result::Err describing a
type-mismatch/failure (or construct an AnyParse error variant) instead of
calling root.unwrap(); update the call site that constructs
AnyParse::Node(NodeParse::new(...)) to only run NodeParse::new when root is Some
and surface a clear diagnostic including the plugin target via self.language()
so failures are recoverable and non-panicking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f3067e6-5d7c-4aca-9b8e-dedde4718700
⛔ Files ignored due to path filters (15)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_cli/tests/snapshots/main_cases_monorepo/plugins_from_root_config_work_in_child_config_extends_root.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_monorepo/plugins_in_child_config_with_extends_root.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_json_plugin.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_plugin_diagnostic_offset_in_vue_file.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_plugin_suppressions.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/source/organizeImports/chunk-because-of-bare-imports.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/source/organizeImports/custom-order-with-bare-imports.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-bare-import-attributes.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-bare-imports.js.snapis excluded by!**/*.snapand included by**e2e-tests/wasm-plugins/plugins/boolean-naming/Cargo.lockis excluded by!**/*.lockand included by**e2e-tests/wasm-plugins/plugins/css-style-conventions/Cargo.lockis excluded by!**/*.lockand included by**e2e-tests/wasm-plugins/plugins/json-naming/Cargo.lockis excluded by!**/*.lockand 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 (36)
.changeset/flat-beers-battle.mdCargo.tomlcrates/biome_analyze/src/signals.rscrates/biome_diagnostics/src/serde.rscrates/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/chunk-because-of-bare-imports.jscrates/biome_js_analyze/tests/specs/source/organizeImports/custom-order-with-bare-imports.jscrates/biome_js_analyze/tests/specs/source/organizeImports/custom-order-with-bare-imports.options.jsoncrates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-bare-import-attributes.jscrates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-bare-imports.jscrates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-bare-imports.options.jsoncrates/biome_js_semantic/src/semantic_model/model.rscrates/biome_lsp/src/utils.rscrates/biome_plugin_loader/README.mdcrates/biome_plugin_loader/src/analyzer_grit_plugin.rscrates/biome_plugin_loader/src/analyzer_wasm_plugin.rscrates/biome_plugin_loader/src/lib.rscrates/biome_plugin_sdk/Cargo.tomlcrates/biome_plugin_sdk/README.mdcrates/biome_plugin_sdk/src/lib.rscrates/biome_plugin_sdk/src/options.rscrates/biome_plugin_sdk_macros/Cargo.tomlcrates/biome_rule_options/src/organize_imports.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/html.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/settings.rscrates/biome_service/src/workspace/server.rscrates/biome_wasm_plugin/Cargo.tomlcrates/biome_wasm_plugin/src/engine.rscrates/biome_wasm_plugin/src/host_state.rse2e-tests/wasm-plugins/plugins/boolean-naming/Cargo.tomle2e-tests/wasm-plugins/plugins/css-style-conventions/Cargo.tomle2e-tests/wasm-plugins/plugins/css-style-conventions/src/lib.rse2e-tests/wasm-plugins/plugins/json-naming/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (9)
- crates/biome_js_semantic/src/semantic_model/model.rs
- e2e-tests/wasm-plugins/plugins/boolean-naming/Cargo.toml
- crates/biome_plugin_sdk/README.md
- e2e-tests/wasm-plugins/plugins/json-naming/Cargo.toml
- crates/biome_lsp/src/utils.rs
- crates/biome_plugin_sdk/src/lib.rs
- crates/biome_plugin_sdk/src/options.rs
- e2e-tests/wasm-plugins/plugins/css-style-conventions/src/lib.rs
- e2e-tests/wasm-plugins/plugins/css-style-conventions/Cargo.toml
Comment on lines
+556
to
+575
| fn load_from_cache( | ||
| engine: &wasmtime::Engine, | ||
| cache_path: &Path, | ||
| expected_hash: &[u8; 32], | ||
| ) -> Option<wasmtime::component::Component> { | ||
| let data = std::fs::read(cache_path).ok()?; | ||
| if data.len() < 32 { | ||
| return None; | ||
| } | ||
| let (stored_hash, serialized) = data.split_at(32); | ||
| if stored_hash != expected_hash { | ||
| return None; | ||
| } | ||
| // SAFETY: The cache was written by this function using the same | ||
| // engine configuration (verified by the hash which includes | ||
| // engine.precompile_compatibility_hash()). | ||
| #[expect(unsafe_code)] | ||
| unsafe { | ||
| wasmtime::component::Component::deserialize(engine, serialized).ok() | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In wasmtime 42, what are the safety requirements for wasmtime::component::Component::deserialize, and should deserialised component bytes be treated as trusted-only input?
💡 Result:
In Wasmtime 42, wasmtime::component::Component::deserialize is unsafe and inherits the same unsafety contract as wasmtime::Module::deserialize. (docs.wasmtime.dev)
Safety requirements (Component::deserialize)
You must ensure that the input bytes are exact, unmodified bytes previously produced by Wasmtime itself, specifically by:
Engine::precompile_component, orComponent::serialize. (docs.wasmtime.dev)
And you must not pass arbitrary / attacker-controlled bytes, because the blob is only lightly validated and can effectively embed/replace compiled machine code, which can lead to arbitrary code execution (i.e., a host compromise). (docs.wasmtime.dev)
Should deserialized component bytes be treated as trusted-only input?
Yes. Treat serialized/precompiled component artifacts as trusted-only (or at minimum integrity-protected, e.g., from your own cache, signed, MAC’d, etc.). Wasmtime’s safety claim is: if you pass in the exact output of Wasmtime’s own serialization/precompilation APIs, then using deserialize can be considered safe; otherwise, it is not safe to expose to untrusted input. (docs.wasmtime.dev)
Citations:
- 1: https://docs.wasmtime.dev/api/wasmtime/component/struct.Component.html
- 2: https://docs.wasmtime.dev/api/wasmtime/component/struct.Component.html
- 3: https://docs.wasmtime.dev/api/wasmtime/struct.Module.html
- 4: https://docs.wasmtime.dev/api/wasmtime/struct.Module.html
Move the cache to an app-owned directory or recompile instead of deserializing.
Component::deserialize requires bytes to be exact output from Wasmtime's own serialization—but <plugin>.wasm.compiled lives in a user-writable plugin directory and can be pre-seeded or tampered with. The hash prefix only verifies cache consistency, not trust. Per Wasmtime's safety requirements, untrusted bytes can lead to arbitrary code execution on the host. Either cache in an app-owned location with integrity protection, or recompile.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_wasm_plugin/src/engine.rs` around lines 556 - 575, The
load_from_cache function currently calls unsafe
wasmtime::component::Component::deserialize on bytes from a user-writable plugin
directory (cache_path) which is untrusted; instead prevent deserializing
untrusted bytes by either moving cache files to an app-owned,
integrity-protected directory before using Component::deserialize or, simpler
and safer, stop deserializing here and return None so the caller (e.g., the
compile path) will recompile the component; update load_from_cache to check that
cache_path is in a trusted app-owned location (or remove the unsafe deserialize
branch) and only call Component::deserialize when that trust check passes,
otherwise return None to force recompilation.
Author
There was a problem hiding this comment.
The AOT cache is a best-effort optimization. The SAFETY comment and hash verification are already present — the hash includes engine.precompile_compatibility_hash() which covers the engine version and configuration. The cache file is written by Biome itself and the hash is verified before deserialization. If the hash doesn't match, deserialization is skipped. This follows the same pattern as wasmtime's own caching recommendations.
Comment on lines
+572
to
+578
| /// Return all comment trivia in the file (walks the root node's token tree). | ||
| pub(crate) fn file_comments(&self) -> Vec<(String, u32, u32)> { | ||
| let Some(root) = self.nodes.first() else { | ||
| return Vec::new(); | ||
| }; | ||
| root.collect_comments() | ||
| } |
Contributor
There was a problem hiding this comment.
file_comments() only sees the current subtree.
Handle 0 is reset to each matched node, so walking self.nodes.first() misses comments outside that node. That breaks file-wide suppression comments and any rule inspecting whole-file comments. Please keep the file root separately and traverse that here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_wasm_plugin/src/host_state.rs` around lines 572 - 578, The
file_comments() implementation only walks the first node's subtree (using
self.nodes.first()), so it misses comments elsewhere because handle 0 is reset
per matched node; update file_comments in host_state.rs to traverse the true
file root instead of self.nodes.first() — locate where root.collect_comments()
is called and change it to collect comments from the stored file root (e.g., a
dedicated file_root field or the original root node) or iterate all top-level
nodes consistently to aggregate comments across the entire file so
suppression/comments are discovered globally.
Author
There was a problem hiding this comment.
This is by design. Handle 0 is the query node matched by the rule's query_kinds, not the file root. Plugins receive the subtree they asked for. File-wide suppression comments (e.g. // biome-ignore) are handled by the host analyzer infrastructure (biome_analyze), not by individual plugin rules. A plugin rule checking CSS_DECLARATION nodes shouldn't need to see comments from unrelated parts of the file.
Hideart
added a commit
to Hideart/biome
that referenced
this pull request
Mar 9, 2026
- Upgrade wasmtime 30→42 and wit-bindgen 0.39→0.53 - Add overlapping edit guard in plugin actions - Apply fixKind override in diagnostic path - Add serde skip_serializing_if on subcategory - Set publish = false on SDK crates - Use workspace dependency convention for SDK crates - Add JSON parser depth tracking in options parser - Fix UTF-8 string parsing in options parser - Use settings-based parser options in reparse closures - Replace OnceLock with RefCell in example plugin - Add end-offset guard in semantic model binding_by_node - Derive grit plugin rule name from file stem - Add subcategory to grit plugin diagnostics - Add PhantomData<Rc<()>> for !Send on WasmPluginSession - Implement handle dedup using SyntaxElementKey - Add generation-based thread-local session cache cleanup - Deduplicate plugin configs by path in as_all_plugins - Restore reverted sortBareImports feature - Remove duplicate changeset, fix markdown tables - Remove unused dev-deps from biome_wasm_plugin - Update CLI snapshots for subcategory display - Collapse nested if for clippy in lsp utils Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Hideart
force-pushed
the
feat/wasm-plugin-system
branch
from
d15bd28 to
ea6f2f2
Compare
Add multi-language WASM plugin support (JavaScript, CSS, JSON) with per-rule options, suppression actions, rule name display in diagnostic headers, and plugin SDK with proc macros. Includes documentation, changeset, and e2e tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When loading WASM plugins through a biome-manifest.jsonc directory, the options_json was hardcoded to None instead of being forwarded from the plugin configuration. This prevented WASM plugins referenced via manifests from receiving their rule options. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Namespace plugin diagnostics with the plugin name (e.g. "wirex-biome-plugin/ruleName") so editors render them like ESLint rules. Forward subcategory through serde Diagnostic and show "@plugin/rule" in LSP diagnostic codes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add two performance optimizations to the WASM plugin system: 1. Source triggers pre-filter: plugins can export `source-triggers-for-rule` to declare text patterns (e.g. "TODO", "FIXME") that must appear in the source file for the rule to fire. The BatchPluginVisitor skips plugins whose triggers don't match, avoiding unnecessary WASM calls. 2. Batch host calls: add `get-node-info`, `node-children-info`, and `file-comments` WIT functions that return structured records in a single host transition, reducing per-node call overhead for plugins that need multiple properties at once. Together these reduce CPU overhead from ~108% to ~56% on the benchmark monorepo (9 plugin rules). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of creating a new Store + instantiating the WASM component for every node check, cache a WasmPluginSession per plugin that persists across calls within the same file. The session resets only the node handle table between checks, keeping the WASM linear memory, compiled regex cache, and guest configuration alive. This eliminates the per-node instantiation overhead which was the main performance bottleneck (visible as high sys time). On the apps/mobile benchmark (~1117 files, 9 plugin rules), CPU overhead dropped from +252% to +47%. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion Replace the shared Mutex<Option<WasmPluginSession>> with a thread_local! HashMap keyed by plugin ID. Each worker thread now has its own WASM session per plugin rule, so parallel file analysis no longer serializes through a single lock. On the apps/mobile benchmark (~1117 files, 9 plugin rules): - Wall overhead: +146% → +67% (0.59s → 0.40s, baseline 0.24s) - CPU overhead: +44% → +22% (3.25s → 2.74s, baseline 2.25s) - Sys overhead: +27% → ~0% (instantiation cost fully amortized) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Cache compiled WASM native code to `<path>.wasm.compiled` with a content+engine hash tag, skipping expensive recompilation on subsequent runs (new_cached constructor). - Load plugins in parallel using rayon::par_iter instead of sequentially. - Together these reduce cold-start overhead from ~1.1s to ~0.31s wall time for 9-rule plugin on full monorepo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix outdated WasmPluginSession doc (Mutex → thread-local) - Add 7 unit tests for AOT cache (hash determinism, cache miss cases) - Fix pre-existing host_state test build (update_graph_for_js_paths sig) - Fix unused plugin_name warning when wasm_plugin feature disabled - Run gen-analyzer + cargo fmt (alphabetical dep sorting, schema gen) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…for next branch - Remove stale `markdown` feature from biome_service/Cargo.toml (removed upstream) - Update html.rs fix_all to use process_action_with_reparse API matching main branch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ature Add missing `plugin_name` argument and `working_directory` parameter to test call sites after API changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Upgrade wasmtime 30→42 and wit-bindgen 0.39→0.53 - Add overlapping edit guard in plugin actions - Apply fixKind override in diagnostic path - Add serde skip_serializing_if on subcategory - Set publish = false on SDK crates - Use workspace dependency convention for SDK crates - Add JSON parser depth tracking in options parser - Fix UTF-8 string parsing in options parser - Use settings-based parser options in reparse closures - Replace OnceLock with RefCell in example plugin - Add end-offset guard in semantic model binding_by_node - Derive grit plugin rule name from file stem - Add subcategory to grit plugin diagnostics - Add PhantomData<Rc<()>> for !Send on WasmPluginSession - Implement handle dedup using SyntaxElementKey - Add generation-based thread-local session cache cleanup - Deduplicate plugin configs by path in as_all_plugins - Restore reverted sortBareImports feature - Remove duplicate changeset, fix markdown tables - Remove unused dev-deps from biome_wasm_plugin - Update CLI snapshots for subcategory display - Collapse nested if for clippy in lsp utils Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Hideart
force-pushed
the
feat/wasm-plugin-system
branch
from
ea6f2f2 to
db4808f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/biome_diagnostics/src/serde.rs (1)
43-44: Please add a round-trip test forSome(subcategory).The current serde tests only exercise the
Nonepath, so a regression in field population or thesubcategoryJSON key would sneak through quietly.Also applies to: 68-70, 96-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_diagnostics/src/serde.rs` around lines 43 - 44, Add a round-trip serde test that covers the Some(subcategory) path: create a Diagnostic (or builder) with a non-empty subcategory, serialize it to JSON and assert the "subcategory" key and value are present, then deserialize and assert diag.subcategory() returns Some(the_same_string). Apply this for the spots around the existing tests that exercise let subcategory = diag.subcategory().map(String::from); (and the analogous cases at the ranges you noted) so both serialization and deserialization paths are verified and will catch regressions in the subcategory field handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_analyze/src/analyzer_plugin.rs`:
- Around line 297-320: Compute a canonical plugin rule id once (e.g., let
canonical_id = diagnostic.subcategory.clone().unwrap_or_else(|| format!("{}/{}",
plugin.name(), plugin.rule_name()) or simply plugin.rule_name()) ) and use that
stable id everywhere instead of re-deriving from diagnostic.subcategory: set
SignalEntry.rule to SignalRuleKey::Plugin(canonical_id.clone()), pass
canonical_id into PluginSignal::new (or via a setter) so PluginSignal uses it
for fixKind and suppression text, and ensure the same canonical_id is used by
the pre-eval disabled gate that currently calls plugin.rule_name(); apply the
same change for the other occurrence mentioned (lines ~540-563) so all lookups
share the identical id.
- Around line 241-272: PluginVisitor currently skips the host-side pre-filter
for source_triggers (unlike BatchPluginVisitor) and calls
self.plugin.evaluate(...) directly; add the same trigger-gate used by
BatchPluginVisitor before calling evaluate by factoring the logic into a shared
helper (e.g., a new helper like should_run_for_triggers(plugin, ctx.options) or
reuse an existing helper used by BatchPluginVisitor), and invoke it in
PluginVisitor just after the domain checks and before profiling/evaluate; the
helper should compare the plugin's source_triggers() against
ctx.options.source_triggers() and return false to early-return from
PluginVisitor when triggers do not match so both visitors share the same
behavior.
In `@crates/biome_diagnostics/src/context.rs`:
- Around line 389-391: subcategory() currently forwards the source's subcategory
unconditionally, which can leave a stale subcategory when with_category()
injects a synthetic self.category; update subcategory() (in the same impl where
category() and with_category() live) to first check whether
self.source.as_diagnostic().category() is Some and only then return the source's
subcategory, otherwise return None so injected categories do not keep a
mismatched old subcategory.
In `@crates/biome_diagnostics/src/display.rs`:
- Around line 153-156: When a diagnostic has a subcategory we currently render
full_name as plain text and drop the category's link; change the branch handling
in display.rs so that when diagnostic.subcategory() is Some and category.link()
is Some we emit markup that preserves the link (e.g., create a href by joining
category.link() with the subcategory or wrap the category portion in an anchor)
instead of plain text — update the logic around diagnostic.subcategory(),
full_name, category.link(), and fmt.write_markup/markup! to produce linked
output and add a small header test that asserts links are preserved when
subcategories exist.
---
Nitpick comments:
In `@crates/biome_diagnostics/src/serde.rs`:
- Around line 43-44: Add a round-trip serde test that covers the
Some(subcategory) path: create a Diagnostic (or builder) with a non-empty
subcategory, serialize it to JSON and assert the "subcategory" key and value are
present, then deserialize and assert diag.subcategory() returns
Some(the_same_string). Apply this for the spots around the existing tests that
exercise let subcategory = diag.subcategory().map(String::from); (and the
analogous cases at the ranges you noted) so both serialization and
deserialization paths are verified and will catch regressions in the subcategory
field handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9977e060-45ca-4bc8-abb3-829104e952ab
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_cli/tests/snapshots/main_cases_monorepo/plugins_from_root_config_work_in_child_config_extends_root.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_monorepo/plugins_in_child_config_with_extends_root.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_json_plugin.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_plugin_diagnostic_offset_in_vue_file.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_plugin_suppressions.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (22)
.changeset/wasm-plugin-system.md.gitignoreCargo.tomlcrates/biome_analyze/CONTRIBUTING.mdcrates/biome_analyze/Cargo.tomlcrates/biome_analyze/src/analyzer_plugin.rscrates/biome_analyze/src/diagnostics.rscrates/biome_analyze/src/lib.rscrates/biome_analyze/src/options.rscrates/biome_analyze/src/rule.rscrates/biome_analyze/src/signals.rscrates/biome_analyze/src/suppression_action.rscrates/biome_cli/Cargo.tomlcrates/biome_css_analyze/tests/spec_tests.rscrates/biome_diagnostics/CONTRIBUTING.mdcrates/biome_diagnostics/src/context.rscrates/biome_diagnostics/src/diagnostic.rscrates/biome_diagnostics/src/display.rscrates/biome_diagnostics/src/error.rscrates/biome_diagnostics/src/serde.rscrates/biome_js_analyze/Cargo.tomlcrates/biome_js_analyze/tests/spec_tests.rs
✅ Files skipped from review due to trivial changes (1)
- crates/biome_diagnostics/CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/biome_js_analyze/Cargo.toml
- crates/biome_diagnostics/src/diagnostic.rs
- crates/biome_js_analyze/tests/spec_tests.rs
- crates/biome_analyze/CONTRIBUTING.md
- crates/biome_analyze/src/options.rs
- .changeset/wasm-plugin-system.md
- crates/biome_cli/Cargo.toml
- crates/biome_css_analyze/tests/spec_tests.rs
Comment on lines
+241
to
+272
| // Per-rule disable check via configuration overrides. | ||
| let rule_name = self.plugin.rule_name(); | ||
| if let Some(ovr) = ctx.options.plugin_rule_override(rule_name) | ||
| && ovr.disabled | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Domain filtering: skip plugin if its domain is disabled or | ||
| // restricted to recommended-only and the plugin is not recommended. | ||
| let plugin_domains = self.plugin.domains(); | ||
| if !plugin_domains.is_empty() { | ||
| let domain_cfg = ctx.options.linter_domains(); | ||
| if !domain_cfg.is_empty() { | ||
| for domain in plugin_domains { | ||
| match domain_cfg.get(domain) { | ||
| Some(&PluginDomainFilter::Disabled) => return, | ||
| Some(&PluginDomainFilter::Recommended) if !self.plugin.is_recommended() => { | ||
| return; | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let rule_timer = profiling::start_plugin_rule("plugin"); | ||
| let diagnostics = self | ||
| .plugin | ||
| .evaluate(node.clone().into(), ctx.options.file_path.clone()); | ||
| let result = self.plugin.evaluate( | ||
| node.clone().into(), | ||
| ctx.options.file_path.clone(), | ||
| ctx.services, | ||
| ); |
Contributor
There was a problem hiding this comment.
PluginVisitor still skips the trigger pre-filter.
source_triggers() is only honoured in BatchPluginVisitor; the single-plugin path goes straight from a kind match to evaluate(). In one-plugin mode, the advertised host-side pre-filter is therefore a no-op. Please apply the same trigger gate here, ideally via a shared helper so the two visitors stop drifting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_analyze/src/analyzer_plugin.rs` around lines 241 - 272,
PluginVisitor currently skips the host-side pre-filter for source_triggers
(unlike BatchPluginVisitor) and calls self.plugin.evaluate(...) directly; add
the same trigger-gate used by BatchPluginVisitor before calling evaluate by
factoring the logic into a shared helper (e.g., a new helper like
should_run_for_triggers(plugin, ctx.options) or reuse an existing helper used by
BatchPluginVisitor), and invoke it in PluginVisitor just after the domain checks
and before profiling/evaluate; the helper should compare the plugin's
source_triggers() against ctx.options.source_triggers() and return false to
early-return from PluginVisitor when triggers do not match so both visitors
share the same behavior.
Author
There was a problem hiding this comment.
Fixed — removed PluginVisitor entirely. It was dead code (only BatchPluginVisitor is instantiated in all three analyze crates). This eliminates the drift risk.
Comment on lines
+297
to
+320
| let signals = result.diagnostics.into_iter().map(move |entry| { | ||
| let name = entry | ||
| .diagnostic | ||
| .subcategory | ||
| .clone() | ||
| .unwrap_or_else(|| "anonymous".into()); | ||
|
|
||
| SignalEntry { | ||
| text_range: diagnostic.span().unwrap_or_default(), | ||
| signal: Box::new(PluginSignal::<L>::new(diagnostic)), | ||
| text_range: entry.diagnostic.span().unwrap_or_default(), | ||
| signal: Box::new( | ||
| PluginSignal::<L>::new( | ||
| entry.diagnostic, | ||
| entry.actions, | ||
| Arc::clone(&source_text), | ||
| root, | ||
| suppression_action, | ||
| options, | ||
| ) | ||
| .with_category(plugin_category) | ||
| .with_deprecated(plugin_deprecated.clone()) | ||
| .with_issue_number(plugin_issue_number.clone()), | ||
| ), | ||
| rule: SignalRuleKey::Plugin(name.into()), | ||
| category: RuleCategory::Lint, | ||
| category: plugin_category, |
Contributor
There was a problem hiding this comment.
Thread a canonical plugin id through the signal.
Here the signal key comes from diagnostic.subcategory and falls back to "anonymous", while the pre-eval disabled gate uses plugin.rule_name(), and PluginSignal later reuses the diagnostic subcategory for fixKind and suppression text. WASM rules already emit qualified names like <plugin>/<rule>, so those lookups can drift apart; diagnostics without a subcategory cannot be configured at all. Please compute one stable id up front and pass it through instead of re-deriving it from the diagnostic.
Also applies to: 540-563
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_analyze/src/analyzer_plugin.rs` around lines 297 - 320, Compute
a canonical plugin rule id once (e.g., let canonical_id =
diagnostic.subcategory.clone().unwrap_or_else(|| format!("{}/{}", plugin.name(),
plugin.rule_name()) or simply plugin.rule_name()) ) and use that stable id
everywhere instead of re-deriving from diagnostic.subcategory: set
SignalEntry.rule to SignalRuleKey::Plugin(canonical_id.clone()), pass
canonical_id into PluginSignal::new (or via a setter) so PluginSignal uses it
for fixKind and suppression text, and ensure the same canonical_id is used by
the pre-eval disabled gate that currently calls plugin.rule_name(); apply the
same change for the other occurrence mentioned (lines ~540-563) so all lookups
share the identical id.
Author
There was a problem hiding this comment.
Fixed — both PluginVisitor and BatchPluginVisitor now compute canonical_id once from plugin.rule_name() (the authoritative source) and pass it through to SignalRuleKey::Plugin. The diagnostic subcategory is no longer re-derived for the signal key.
Hideart
force-pushed
the
feat/wasm-plugin-system
branch
from
2e69f53 to
230c8df
Compare
- Remove dead PluginVisitor (only BatchPluginVisitor is used). - Use canonical plugin rule id (from plugin.rule_name()) instead of re-deriving from diagnostic.subcategory for SignalRuleKey, suppression text, and fixKind lookups. - Don't forward stale subcategory when with_category() injects a synthetic category in CategoryDiagnostic. - Preserve category hyperlink when subcategory is present in diagnostic display output. - Restore markdown feature dropped during rebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Hideart
force-pushed
the
feat/wasm-plugin-system
branch
from
230c8df to
3d2ac29
Compare
Author
|
@dyc3 coderabbit marked you as suggested reviewer, I'm not sure what's process you have now around PRs Sorry to bothering you and thanks in advance |
Member
|
@Hideart We are discussing internally if we can accept this PR. We already have GritQL and JS plugins, and having 3 different languages for plugins will make complexity for users and plugin authors. |
Author
@siketyan Thank you for such quick response! Yeah, but it looks like wasm plugins can do it faster, deeper and more complexly than gritql, also it seems more flexible for scaling and maintain. Also we can get the end wasm binary a lot of ways and languages that compiles to But I completely agree with you about so many different languages for authors and users, it'll confuse I just wanted to throw out of eslint and prettier at all of my projects, but I can't (I guess how many people too), because of gritql-plugns restrictions and js-plugins current state |
Member
It seems like you're suggesting to ditch GritQL :D GritQL is an advanced DSL, while WASM are simply a binary. They serve different purposes. It's like comparing apple and oranges. Regardless. This PR is too big. We aren't going to review it. It's a whopping 11k of changes, mostly code, vibe coded. That's a big no. |
Author
I'm suggesting rather to ditch JS plugins system
Yeah, why? Mostly code just written by Claude (docs, tests, examples, optimisation and plugin repo I provided). All of the functional, architectural changes provided and reviewed myself on each step. I spent about 2-3 weeks to implement it, test on real project and make a lot of benchmarks. What's wrong with an AI assistance? Anyway, the final decision is yours, I just thought the general purpose of the biome is to compete with eslint and the PR contains a huge plugin loader for authors like in eslint. It completely behind a feature flag, idk what a risk is there on your side |
Member
|
There's nothing wrong with AI assistance. The PR is too large. Usually, these big features are broken down into smaller PRs to make them more digestible for maintainers to review. If you're willing to break it down, that's a good start. Also, AI assistance or not, it's your code, you wrote it. |
Author
Usually it depends on a specific repo and maintainers, you can just politely tell me about the way you wanna see it here and I'll modify the changes to correspond. This is an opensource with a lot of contributors, each of them has many repos (or even a main job), supporting your project with approximately zero motivation and knowledge of your local rules here, isn't it? I understand you have a lot of AI-genrated pieces of code, that's a big problem now, yeah. But this isn't a reason to be toxic with each new contributor I'm absolutely ready to split the PR to smaller separate parts and cooperate with maintainers on any other moments (I even leaved maintenance commitment section in the PR description)
Did I say otherwise? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in WASM Component Model plugin system that lets users write custom lint rules in Rust (or any language targeting
wasm32-wasip2), compiled to.wasmmodules and loaded at runtime viawasmtime. This complements the existing GritQL plugin system with a high-performance alternative for complex rules that need full AST traversal, semantic model access, and type information.Motivation: Biome's plugin roadmap (#1649, #1762) acknowledges demand for custom rules beyond what GritQL can express. GritQL excels at pattern matching but struggles with stateful analysis — e.g. cross-reference checking, ordering enforcement, or type-aware rules (#6210 reports performance issues with complex GritQL rules). WASM plugins fill this gap: they run near-native speed, are sandboxed by design, and can access Biome's semantic model and type inference through host functions.
This is entirely behind a feature flag (
wasm_plugin) and changes zero behavior when disabled. The existing GritQL and JS plugin systems are untouched.Comparison with native Biome rules
What WASM plugins CAN do
expression_type, literal values, unions)// biome-ignore plugin/rule)biome.jsonWhat WASM plugins CANNOT do (yet)
recommendedrule sets in plugin configis_reachable()availableArchitecture
New crates
biome_plugin_sdk— Guest-side SDK for plugin authors. Provides thegenerate_plugin!()proc macro, syntax kind constants (auto-generated from Biome's syntax crates at build time), and a lightweight JSON options parser (no serde dependency — keeps.wasmsmall, ~70KB per plugin).biome_plugin_sdk_macros— Proc macro crate forgenerate_plugin!().biome_wasm_plugin— Host-side engine. Manages wasmtimeEngine/Store/Componentlifecycle, implements 60+ host functions exposing Biome's AST, semantic model, and type inference to guest code. Includes AOT compilation cache and thread-local session pooling.Plugin authoring example
Configuration
Performance
Benchmarked on Apple Silicon against a production JavaScript monorepo (web and mobile apps). All numbers are 5-run medians with warm disk cache.
Full monorepo (4 apps + shared modules, ~7,800 files)
Single app cluster (2 apps, ~1,300 files)
Multi-plugin scaling
Adding 10 extra WASM modules (each with 1 rule) on top of an existing 9-rule plugin adds only +6% wall / +1% CPU — plugin count scales nearly linearly with negligible per-module overhead.
Cold start
First run without AOT cache: 0.26s vs 0.18s cached (single app cluster, 1 plugin). The AOT compilation cache (
Component::serialize/deserializewith content+engine hash) eliminates ~80ms cold-start per plugin on subsequent runs.Optimization journey
Started at +73% wall / +108% CPU overhead. Final result: +13% wall / +20% CPU on full monorepo. Key wins:
Store+bindings per-thread, avoid per-node instantiationrayon::par_iterfor independent plugin compilationBiome (with WASM plugins) vs ESLint vs TypeScript
Comparative benchmark on single app cluster (2 apps, ~1,300 files). 5-run median, warm cache:
Biome with WASM plugins lints the same codebase 18.6× faster than ESLint (wall clock), while providing equivalent rule coverage for the migrated rules.
Real-world validation
The plugin system was developed and tested against a production JavaScript monorepo (web and mobile apps). 9 custom lint rules were migrated from a private ESLint plugin to WASM, covering patterns like boolean naming conventions, hook ordering, and import restrictions. All rules produce correct diagnostics, suppressions work, and the system runs stable in daily development via Zed IDE with Biome LSP.
The plugin used for testing and benchmarks is open source: wirex-biome-plugin.
What's included
biome_plugin_sdk,biome_plugin_sdk_macros,biome_wasm_plugin)biome_plugin_loader,biome_analyze,biome_servicePluginPathWithOptionsconfig variant for per-rule optionsplugin/my-plugin/ruleName)e2e-tests/wasm-plugins/(JS, CSS, JSON)What's NOT included
--pluginsinfrastructure)wasm_pluginfeature is disabledbuild.rs)Test plan
cargo check --testspasses for all modified cratescargo clippycleancargo build --target wasm32-wasip2 --release, runbiome linton demo filesMaintenance commitment
I'm committed to maintaining this feature long-term — fixing bugs, responding to review feedback, keeping it in sync with Biome's evolving plugin infrastructure, and iterating on the API based on community needs. This is actively used in our production workflow, so I have a direct stake in its stability.
AI disclosure
This PR was authored by a human developer with assistance from Claude Code (Claude Opus) for implementation, testing, and documentation. All architectural decisions, design choices, and code review were performed by the human author.
🤖 Generated with Claude Code