refactor(core): semantic, embedded bindings and references as salsa tracked functions#10672
Conversation
🦋 Changeset detectedLatest commit: a2321bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will improve performance by 6.28%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes |
|
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 adds Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
crates/biome_service/src/workspace/document/mod.rs (1)
24-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOutdated doc comment references
AnyParse.The comment still mentions
AnyParse: the result of the parsed file, but the field is nowOption<Result<(), FileTooLarge>>. The comment should be updated to reflect that the field now only tracks whether parsing was attempted and whether the file was too large, as the actual parse result lives in the Salsa database.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/workspace/document/mod.rs` around lines 24 - 29, Update the doc comment for the `syntax` field to accurately reflect its current type and purpose. Remove the outdated reference to `AnyParse` and replace it with an explanation that the field now only tracks whether parsing was attempted (via `Option`) and whether the file was too large (via `Result<(), FileTooLarge>`), clarifying that the actual parse result is no longer stored here but instead lives in the Salsa database.crates/biome_service/src/workspace/server.rs (4)
1236-1263:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRead HTML snippets from the DB, not through
get_parse().
get_parse(path)first requires aDocumententry. Indexed dependency/ignored files can be parsed intoWorkspaceDbwithout being stored indocuments, so this silently drops embedded JS/CSS from HTML module resolution. Use the parsed source already carried by the update, ordb.parsed_snippets_for_path(path).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/workspace/server.rs` around lines 1236 - 1263, The embedded_content retrieval is using self.get_parse(path) which requires a Document entry and silently drops embedded JS/CSS from files that are parsed into WorkspaceDb without being stored in documents (like indexed dependencies or ignored files). Replace the self.get_parse(path) call with db.parsed_snippets_for_path(path) to directly read HTML snippets from the database, or use the parsed source already carried by the update if available. This ensures all embedded content is properly captured regardless of whether the file is stored as a Document.
2886-2897:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnload DB parses when closing non-indexed documents.
Closing now removes the
Documentand node cache, but leavesParsedSource/snippets inWorkspaceDb. Non-indexed editor files can leak CSTs and leave stale source data behind.Suggested fix
let path = params.path.as_path(); + let was_indexed = self.is_indexed(path); self.documents.pin().remove(path); self.node_cache.lock().unwrap().remove(path); - if self.is_indexed(path) { + if was_indexed { // This may look counter-intuitive, but we need to consider that the // file may have gone out-of-sync between the client and the // filesystem. So when the client closes it, and the scanner still // wants to index it, we need to re-index it to make sure they're // back in sync. self.scanner.reindex_file(path.to_path_buf()); + } else { + self.db_unload_path(path); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/workspace/server.rs` around lines 2886 - 2897, When closing documents that are not indexed by the scanner, the ParsedSource and snippets are left behind in WorkspaceDb, causing CST leaks and stale data. After removing the document from self.documents and self.node_cache, you also need to unload or remove the ParsedSource and snippets from WorkspaceDb. This cleanup should occur regardless of whether the file is indexed, ensuring that non-indexed editor files do not accumulate stale parsed data in the database.
811-816:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the resolved source to select embedded parsing capabilities.
parse_embedded_language_snippets()receivesfile_source, but then re-derives capabilities from the path/DB. That can skip embedded parsing for editor-provided or parser-upgraded sources.Suggested fix
- let capabilities = self.get_file_capabilities( - path, - settings.as_ref().experimental_full_html_support_enabled(), - ); + let capabilities = self.features.get_deprecated_capabilities(*file_source);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/workspace/server.rs` around lines 811 - 816, The issue is that `parse_embedded_language_snippets()` receives a `file_source` parameter but then derives capabilities using `self.get_file_capabilities(path, ...)` which re-derives capabilities from the path/DB instead of using the resolved source. This causes embedded parsing to be skipped for editor-provided or parser-upgraded sources. Modify the code to use the resolved source when selecting embedded parsing capabilities instead of re-deriving capabilities fresh from the path lookup, ensuring that the actual source content being processed determines whether embedded nodes should be parsed.
2024-2057:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not replace the document node cache with the embedded parse cache.
The second
let mut node_cache = NodeCache::default()shadows the warmed cache from the main parse; Line 2057 then stores the embedded/empty cache, losing incremental reparsing benefits on the next edit.Suggested fix
- let mut node_cache = NodeCache::default(); + let mut embedded_node_cache = NodeCache::default(); // Second-pass parsing for HTML files with embedded JavaScript and CSS content let embedded_snippets = if DocumentFileSource::can_contain_embeds( @@ &document_source, &any_parse, - &mut node_cache, + &mut embedded_node_cache, &settings, )?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/workspace/server.rs` around lines 2024 - 2057, The code declares node_cache twice with NodeCache::default(), and the second declaration on line 2025 shadows the warmed cache from the main parse, causing the empty cache to be stored at line 2057 instead of the warmed one. Remove the duplicate node_cache declaration and either use a separate variable name like embedded_node_cache for the embedded snippet parsing, or ensure the parse_embedded_language_snippets method works with the warmed cache from the first declaration without creating a new empty cache that shadows it.
🧹 Nitpick comments (7)
crates/biome_workspace_db/src/lib.rs (1)
38-43: ⚖️ Poor tradeoffLinear search could become a performance bottleneck.
The
insert_sourcemethod performs a linear search through all existing file sources before inserting. With many sources in a large project, this O(n) operation could degrade performance. Consider maintaining a secondaryHashMap<DocumentFileSource, usize>to enable O(1) lookups for deduplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_workspace_db/src/lib.rs` around lines 38 - 43, The insert_source method currently performs a linear search through the file_sources vector for every insertion, which is O(n) and can degrade performance. Refactor this by adding a secondary HashMap field to the struct that maps DocumentFileSource to its index (usize). When inserting, first check if the source exists in the HashMap for O(1) lookup. If found, return the cached index; if not found, add it to both the vector and the HashMap to maintain consistency. This ensures both the vector and HashMap are always kept in sync.crates/biome_service/src/workspace/server.tests.rs (1)
206-209: 💤 Low valueMinor cleanup: unused variable.
The
documentvariable is fetched and asserted to exist, but isn't used afterward (the refactor now queries snippets viaworkspace.get_snippetsinstead). Consider removing lines 206–207 if the assertion isn't needed, or adding a comment explaining why we verify the document exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/workspace/server.tests.rs` around lines 206 - 209, The `document` variable is fetched and an assertion is added for it, but since the refactor now uses `workspace.get_snippets` instead, this variable is unused and unnecessary. Remove the lines that fetch the document using `documents.get()` and the `assert!(document.is_some())` assertion unless there is a specific reason to verify the document exists at that point in the test. If the assertion should remain for validation purposes, add a comment explaining why the document existence check is important for the test.crates/biome_db/Cargo.toml (1)
21-21: 💤 Low valueEmpty features section.
The
[features]header with no content can be removed unless you're planning to add features soon.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_db/Cargo.toml` at line 21, Remove the empty [features] section header from the Cargo.toml file in crates/biome_db since there are no features defined underneath it. Simply delete the [features] line as it serves no purpose without any feature definitions.crates/biome_js_analyze/src/lib.rs (1)
241-241: 💤 Low valueClone of semantic model before service insertion.
The transition from owned
SemanticModelto borrowed&'a SemanticModelnecessitates this clone when inserting into services. This is correct, though it does introduce an allocation. Consider documenting why the borrowed reference pattern was chosen despite requiring this clone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_js_analyze/src/lib.rs` at line 241, Add a comment above the services.insert_service(semantic_model.clone()) call explaining the rationale for using the borrowed reference pattern (&'a SemanticModel) and why this necessitates the clone operation. The comment should document the design decision to help future maintainers understand the tradeoff between using borrowed references and the allocation cost of the clone.crates/biome_analyze/src/analyzer_plugin.rs (1)
55-55: Borrow the plugin path to avoid per-node allocations.
evaluate()is called from visitor hot paths, so takingUtf8PathBufforces a path clone for every matching node/plugin. Passing&Utf8Pathkeeps the API simple and avoids that churn.Proposed refactor
- fn evaluate(&self, node: AnySyntaxNode, path: Utf8PathBuf) -> PluginEvalResult; + fn evaluate(&self, node: AnySyntaxNode, path: &Utf8Path) -> PluginEvalResult;- .evaluate(node.clone().into(), ctx.options.file_path.clone()); + .evaluate(node.clone().into(), &ctx.options.file_path);- let eval_result = plugin.evaluate(node.clone().into(), ctx.options.file_path.clone()); + let eval_result = plugin.evaluate(node.clone().into(), &ctx.options.file_path);Also update the implementation in
crates/biome_plugin_loader/src/analyzer_grit_plugin.rsline 90 to match the borrowed signature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_analyze/src/analyzer_plugin.rs` at line 55, In the AnalyzerPlugin trait, change the evaluate method signature to borrow the path parameter as &Utf8Path instead of taking ownership of Utf8PathBuf. This reduces allocations in hot paths where evaluate is called for each node. Additionally, update the corresponding implementation of the evaluate method in the AnalyzerGritPlugin struct to use the same borrowed &Utf8Path parameter type to match the trait signature.crates/biome_service/src/file_handlers/html/parse_embedded_nodes.rs (1)
353-354: 💤 Low valueEmpty match arm for
SvelteEachKeyedItem.The
SvelteEachKeyedItemvariant now does nothing, whereasSvelteEachAsKeyedItemstill processes the key expression. This asymmetry is worth a brief inline comment explaining why keyed items without theasclause don't need processing (presumably because binding registration moved elsewhere or is no longer needed in this context).Also applies to: 439-439
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/file_handlers/html/parse_embedded_nodes.rs` around lines 353 - 354, In the parse_svelte_blocks function, there is a match statement handling Svelte item types where the SvelteEachKeyedItem variant has an empty match arm (does nothing) while SvelteEachAsKeyedItem processes the key expression, creating an asymmetry that needs clarification. Add a brief inline comment above or within the SvelteEachKeyedItem match arm explaining why this variant does not require processing of the key expression (for example, noting that binding registration has moved elsewhere or is no longer needed in this parsing context).crates/biome_service/src/file_handlers/css/go_to.rs (1)
52-54: ⚡ Quick winAvoid per-selector string allocation in class matching.
text_trimmed().to_string()allocates for every candidate class. A borrowed-text comparison here keeps go-to-definition lean on larger stylesheets.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_service/src/file_handlers/css/go_to.rs` around lines 52 - 54, The class name matching logic in the class_sel.name() condition is allocating a new String for every candidate class selector by calling text_trimmed().to_string(), which is inefficient on larger stylesheets. Replace the string allocation comparison with a direct borrowed-text comparison between the trimmed text and the class_name reference, avoiding the to_string() call and its memory overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/biome_css_semantic/src/semantic_model/model.rs`:
- Around line 111-127: The `PartialEq` implementation for `SemanticModel` (and
related implementations at lines 163-180, 464-468, 570-574, 612-619, 664-689) is
incomplete and ignores critical fields used for cache invalidation. Update the
`eq` method in `SemanticModel` to also compare the `top_level_rule_ids` and
`range_to_rule_id` fields in addition to the current checks, and apply similar
fixes to the other `PartialEq` implementations mentioned (such as `Rule::eq`
which should include parent/child link comparisons, and
`CssPropertyInitialValue::eq` which should compare actual value payloads) to
ensure all semantically significant fields are included in equality checks.
In `@crates/biome_js_analyze/benches/js_analyzer.rs`:
- Around line 63-68: The source_from_index method in the LanguageDb
implementation hard-codes JsFileSource::tsx() for all benchmark cases instead of
using the actual file_source that is stored during initialization. Replace the
hard-coded Some(DocumentFileSource::Js(JsFileSource::tsx())) return value with
the actual file_source variable that is captured and stored around line 111.
This will ensure each benchmark case uses its intended file source type rather
than always testing with TSX, making the benchmark results accurate for rule
gating across different file types.
In `@crates/biome_js_analyze/Cargo.toml`:
- Around line 67-71: In the dev-dependencies section of
crates/biome_js_analyze/Cargo.toml, replace the workspace = true directives with
path-based dependencies. Specifically, change the biome_db dependency on line 67
from using workspace = true to using path = "../biome_db", and change the
biome_parser dependency on line 70 from using workspace = true to using path =
"../biome_parser". This aligns with the repository guideline that internal biome
crates in dev-dependencies should use relative path dependencies instead of
workspace references.
In `@crates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rs`:
- Around line 74-76: The `.expect()` call on
`ctx.get_service::<EmbeddedService>()` will panic if the service is missing,
causing a crash instead of producing diagnostics. Replace the `.expect()` with
proper error handling such as pattern matching (if let Some) or returning early
with a diagnostic error. This resilience pattern needs to be applied at all
occurrences where `EmbeddedService` is retrieved via `get_service()` in this
file (including the location mentioned at lines 101-104).
In `@crates/biome_js_analyze/src/lint/style/use_import_type.rs`:
- Around line 191-193: The `expect()` call on the EmbeddedService in the
get_service method will panic if the service is unavailable, but the
useImportType lint should still work correctly without it. Replace the
`expect()` with optional handling (such as using methods like `ok()` to convert
the Result to an Option) so the service is treated as optional. Then, adjust the
logic to only veto type-only imports when the EmbeddedService is present and
actually reports a value use. Apply this same fix pattern to all occurrences of
this issue across the file (including the locations around lines 822-830 and
855-860).
In `@crates/biome_js_analyze/tests/spec_tests.rs`:
- Around line 35-75: The TestDb implementation ignores the actual source_type
and path parameters passed to embedded_db() and instead uses hardcoded values.
Fix source_from_index() in the LanguageDb impl to return the stored source_type
field instead of always returning tsx, fix parsed_source_for_path() in the Db
impl to validate that the requested path matches the stored path before
returning the parsed source, and add a path field to the TestDb struct to store
the path parameter from embedded_db() so it can be used for comparison in
parsed_source_for_path().
In `@crates/biome_js_semantic/src/semantic_model/model.rs`:
- Around line 173-197: The PartialEq implementation for SemanticModelData only
compares the length of exported instead of its actual content, and it completely
omits comparisons for bindings_by_start and declared_at_by_start. This allows
two models with different binding-to-range mappings to be considered equal,
risking stale semantic lookups. In the eq method, replace the single-length
comparison of self.exported with a comparison that validates the actual exported
content equality, and add additional checks that compare the bindings_by_start
and declared_at_by_start fields (not just their lengths) to ensure the
range-based lookup structures are identical between the two SemanticModelData
instances.
In `@crates/biome_service/src/file_handlers/html/go_to.rs`:
- Around line 79-87: The binding lookup in the go_to function using
get_binding_with_source is not range-aware, which causes duplicate identifiers
in different embedded scopes to resolve incorrectly. Modify both occurrences of
get_binding_with_source (at lines 79-87 and 97-104) to make the binding lookup
range-aware by either including the token/reference range information in the
InternedBindingTokenText construction or by selecting the binding using
params.cursor_offset instead of just the current path and token text. This
ensures that identifiers are correctly resolved within their specific embedded
scopes rather than returning the first match regardless of scope.
In `@crates/biome_service/src/file_handlers/javascript.rs`:
- Around line 1550-1552: The JsFileSource is being inferred from the file path
using JsFileSource::try_from(path.as_path()), but this loses source metadata for
extensionless or virtual TS/TSX documents when they come from LSP overrides.
Instead of extracting source_type from the path parameter, extract it directly
from the parse parameter (which is of type AnyParsedSource) that already
contains the correct source information. This will preserve the proper source
metadata and ensure the semantic_model function receives the correct source
type.
In `@crates/biome_service/src/file_handlers/javascript/go_to.rs`:
- Line 186: Fix the typo in the variable name by renaming `offest` to `offset`
in the assignment statement where it is assigned the result of
`params.parsed_source.diagnostic_offset(¶ms.workspace_db)`. Ensure all
references to this variable throughout the function use the corrected spelling
`offset`.
In `@crates/biome_service/src/workspace/server.rs`:
- Around line 758-763: The code currently subtracts snippet_offset from
cursor_offset to calculate local_cursor before verifying that cursor_offset
actually falls within the snippet's range. This can cause underflow or panic for
cursors positioned before a snippet. Move the range check (using
snippet.content_range() and comparing cursor_offset against
snippet_range.start() and snippet_range.end()) to execute first, and only
calculate local_cursor by subtracting snippet_offset after confirming the cursor
is within the valid range.
- Around line 2505-2507: The guard condition in the format check only validates
the main parse for errors using parse.has_errors() but does not account for
embedded parse errors within code snippets. Modify the condition that checks
format_with_errors_enabled_for_this_file_path to also include a check for
embedded parse errors on the parse object in addition to the existing has_errors
call, ensuring that formatting is blocked when the format_with_errors setting is
disabled and either the main parse or any embedded parses contain errors.
- Around line 2021-2044: The parser-updated file source information,
specifically the refined language from ParseResult, is being lost when the
database is updated. Ensure that when calling db_update_parsed_file with the
parsed result and embedded snippets, the method preserves the updated file
source information from the parsed result rather than allowing the old source
index to overwrite it. Check that the ParseResult language field is being
retained and passed through to the database update to prevent the document
source from becoming stale after parsing refines it.
- Line 2136: The `parse_errors` variable currently only counts root parse errors
via `parse.error_count(&workspace_db)` but excludes embedded parse errors from
CSS/JS snippets, making the parse state appear clean in LSP clients while
embedded content has syntax errors. Modify the `parse_errors` calculation to
include error counts from embedded parses in addition to the root parse error
count, ensuring the total reflects all syntax errors across the entire document
including embedded content.
---
Outside diff comments:
In `@crates/biome_service/src/workspace/document/mod.rs`:
- Around line 24-29: Update the doc comment for the `syntax` field to accurately
reflect its current type and purpose. Remove the outdated reference to
`AnyParse` and replace it with an explanation that the field now only tracks
whether parsing was attempted (via `Option`) and whether the file was too large
(via `Result<(), FileTooLarge>`), clarifying that the actual parse result is no
longer stored here but instead lives in the Salsa database.
In `@crates/biome_service/src/workspace/server.rs`:
- Around line 1236-1263: The embedded_content retrieval is using
self.get_parse(path) which requires a Document entry and silently drops embedded
JS/CSS from files that are parsed into WorkspaceDb without being stored in
documents (like indexed dependencies or ignored files). Replace the
self.get_parse(path) call with db.parsed_snippets_for_path(path) to directly
read HTML snippets from the database, or use the parsed source already carried
by the update if available. This ensures all embedded content is properly
captured regardless of whether the file is stored as a Document.
- Around line 2886-2897: When closing documents that are not indexed by the
scanner, the ParsedSource and snippets are left behind in WorkspaceDb, causing
CST leaks and stale data. After removing the document from self.documents and
self.node_cache, you also need to unload or remove the ParsedSource and snippets
from WorkspaceDb. This cleanup should occur regardless of whether the file is
indexed, ensuring that non-indexed editor files do not accumulate stale parsed
data in the database.
- Around line 811-816: The issue is that `parse_embedded_language_snippets()`
receives a `file_source` parameter but then derives capabilities using
`self.get_file_capabilities(path, ...)` which re-derives capabilities from the
path/DB instead of using the resolved source. This causes embedded parsing to be
skipped for editor-provided or parser-upgraded sources. Modify the code to use
the resolved source when selecting embedded parsing capabilities instead of
re-deriving capabilities fresh from the path lookup, ensuring that the actual
source content being processed determines whether embedded nodes should be
parsed.
- Around line 2024-2057: The code declares node_cache twice with
NodeCache::default(), and the second declaration on line 2025 shadows the warmed
cache from the main parse, causing the empty cache to be stored at line 2057
instead of the warmed one. Remove the duplicate node_cache declaration and
either use a separate variable name like embedded_node_cache for the embedded
snippet parsing, or ensure the parse_embedded_language_snippets method works
with the warmed cache from the first declaration without creating a new empty
cache that shadows it.
---
Nitpick comments:
In `@crates/biome_analyze/src/analyzer_plugin.rs`:
- Line 55: In the AnalyzerPlugin trait, change the evaluate method signature to
borrow the path parameter as &Utf8Path instead of taking ownership of
Utf8PathBuf. This reduces allocations in hot paths where evaluate is called for
each node. Additionally, update the corresponding implementation of the evaluate
method in the AnalyzerGritPlugin struct to use the same borrowed &Utf8Path
parameter type to match the trait signature.
In `@crates/biome_db/Cargo.toml`:
- Line 21: Remove the empty [features] section header from the Cargo.toml file
in crates/biome_db since there are no features defined underneath it. Simply
delete the [features] line as it serves no purpose without any feature
definitions.
In `@crates/biome_js_analyze/src/lib.rs`:
- Line 241: Add a comment above the
services.insert_service(semantic_model.clone()) call explaining the rationale
for using the borrowed reference pattern (&'a SemanticModel) and why this
necessitates the clone operation. The comment should document the design
decision to help future maintainers understand the tradeoff between using
borrowed references and the allocation cost of the clone.
In `@crates/biome_service/src/file_handlers/css/go_to.rs`:
- Around line 52-54: The class name matching logic in the class_sel.name()
condition is allocating a new String for every candidate class selector by
calling text_trimmed().to_string(), which is inefficient on larger stylesheets.
Replace the string allocation comparison with a direct borrowed-text comparison
between the trimmed text and the class_name reference, avoiding the to_string()
call and its memory overhead.
In `@crates/biome_service/src/file_handlers/html/parse_embedded_nodes.rs`:
- Around line 353-354: In the parse_svelte_blocks function, there is a match
statement handling Svelte item types where the SvelteEachKeyedItem variant has
an empty match arm (does nothing) while SvelteEachAsKeyedItem processes the key
expression, creating an asymmetry that needs clarification. Add a brief inline
comment above or within the SvelteEachKeyedItem match arm explaining why this
variant does not require processing of the key expression (for example, noting
that binding registration has moved elsewhere or is no longer needed in this
parsing context).
In `@crates/biome_service/src/workspace/server.tests.rs`:
- Around line 206-209: The `document` variable is fetched and an assertion is
added for it, but since the refactor now uses `workspace.get_snippets` instead,
this variable is unused and unnecessary. Remove the lines that fetch the
document using `documents.get()` and the `assert!(document.is_some())` assertion
unless there is a specific reason to verify the document exists at that point in
the test. If the assertion should remain for validation purposes, add a comment
explaining why the document existence check is important for the test.
In `@crates/biome_workspace_db/src/lib.rs`:
- Around line 38-43: The insert_source method currently performs a linear search
through the file_sources vector for every insertion, which is O(n) and can
degrade performance. Refactor this by adding a secondary HashMap field to the
struct that maps DocumentFileSource to its index (usize). When inserting, first
check if the source exists in the HashMap for O(1) lookup. If found, return the
cached index; if not found, add it to both the vector and the HashMap to
maintain consistency. This ensures both the vector and HashMap are always kept
in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa009d54-2b2c-4ee7-b48a-1db5f8acaba9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**
📒 Files selected for processing (114)
Cargo.tomlcrates/biome_analyze/src/analyzer_plugin.rscrates/biome_analyze/src/options.rscrates/biome_css_analyze/src/lib.rscrates/biome_css_analyze/src/lint/nursery/no_unused_classes.rscrates/biome_css_analyze/src/services/module_graph.rscrates/biome_css_analyze/tests/spec_tests.rscrates/biome_css_semantic/Cargo.tomlcrates/biome_css_semantic/src/semantic_model/builder.rscrates/biome_css_semantic/src/semantic_model/db.rscrates/biome_css_semantic/src/semantic_model/mod.rscrates/biome_css_semantic/src/semantic_model/model.rscrates/biome_css_semantic/src/tests/eq.rscrates/biome_css_semantic/src/tests/mod.rscrates/biome_db/Cargo.tomlcrates/biome_db/src/lib.rscrates/biome_db/src/testing.rscrates/biome_fs/Cargo.tomlcrates/biome_graphql_analyze/Cargo.tomlcrates/biome_graphql_formatter/Cargo.tomlcrates/biome_grit_patterns/src/grit_context.rscrates/biome_html_analyze/src/lib.rscrates/biome_html_analyze/src/lint/nursery/no_undeclared_classes.rscrates/biome_html_analyze/src/services/module_graph.rscrates/biome_html_analyze/tests/spec_tests.rscrates/biome_js_analyze/Cargo.tomlcrates/biome_js_analyze/benches/js_analyzer.rscrates/biome_js_analyze/src/frameworks/vue/vue_component.rscrates/biome_js_analyze/src/lib.rscrates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_js_analyze/src/lint/correctness/no_unused_variables.rscrates/biome_js_analyze/src/lint/nursery/no_undeclared_classes.rscrates/biome_js_analyze/src/lint/style/use_export_type.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_js_analyze/src/services/database.rscrates/biome_js_analyze/src/services/embedded.rscrates/biome_js_analyze/src/services/embedded_bindings.rscrates/biome_js_analyze/src/services/embedded_value_references.rscrates/biome_js_analyze/src/services/mod.rscrates/biome_js_analyze/src/suppressions.tests.rscrates/biome_js_analyze/tests/quick_test.rscrates/biome_js_analyze/tests/spec_tests.rscrates/biome_js_semantic/Cargo.tomlcrates/biome_js_semantic/src/db.rscrates/biome_js_semantic/src/lib.rscrates/biome_js_semantic/src/semantic_model.rscrates/biome_js_semantic/src/semantic_model/binding.rscrates/biome_js_semantic/src/semantic_model/builder.rscrates/biome_js_semantic/src/semantic_model/model.rscrates/biome_js_semantic/src/tests/db.rscrates/biome_js_semantic/src/tests/mod.rscrates/biome_js_type_info/Cargo.tomlcrates/biome_json_formatter/Cargo.tomlcrates/biome_json_formatter/src/context.rscrates/biome_languages/Cargo.tomlcrates/biome_languages/src/db.rscrates/biome_languages/src/lib.rscrates/biome_lsp/Cargo.tomlcrates/biome_lsp/src/handlers/navigation.rscrates/biome_lsp/src/handlers/text_document.rscrates/biome_lsp/src/session.rscrates/biome_module_graph/Cargo.tomlcrates/biome_module_graph/src/db/mod.rscrates/biome_module_graph/src/db/project_database.rscrates/biome_module_graph/src/js_module_info/module_resolver.rscrates/biome_module_graph/src/lib.rscrates/biome_module_graph/src/module_graph.rscrates/biome_module_graph/tests/spec_tests.rscrates/biome_parser/src/lib.rscrates/biome_plugin_loader/Cargo.tomlcrates/biome_plugin_loader/src/analyzer_grit_plugin.rscrates/biome_plugin_loader/src/analyzer_js_plugin.rscrates/biome_resolver/src/db/inputs.rscrates/biome_resolver/src/db/mod.rscrates/biome_resolver/src/lib.rscrates/biome_rowan/src/syntax/node.rscrates/biome_ruledoc_utils/Cargo.tomlcrates/biome_ruledoc_utils/src/lib.rscrates/biome_service/Cargo.tomlcrates/biome_service/src/embed/types.rscrates/biome_service/src/file_handlers/astro.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/css/go_to.rscrates/biome_service/src/file_handlers/graphql.rscrates/biome_service/src/file_handlers/grit.rscrates/biome_service/src/file_handlers/html.rscrates/biome_service/src/file_handlers/html/go_to.rscrates/biome_service/src/file_handlers/html/parse_embedded_nodes.rscrates/biome_service/src/file_handlers/html/parse_embedded_nodes.tests.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/file_handlers/javascript/go_to.rscrates/biome_service/src/file_handlers/json.rscrates/biome_service/src/file_handlers/md.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/file_handlers/svelte.rscrates/biome_service/src/file_handlers/vue.rscrates/biome_service/src/file_handlers/yaml.rscrates/biome_service/src/workspace.rscrates/biome_service/src/workspace/db.rscrates/biome_service/src/workspace/document/mod.rscrates/biome_service/src/workspace/document/services/embedded_value_references.rscrates/biome_service/src/workspace/document/services/mod.rscrates/biome_service/src/workspace/server.rscrates/biome_service/src/workspace/server.tests.rscrates/biome_test_utils/Cargo.tomlcrates/biome_test_utils/src/lib.rscrates/biome_workspace_db/Cargo.tomlcrates/biome_workspace_db/src/embedded/bindings.rscrates/biome_workspace_db/src/embedded/mod.rscrates/biome_workspace_db/src/embedded/references.rscrates/biome_workspace_db/src/embedded/visitor.rscrates/biome_workspace_db/src/lib.rsxtask/rules_check/src/lib.rs
💤 Files with no reviewable changes (11)
- crates/biome_resolver/src/db/mod.rs
- crates/biome_js_analyze/src/services/embedded_value_references.rs
- crates/biome_module_graph/src/lib.rs
- crates/biome_js_analyze/src/services/embedded_bindings.rs
- crates/biome_lsp/src/handlers/navigation.rs
- crates/biome_module_graph/src/db/mod.rs
- crates/biome_resolver/src/db/inputs.rs
- crates/biome_module_graph/src/db/project_database.rs
- crates/biome_service/src/workspace/document/services/embedded_value_references.rs
- crates/biome_js_semantic/src/semantic_model.rs
- crates/biome_resolver/src/lib.rs
82e79f1 to
62c0252
Compare
a288320 to
78d37bd
Compare
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | imports | patch | [`2.5.1` -> `2.5.2`](https://renovatebot.com/diffs/npm/@biomejs%2fbiome/2.5.1/2.5.2) | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.5.2`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#252) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.5.1...@biomejs/biome@2.5.2) ##### Patch Changes - [#​10595](biomejs/biome#10595) [`f458028`](biomejs/biome@f458028) Thanks [@​pkallos](https://github.com/pkallos)! - Added the option `ignoreBooleanCoercion` to [useNullishCoalescing](https://biomejs.dev/linter/rules/use-nullish-coalescing/). When enabled, Biome ignores `||` and `||=` used inside a `Boolean()` call, where coalescing on falsy values is intentional. - [#​10798](biomejs/biome#10798) [`4a32b63`](biomejs/biome@4a32b63) Thanks [@​pkallos](https://github.com/pkallos)! - Added the option `ignorePrimitives` to [useNullishCoalescing](https://biomejs.dev/linter/rules/use-nullish-coalescing/). When enabled, Biome ignores `||`, `||=`, and ternary expressions whose non-nullish operands are all primitives the option opts out of. Use `true` to ignore all primitives, or an object selecting `string`, `number`, `boolean`, or `bigint`. - [#​10545](biomejs/biome#10545) [`f3d4c00`](biomejs/biome@f3d4c00) Thanks [@​Mokto](https://github.com/Mokto)! - Added the new nursery rule [`noSvelteUnnecessaryStateWrap`](https://biomejs.dev/linter/rules/no-svelte-unnecessary-state-wrap/), which reports unnecessary `$state()` wrapping of classes from `svelte/reactivity` that are already reactive. ```svelte <script> import { SvelteMap } from "svelte/reactivity"; const map = $state(new SvelteMap()); // redundant </script> ``` - [#​10752](biomejs/biome#10752) [`f62fb8b`](biomejs/biome@f62fb8b) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [#​10739](biomejs/biome#10739). Now the rule [`useValidAutocomplete`](https://biomejs.dev/linter/rules/use-valid-autocomplete/) correctly flags the `autoComplete` attribute. - [#​10796](biomejs/biome#10796) [`f1b3ab2`](biomejs/biome@f1b3ab2) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [#​10768](biomejs/biome#10768). Improved the performance of the Biome Language Server by cancelling certain in-flight operations when there are fast updates. - [#​10719](biomejs/biome#10719) [`aa649b5`](biomejs/biome@aa649b5) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) false positive on returns that use a widening type assertion: `"a" as string` is no longer reported as misleading. The rule now also reports a literal-pinning assertion such as `false as false`, matching the existing `as const` behavior. ```ts // No longer flagged (returns are `string`): function getValue(b: boolean): string { if (b) return "a" as string; return "b" as string; } // Now also reported, like `as const` (returns `false`): function isReady(): boolean { return false as false; } ``` - [#​10678](biomejs/biome#10678) [`8f073a7`](biomejs/biome@8f073a7) Thanks [@​PranavAchar01](https://github.com/PranavAchar01)! - Fixed [#​7718](biomejs/biome#7718): Biome now correctly parses CSS nesting selectors when `&` appears as a trailing sub-selector after a type selector, e.g. `h1& { color: red; }`. - [#​10756](biomejs/biome#10756) [`5ec965a`](biomejs/biome@5ec965a) Thanks [@​denbezrukov](https://github.com/denbezrukov)! - Fixed CSS formatter output for selector lists with `allowWrongLineComments` and `//` comments after a selector comma. Biome now keeps the selector before the line comment inline instead of breaking it across descendant combinators. ```diff -.powerPathNavigator - .helm - button.pressedButton, // pressed +.powerPathNavigator .helm button.pressedButton, // pressed .powerPathNavigator .helm button:active:not(.disabledButton) { } ``` - [#​10757](biomejs/biome#10757) [`6232fcd`](biomejs/biome@6232fcd) Thanks [@​PranavAchar01](https://github.com/PranavAchar01)! - Fixed [#​8269](biomejs/biome#8269): the CSS parser now accepts Tailwind `@variant` and `@utility` names that start with a digit, such as the `2xl` breakpoint. ```css @​utility container { @​variant 2xl { max-width: 1400px; } } ``` - [#​10777](biomejs/biome#10777) [`575ced6`](biomejs/biome@575ced6) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed an issue reported in [#​10708](biomejs/biome#10708): the GitLab reporter now handles `--verbose` diagnostics filtering correctly. - [#​10281](biomejs/biome#10281) [`0efe244`](biomejs/biome@0efe244) Thanks [@​Zelys-DFKH](https://github.com/Zelys-DFKH)! - Fixed a bug where GritQL patterns rejected positional (unkeyed) arguments. - [#​10758](biomejs/biome#10758) [`e36fd8a`](biomejs/biome@e36fd8a) Thanks [@​henrybrewer00-dotcom](https://github.com/henrybrewer00-dotcom)! - Fixed [#​10697](biomejs/biome#10697): The formatter no longer removes the parentheses around an `await` or `yield` expression used as the target of a TypeScript instantiation expression. For example, `(await makeFactory)<Value>` is no longer reformatted to `await makeFactory<Value>`, which would change the meaning of the code. - [#​10586](biomejs/biome#10586) [`3617094`](biomejs/biome@3617094) Thanks [@​IxxyDev](https://github.com/IxxyDev)! - Fixed [#​9568](biomejs/biome#9568): [`noFloatingPromises`](https://biomejs.dev/linter/rules/no-floating-promises/) no longer reports a false positive when calling an overloaded function and the selected overload does not return a promise. ```ts function bestEffort(cb: () => Promise<number>): Promise<number>; function bestEffort(cb: () => number): number; function bestEffort( cb: () => number | Promise<number>, ): Promise<number> | number { return cb() as Promise<number> | number; } // This resolves to the second overload, which returns `number`, so it is no // longer flagged as a floating promise. bestEffort(() => 42); ``` - [#​10766](biomejs/biome#10766) [`7aff4c1`](biomejs/biome@7aff4c1) Thanks [@​JamBalaya56562](https://github.com/JamBalaya56562)! - Fixed [#​2862](biomejs/biome#2862): [`noInteractiveElementToNoninteractiveRole`](https://biomejs.dev/linter/rules/no-interactive-element-to-noninteractive-role/) no longer reports custom elements (a tag name containing a dash, e.g. `<my-button role="img" />`). Per the [W3C HTML-ARIA specification](https://www.w3.org/TR/html-aria/#el-autonomous-custom-element), a custom element may be given any role or none. - [#​10680](biomejs/biome#10680) [`771daa4`](biomejs/biome@771daa4) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed [#​10635](biomejs/biome#10635): Biome now recognizes chained table tests such as `test.concurrent.each()` and `it.concurrent.each()` as test calls, fixing `noMisplacedAssertion` false positives and improving formatting for those test declarations. - [#​10759](biomejs/biome#10759) [`34570b5`](biomejs/biome@34570b5) Thanks [@​henrybrewer00-dotcom](https://github.com/henrybrewer00-dotcom)! - Fixed [#​10636](biomejs/biome#10636): [noStaticElementInteractions](https://biomejs.dev/linter/rules/no-static-element-interactions/) no longer reports a false positive for event handlers on Svelte special elements such as `<svelte:window>`, `<svelte:document>`, and `<svelte:body>`. These are not real DOM elements, so they are now ignored by the rule. - [#​10741](biomejs/biome#10741) [`bd2364e`](biomejs/biome@bd2364e) Thanks [@​JamBalaya56562](https://github.com/JamBalaya56562)! - Fixed [#​6686](biomejs/biome#6686): the `rage` command now respects the `--config-path` option and the `BIOME_CONFIG_PATH` environment variable when loading the Biome configuration. Previously it always used the default configuration resolution and reported the configuration as `Not set` when no `biome.json` existed in the working directory. - [#​10763](biomejs/biome#10763) [`2c3e82d`](biomejs/biome@2c3e82d) Thanks [@​Aqu1bp](https://github.com/Aqu1bp)! - Fixed [#​10742](biomejs/biome#10742): [`noSolidDestructuredProps`](https://biomejs.dev/linter/rules/no-solid-destructured-props) now reports destructured props in Solid function components and JSX children. - [#​10606](biomejs/biome#10606) [`a4cc4ab`](biomejs/biome@a4cc4ab) Thanks [@​Mokto](https://github.com/Mokto)! - Fixed false positives in `noUnusedImports`, `noUnusedVariables`, and `useImportType` for Svelte components that use both a `<script module>` and a `<script>` block. The two blocks compile to a single module and share a top-level scope, so a binding (import, function, or variable) declared in one block and used only in the other is no longer reported as unused. - [#​10767](biomejs/biome#10767) [`36d5aa7`](biomejs/biome@36d5aa7) Thanks [@​otkrickey](https://github.com/otkrickey)! - Fixed [#​10754](biomejs/biome#10754): [`useVueValidVBind`](https://biomejs.dev/linter/rules/use-vue-valid-v-bind/) no longer reports the Vue 3.4+ same-name shorthand as missing a value. `:foo` and `v-bind:foo` are now accepted as equivalent to `:foo="foo"`, while `v-bind`, `v-bind:[dynamicArg]`, and `:[dynamicArg]` without a value continue to be reported. - [#​10775](biomejs/biome#10775) [`a918af0`](biomejs/biome@a918af0) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed an issue reported in [#​10708](biomejs/biome#10708): `biome rage` didn't detect running Biome daemon pipes on Windows. - [#​10730](biomejs/biome#10730) [`5a2e65b`](biomejs/biome@5a2e65b) Thanks [@​dinocosta](https://github.com/dinocosta)! - Fixed an issue where Biome was resolving [the well-known Zed settings file](https://biomejs.dev/guides/configure-biome/#well-known-files) from the wrong location on macOS and Windows. - [#​10807](biomejs/biome#10807) [`d97fffe`](biomejs/biome@d97fffe) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where `.scss` files were incorrectly analyzed when running `biome check`. - [#​10672](biomejs/biome#10672) [`53c6efc`](biomejs/biome@53c6efc) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed a bug where Biome incorrectly formatted snippets that have parsing errors. - [#​10719](biomejs/biome#10719) [`aa649b5`](biomejs/biome@aa649b5) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [`useAwaitThenable`](https://biomejs.dev/linter/rules/use-await-thenable/) false positive when awaiting a custom thenable that is not the global `Promise`. A value with a callable `then` member is now recognized as awaitable. ```ts interface Thenable<T> { then(onfulfilled: (value: T) => void): void; } declare const t: Thenable<number>; async function f() { await t; } ``` - [#​10734](biomejs/biome#10734) [`4396496`](biomejs/biome@4396496) Thanks [@​BangDori](https://github.com/BangDori)! - Fixed [#​10708](biomejs/biome#10708): `biome migrate` now preserves trivia when migrating the deprecated `recommended` option to `preset`. - [#​10683](biomejs/biome#10683) [`ae31a00`](biomejs/biome@ae31a00) Thanks [@​Netail](https://github.com/Netail)! - Fixed [#​10657](biomejs/biome#10657) [#​10671](biomejs/biome#10671) [#​10661](biomejs/biome#10661) [#​10637](biomejs/biome#10637) [#​10718](biomejs/biome#10718): HTML rules now correctly handle dynamic attributes. - [#​10746](biomejs/biome#10746) [`54e8239`](biomejs/biome@54e8239) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where [`noUndeclaredClasses`](https://biomejs.dev/linter/rules/no-undeclared-classes) didn't correctly detect styles defined inside the Astro directive `is:global`. - [#​10770](biomejs/biome#10770) [`dd1429c`](biomejs/biome@dd1429c) Thanks [@​ematipico](https://github.com/ematipico)! - Improved the Biome Language Server DX by orchestrating certain operations, so that they won't block the editor during typing. This improvement is more visible in large documents. - [#​10473](biomejs/biome#10473) [`d9b5133`](biomejs/biome@d9b5133) Thanks [@​Mokto](https://github.com/Mokto)! - Improved [`noUnusedImports`](https://biomejs.dev/linter/rules/no-unused-imports/), [`noUnusedVariables`](https://biomejs.dev/linter/rules/no-unused-variables/), [`noUnusedFunctionParameters`](https://biomejs.dev/linter/rules/no-unused-function-parameters/), and [`useImportType`](https://biomejs.dev/linter/rules/use-import-type/) for Svelte, Vue, and Astro files (with `html.experimentalFullSupportEnabled`). Bindings used only in the template — including component tags, attribute interpolations, directives, `bind:` shorthand, and snippet parameters — are no longer reported as unused, while genuinely unused ones still are. - [#​10796](biomejs/biome#10796) [`f1b3ab2`](biomejs/biome@f1b3ab2) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where the Biome Language Server didn't enable project or type-aware lint rules, even when they were explicitly enabled. - [#​10746](biomejs/biome#10746) [`54e8239`](biomejs/biome@54e8239) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where [`noUndeclaredClasses`](https://biomejs.dev/linter/rules/no-undeclared-classes) didn't detect styles declared inside HTML documents. - [#​10774](biomejs/biome#10774) [`bde945b`](biomejs/biome@bde945b) Thanks [@​pattrickrice](https://github.com/pattrickrice)! - Fixed [#​10268](biomejs/biome#10268) where a race condition resulted in internal errors such as: `The file biome.json does not exist in the workspace`. </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yMzQuMCIsInVwZGF0ZWRJblZlciI6IjQzLjIzNC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Client/pulls/9
This PR contains the following updates: | Package | Type | Update | Change | Pending | |---|---|---|---|---| | [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | imports | patch | [`2.5.1` -> `2.5.2`](https://renovatebot.com/diffs/npm/@biomejs%2fbiome/2.5.1/2.5.2) | `2.5.3` | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.5.2`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#252) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.5.1...@biomejs/biome@2.5.2) ##### Patch Changes - [#​10595](biomejs/biome#10595) [`f458028`](biomejs/biome@f458028) Thanks [@​pkallos](https://github.com/pkallos)! - Added the option `ignoreBooleanCoercion` to [useNullishCoalescing](https://biomejs.dev/linter/rules/use-nullish-coalescing/). When enabled, Biome ignores `||` and `||=` used inside a `Boolean()` call, where coalescing on falsy values is intentional. - [#​10798](biomejs/biome#10798) [`4a32b63`](biomejs/biome@4a32b63) Thanks [@​pkallos](https://github.com/pkallos)! - Added the option `ignorePrimitives` to [useNullishCoalescing](https://biomejs.dev/linter/rules/use-nullish-coalescing/). When enabled, Biome ignores `||`, `||=`, and ternary expressions whose non-nullish operands are all primitives the option opts out of. Use `true` to ignore all primitives, or an object selecting `string`, `number`, `boolean`, or `bigint`. - [#​10545](biomejs/biome#10545) [`f3d4c00`](biomejs/biome@f3d4c00) Thanks [@​Mokto](https://github.com/Mokto)! - Added the new nursery rule [`noSvelteUnnecessaryStateWrap`](https://biomejs.dev/linter/rules/no-svelte-unnecessary-state-wrap/), which reports unnecessary `$state()` wrapping of classes from `svelte/reactivity` that are already reactive. ```svelte <script> import { SvelteMap } from "svelte/reactivity"; const map = $state(new SvelteMap()); // redundant </script> ``` - [#​10752](biomejs/biome#10752) [`f62fb8b`](biomejs/biome@f62fb8b) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [#​10739](biomejs/biome#10739). Now the rule [`useValidAutocomplete`](https://biomejs.dev/linter/rules/use-valid-autocomplete/) correctly flags the `autoComplete` attribute. - [#​10796](biomejs/biome#10796) [`f1b3ab2`](biomejs/biome@f1b3ab2) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [#​10768](biomejs/biome#10768). Improved the performance of the Biome Language Server by cancelling certain in-flight operations when there are fast updates. - [#​10719](biomejs/biome#10719) [`aa649b5`](biomejs/biome@aa649b5) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) false positive on returns that use a widening type assertion: `"a" as string` is no longer reported as misleading. The rule now also reports a literal-pinning assertion such as `false as false`, matching the existing `as const` behavior. ```ts // No longer flagged (returns are `string`): function getValue(b: boolean): string { if (b) return "a" as string; return "b" as string; } // Now also reported, like `as const` (returns `false`): function isReady(): boolean { return false as false; } ``` - [#​10678](biomejs/biome#10678) [`8f073a7`](biomejs/biome@8f073a7) Thanks [@​PranavAchar01](https://github.com/PranavAchar01)! - Fixed [#​7718](biomejs/biome#7718): Biome now correctly parses CSS nesting selectors when `&` appears as a trailing sub-selector after a type selector, e.g. `h1& { color: red; }`. - [#​10756](biomejs/biome#10756) [`5ec965a`](biomejs/biome@5ec965a) Thanks [@​denbezrukov](https://github.com/denbezrukov)! - Fixed CSS formatter output for selector lists with `allowWrongLineComments` and `//` comments after a selector comma. Biome now keeps the selector before the line comment inline instead of breaking it across descendant combinators. ```diff -.powerPathNavigator - .helm - button.pressedButton, // pressed +.powerPathNavigator .helm button.pressedButton, // pressed .powerPathNavigator .helm button:active:not(.disabledButton) { } ``` - [#​10757](biomejs/biome#10757) [`6232fcd`](biomejs/biome@6232fcd) Thanks [@​PranavAchar01](https://github.com/PranavAchar01)! - Fixed [#​8269](biomejs/biome#8269): the CSS parser now accepts Tailwind `@variant` and `@utility` names that start with a digit, such as the `2xl` breakpoint. ```css @​utility container { @​variant 2xl { max-width: 1400px; } } ``` - [#​10777](biomejs/biome#10777) [`575ced6`](biomejs/biome@575ced6) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed an issue reported in [#​10708](biomejs/biome#10708): the GitLab reporter now handles `--verbose` diagnostics filtering correctly. - [#​10281](biomejs/biome#10281) [`0efe244`](biomejs/biome@0efe244) Thanks [@​Zelys-DFKH](https://github.com/Zelys-DFKH)! - Fixed a bug where GritQL patterns rejected positional (unkeyed) arguments. - [#​10758](biomejs/biome#10758) [`e36fd8a`](biomejs/biome@e36fd8a) Thanks [@​henrybrewer00-dotcom](https://github.com/henrybrewer00-dotcom)! - Fixed [#​10697](biomejs/biome#10697): The formatter no longer removes the parentheses around an `await` or `yield` expression used as the target of a TypeScript instantiation expression. For example, `(await makeFactory)<Value>` is no longer reformatted to `await makeFactory<Value>`, which would change the meaning of the code. - [#​10586](biomejs/biome#10586) [`3617094`](biomejs/biome@3617094) Thanks [@​IxxyDev](https://github.com/IxxyDev)! - Fixed [#​9568](biomejs/biome#9568): [`noFloatingPromises`](https://biomejs.dev/linter/rules/no-floating-promises/) no longer reports a false positive when calling an overloaded function and the selected overload does not return a promise. ```ts function bestEffort(cb: () => Promise<number>): Promise<number>; function bestEffort(cb: () => number): number; function bestEffort( cb: () => number | Promise<number>, ): Promise<number> | number { return cb() as Promise<number> | number; } // This resolves to the second overload, which returns `number`, so it is no // longer flagged as a floating promise. bestEffort(() => 42); ``` - [#​10766](biomejs/biome#10766) [`7aff4c1`](biomejs/biome@7aff4c1) Thanks [@​JamBalaya56562](https://github.com/JamBalaya56562)! - Fixed [#​2862](biomejs/biome#2862): [`noInteractiveElementToNoninteractiveRole`](https://biomejs.dev/linter/rules/no-interactive-element-to-noninteractive-role/) no longer reports custom elements (a tag name containing a dash, e.g. `<my-button role="img" />`). Per the [W3C HTML-ARIA specification](https://www.w3.org/TR/html-aria/#el-autonomous-custom-element), a custom element may be given any role or none. - [#​10680](biomejs/biome#10680) [`771daa4`](biomejs/biome@771daa4) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed [#​10635](biomejs/biome#10635): Biome now recognizes chained table tests such as `test.concurrent.each()` and `it.concurrent.each()` as test calls, fixing `noMisplacedAssertion` false positives and improving formatting for those test declarations. - [#​10759](biomejs/biome#10759) [`34570b5`](biomejs/biome@34570b5) Thanks [@​henrybrewer00-dotcom](https://github.com/henrybrewer00-dotcom)! - Fixed [#​10636](biomejs/biome#10636): [noStaticElementInteractions](https://biomejs.dev/linter/rules/no-static-element-interactions/) no longer reports a false positive for event handlers on Svelte special elements such as `<svelte:window>`, `<svelte:document>`, and `<svelte:body>`. These are not real DOM elements, so they are now ignored by the rule. - [#​10741](biomejs/biome#10741) [`bd2364e`](biomejs/biome@bd2364e) Thanks [@​JamBalaya56562](https://github.com/JamBalaya56562)! - Fixed [#​6686](biomejs/biome#6686): the `rage` command now respects the `--config-path` option and the `BIOME_CONFIG_PATH` environment variable when loading the Biome configuration. Previously it always used the default configuration resolution and reported the configuration as `Not set` when no `biome.json` existed in the working directory. - [#​10763](biomejs/biome#10763) [`2c3e82d`](biomejs/biome@2c3e82d) Thanks [@​Aqu1bp](https://github.com/Aqu1bp)! - Fixed [#​10742](biomejs/biome#10742): [`noSolidDestructuredProps`](https://biomejs.dev/linter/rules/no-solid-destructured-props) now reports destructured props in Solid function components and JSX children. - [#​10606](biomejs/biome#10606) [`a4cc4ab`](biomejs/biome@a4cc4ab) Thanks [@​Mokto](https://github.com/Mokto)! - Fixed false positives in `noUnusedImports`, `noUnusedVariables`, and `useImportType` for Svelte components that use both a `<script module>` and a `<script>` block. The two blocks compile to a single module and share a top-level scope, so a binding (import, function, or variable) declared in one block and used only in the other is no longer reported as unused. - [#​10767](biomejs/biome#10767) [`36d5aa7`](biomejs/biome@36d5aa7) Thanks [@​otkrickey](https://github.com/otkrickey)! - Fixed [#​10754](biomejs/biome#10754): [`useVueValidVBind`](https://biomejs.dev/linter/rules/use-vue-valid-v-bind/) no longer reports the Vue 3.4+ same-name shorthand as missing a value. `:foo` and `v-bind:foo` are now accepted as equivalent to `:foo="foo"`, while `v-bind`, `v-bind:[dynamicArg]`, and `:[dynamicArg]` without a value continue to be reported. - [#​10775](biomejs/biome#10775) [`a918af0`](biomejs/biome@a918af0) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed an issue reported in [#​10708](biomejs/biome#10708): `biome rage` didn't detect running Biome daemon pipes on Windows. - [#​10730](biomejs/biome#10730) [`5a2e65b`](biomejs/biome@5a2e65b) Thanks [@​dinocosta](https://github.com/dinocosta)! - Fixed an issue where Biome was resolving [the well-known Zed settings file](https://biomejs.dev/guides/configure-biome/#well-known-files) from the wrong location on macOS and Windows. - [#​10807](biomejs/biome#10807) [`d97fffe`](biomejs/biome@d97fffe) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where `.scss` files were incorrectly analyzed when running `biome check`. - [#​10672](biomejs/biome#10672) [`53c6efc`](biomejs/biome@53c6efc) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed a bug where Biome incorrectly formatted snippets that have parsing errors. - [#​10719](biomejs/biome#10719) [`aa649b5`](biomejs/biome@aa649b5) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [`useAwaitThenable`](https://biomejs.dev/linter/rules/use-await-thenable/) false positive when awaiting a custom thenable that is not the global `Promise`. A value with a callable `then` member is now recognized as awaitable. ```ts interface Thenable<T> { then(onfulfilled: (value: T) => void): void; } declare const t: Thenable<number>; async function f() { await t; } ``` - [#​10734](biomejs/biome#10734) [`4396496`](biomejs/biome@4396496) Thanks [@​BangDori](https://github.com/BangDori)! - Fixed [#​10708](biomejs/biome#10708): `biome migrate` now preserves trivia when migrating the deprecated `recommended` option to `preset`. - [#​10683](biomejs/biome#10683) [`ae31a00`](biomejs/biome@ae31a00) Thanks [@​Netail](https://github.com/Netail)! - Fixed [#​10657](biomejs/biome#10657) [#​10671](biomejs/biome#10671) [#​10661](biomejs/biome#10661) [#​10637](biomejs/biome#10637) [#​10718](biomejs/biome#10718): HTML rules now correctly handle dynamic attributes. - [#​10746](biomejs/biome#10746) [`54e8239`](biomejs/biome@54e8239) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where [`noUndeclaredClasses`](https://biomejs.dev/linter/rules/no-undeclared-classes) didn't correctly detect styles defined inside the Astro directive `is:global`. - [#​10770](biomejs/biome#10770) [`dd1429c`](biomejs/biome@dd1429c) Thanks [@​ematipico](https://github.com/ematipico)! - Improved the Biome Language Server DX by orchestrating certain operations, so that they won't block the editor during typing. This improvement is more visible in large documents. - [#​10473](biomejs/biome#10473) [`d9b5133`](biomejs/biome@d9b5133) Thanks [@​Mokto](https://github.com/Mokto)! - Improved [`noUnusedImports`](https://biomejs.dev/linter/rules/no-unused-imports/), [`noUnusedVariables`](https://biomejs.dev/linter/rules/no-unused-variables/), [`noUnusedFunctionParameters`](https://biomejs.dev/linter/rules/no-unused-function-parameters/), and [`useImportType`](https://biomejs.dev/linter/rules/use-import-type/) for Svelte, Vue, and Astro files (with `html.experimentalFullSupportEnabled`). Bindings used only in the template — including component tags, attribute interpolations, directives, `bind:` shorthand, and snippet parameters — are no longer reported as unused, while genuinely unused ones still are. - [#​10796](biomejs/biome#10796) [`f1b3ab2`](biomejs/biome@f1b3ab2) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where the Biome Language Server didn't enable project or type-aware lint rules, even when they were explicitly enabled. - [#​10746](biomejs/biome#10746) [`54e8239`](biomejs/biome@54e8239) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed an issue where [`noUndeclaredClasses`](https://biomejs.dev/linter/rules/no-undeclared-classes) didn't detect styles declared inside HTML documents. - [#​10774](biomejs/biome#10774) [`bde945b`](biomejs/biome@bde945b) Thanks [@​pattrickrice](https://github.com/pattrickrice)! - Fixed [#​10268](biomejs/biome#10268) where a race condition resulted in internal errors such as: `The file biome.json does not exist in the workspace`. </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNDYuMSIsInVwZGF0ZWRJblZlciI6IjQzLjI0Ni4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Server/pulls/22
Summary
This PR refactors our core infra to lean more towards the Salsa model.
The core changes are two:
#[salsa::tracked]#[salsa::tracked]These changes, for one reason or another, have a widespread impact, which is why they touched so many files.
Tracked functions
What does this mean? It means those services are computed based on their respective CSTs. It also means that:
In salsa, tracked functions must be pure, and that's one of the keys to its model: calling a function with the same inputs always yields the same result, which should explain what "exit early" means. We can compose tracked functions that depend on other tracked functions, and if one of them returns a memoised value, the downstream functions will yield the same result, and salsa exists early, avoiding additional recomputations.
For now, we don't have many tracked functions, but I plan to add more.
In order to understand when the output of a tracked function "is the same", we must implement
PartialEq, and that's what I have done here for the JavaScript semantic model and the CSS semantic model. Before, their implementation was absent (CSS) or incorrect (JavaScript).Salsa inputs
To make these services trackable, we need
salsa::input. In this PR, I created a newParsedSourceinput that contains the most important information needed for computing the services:AnyParseDocumentFileSourceindexThese inputs are retrieved from the "primordial databases". I call them like this because they are the foundations of any tracked function and query.
The foundation is
biome_db::Db, which exposes the functionparsed_source_for_path. It returns the CST of a given path.The second foundation is the
LanguageDb, added in thebiome_languagescrate. It exposes another primordial function calledsource_from_index, which returns theDocumentFileSourcefor a given index (the index is stored in theParsedSource).Once we have both, we can compute everything.
Workspace
Because now the CST needs to live in the Salsa database, and the services are tracked functions, we don't
DocumentServices, snippets andDocument::syntax.Document::syntaxis not an empty type because we still need to evaluate theOption(the document doesn't exist) and theResult(file size). I created anassert_parsefunction that's used before querying the database.I tried to move all the functions that interact with the database (CRUD operations) into the same region and to use a semi-naming convention. Let me know what you think. It's possible that I missed something.
Where I used AI assistance
PartialEqfor the semantic modelsList of changes
Arcfrom some services, because not neededDocumentServices,AnyEmbeddedSnippet, etc.AnyParsedSource, which is similar toAnyParseWorkspaceDbandAnyParsedSourceProjectDatabasedoesn't exist anymore, and it's now calledWorkspaceDb. This is the database where we store the data needed in theWorkspace. When downstream crates such as*_analyzeandbiome_module_graphneed to use specific functions or logic, they use a trait. That's why we have nowRc<dyn ModuleDb>andRc<dyn LanguageDb>.Test Plan
Docs