perf: reduced queries in rules#10258
Conversation
🦋 Changeset detectedLatest commit: 2e8108a 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 |
3210dcc to
5ac164a
Compare
WalkthroughThis PR adds a new node union Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/correctness/use_is_nan.rs (1)
316-320: 💤 Low valueRedundant
omit_parentheses()call.
member_objectis already the result ofomit_parentheses()on line 315, so calling it again on line 318 is unnecessary.🧹 Proposed simplification
- let (reference, name) = global_identifier( - &member_object - .omit_parentheses() - .as_any_global_identifier_expression()?, - )?; + let (reference, name) = + global_identifier(&member_object.as_any_global_identifier_expression()?)?;🤖 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/lint/correctness/use_is_nan.rs` around lines 316 - 320, The call to omit_parentheses() is duplicated: member_object was already obtained via omit_parentheses() earlier, so remove the redundant second call when passing to global_identifier; change the argument to global_identifier to use member_object.as_any_global_identifier_expression()? directly (i.e., drop the extra member_object.omit_parentheses()), leaving the rest of the invocation (global_identifier(...)? and the let (reference, name) = ...) unchanged.crates/biome_js_analyze/src/lint/nursery/no_implied_eval.rs (1)
223-227: 💤 Low valueRemove unnecessary
.clone()on line 224.
expressionis already a reference, andas_any_global_identifier_expression()takes&self, so the clone is redundant. Rust's auto-deref handles this without needing an owned copy.🤖 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/lint/nursery/no_implied_eval.rs` around lines 223 - 227, Remove the unnecessary .clone() call on expression in the pattern matching line: expression.clone().as_any_global_identifier_expression() should be called directly on expression because as_any_global_identifier_expression() takes &self and expression is already a reference; update the let binding to call expression.as_any_global_identifier_expression().and_then(|e| global_identifier(&e)) (keeping the surrounding let Some((reference, name)) = ... else { } intact) so no owned clone is created.
🤖 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_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs`:
- Around line 96-110: The narrowed union AnyBooleanCastLike removed
JsNewExpression but run() still handles JsNewExpression (see run() and
JsNewExpression::cast_ref), so restore JsNewExpression to the union and ensure
as_any_js_expression maps it to AnyJsExpression::JsNewExpression; update the
declare_node_union to include JsNewExpression and add the matching arm in
as_any_js_expression so NoExtraBooleanCast::run() can still receive and process
new Boolean(...) expressions.
In `@crates/biome_js_analyze/src/lint/style/no_process_env.rs`:
- Around line 65-66: The check should normalize parenthesised objects before
extracting the global identifier: call omit_parentheses() on the object and then
pass the normalized object into as_any_global_identifier_expression() and
global_identifier(); i.e., replace the direct use of object in the
global_identifier(&object.as_any_global_identifier_expression()?) call by first
calling object.omit_parentheses() (then use that normalized object's
as_any_global_identifier_expression() with global_identifier()) so cases like
(process).env are caught.
In `@crates/biome_js_analyze/src/lint/style/use_number_namespace.rs`:
- Around line 213-223: Special-case computed member access with property string
"Infinity" inside the AnyJsMemberExpression::JsComputedMemberExpression arm: if
the property is a string literal "Infinity" then do NOT prepend Number as a
static member; instead replace the whole expression with the appropriate numeric
member (Number.POSITIVE_INFINITY or Number.NEGATIVE_INFINITY) and preserve the
unary +/- parent replacement used by the other arms (reuse the same parent-check
and replacement logic). Concretely, detect the property token value "Infinity",
choose make::js_static_member_expression(object? no — replace node entirely with
make::js_static_member_expression(make::js_name(make::ident("Number")),
make::token(T![.]), make::js_name(make::ident("POSITIVE_INFINITY")) ) or
NEGATIVE when a unary - parent is present, and apply the same unary parent
transformation used by the other branches so runtime semantics are unchanged.
In `@crates/biome_js_analyze/src/lint/suspicious/no_alert.rs`:
- Around line 135-136: The code is missing omit_parentheses() when locating
global identifiers in member-object paths, so parenthesised globals like
(window).alert or (globalThis)["alert"] are not recognized; update both call
sites that use object.as_any_global_identifier_expression()? and similar at the
other occurrence to first call object.omit_parentheses() (and then
as_any_global_identifier_expression()?) so the global_identifier(...) receives
the unparenthesised expression; adjust the binding that currently does let
(reference, object_name) =
global_identifier(&object.as_any_global_identifier_expression()?)? to use
global_identifier(&object.omit_parentheses().as_any_global_identifier_expression()?)?
(and the analogous change for the second occurrence) ensuring proper error
handling remains.
In `@crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs`:
- Around line 175-180: The code calling global_identifier currently uses
member_expression.object().ok()?.as_any_global_identifier_expression()? without
normalizing parentheses; update the lookup to normalize the object first by
invoking omit_parentheses() (i.e., use
member_expression.object().ok()?.omit_parentheses().as_any_global_identifier_expression()?)
before passing to global_identifier so cases like (Array).from(...) are detected
consistent with other rules.
---
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/correctness/use_is_nan.rs`:
- Around line 316-320: The call to omit_parentheses() is duplicated:
member_object was already obtained via omit_parentheses() earlier, so remove the
redundant second call when passing to global_identifier; change the argument to
global_identifier to use member_object.as_any_global_identifier_expression()?
directly (i.e., drop the extra member_object.omit_parentheses()), leaving the
rest of the invocation (global_identifier(...)? and the let (reference, name) =
...) unchanged.
In `@crates/biome_js_analyze/src/lint/nursery/no_implied_eval.rs`:
- Around line 223-227: Remove the unnecessary .clone() call on expression in the
pattern matching line: expression.clone().as_any_global_identifier_expression()
should be called directly on expression because
as_any_global_identifier_expression() takes &self and expression is already a
reference; update the let binding to call
expression.as_any_global_identifier_expression().and_then(|e|
global_identifier(&e)) (keeping the surrounding let Some((reference, name)) =
... else { } intact) so no owned clone is created.
🪄 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: 4c776bc1-2fbf-45ee-be08-a359f4ed43f6
📒 Files selected for processing (36)
.changeset/sixty-hairs-rush.mdcrates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rscrates/biome_js_analyze/src/lint/complexity/use_numeric_literals.rscrates/biome_js_analyze/src/lint/complexity/use_regex_literals.rscrates/biome_js_analyze/src/lint/correctness/no_constant_math_min_max_clamp.rscrates/biome_js_analyze/src/lint/correctness/no_global_object_calls.rscrates/biome_js_analyze/src/lint/correctness/no_invalid_builtin_instantiation.rscrates/biome_js_analyze/src/lint/correctness/no_process_global.rscrates/biome_js_analyze/src/lint/correctness/no_qwik_use_visible_task.rscrates/biome_js_analyze/src/lint/correctness/use_is_nan.rscrates/biome_js_analyze/src/lint/correctness/use_parse_int_radix.rscrates/biome_js_analyze/src/lint/nursery/no_base_to_string.rscrates/biome_js_analyze/src/lint/nursery/no_implied_eval.rscrates/biome_js_analyze/src/lint/nursery/no_undeclared_env_vars.rscrates/biome_js_analyze/src/lint/nursery/no_useless_type_conversion.rscrates/biome_js_analyze/src/lint/nursery/use_named_capture_group.rscrates/biome_js_analyze/src/lint/nursery/use_unicode_regex.rscrates/biome_js_analyze/src/lint/security/no_global_eval.rscrates/biome_js_analyze/src/lint/style/no_common_js.rscrates/biome_js_analyze/src/lint/style/no_process_env.rscrates/biome_js_analyze/src/lint/style/use_array_literals.rscrates/biome_js_analyze/src/lint/style/use_consistent_builtin_instantiation.rscrates/biome_js_analyze/src/lint/style/use_exponentiation_operator.rscrates/biome_js_analyze/src/lint/style/use_number_namespace.rscrates/biome_js_analyze/src/lint/style/use_object_spread.rscrates/biome_js_analyze/src/lint/style/use_symbol_description.rscrates/biome_js_analyze/src/lint/suspicious/no_alert.rscrates/biome_js_analyze/src/lint/suspicious/no_console.rscrates/biome_js_analyze/src/lint/suspicious/no_document_cookie.rscrates/biome_js_analyze/src/lint/suspicious/no_global_is_finite.rscrates/biome_js_analyze/src/lint/suspicious/no_global_is_nan.rscrates/biome_js_analyze/src/lint/suspicious/use_error_message.rscrates/biome_js_analyze/src/lint/suspicious/use_is_array.rscrates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rscrates/biome_js_analyze/src/lint/suspicious/use_static_response_methods.rscrates/biome_js_syntax/src/expr_ext.rs
| declare_node_union! { | ||
| pub AnyBooleanCastLike = JsUnaryExpression | JsCallExpression | ||
| } | ||
|
|
||
| impl AnyBooleanCastLike { | ||
| pub(crate) fn as_any_js_expression(&self) -> AnyJsExpression { | ||
| match self { | ||
| Self::JsUnaryExpression(expr) => AnyJsExpression::JsUnaryExpression(expr.clone()), | ||
| Self::JsCallExpression(expr) => AnyJsExpression::JsCallExpression(expr.clone()), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Rule for NoExtraBooleanCast { | ||
| type Query = Ast<AnyJsExpression>; | ||
| type Query = Ast<AnyBooleanCastLike>; |
There was a problem hiding this comment.
Keep JsNewExpression in the narrowed query union.
run() still has a JsNewExpression::cast_ref(n.syntax()) path at Lines 154-170, but AnyBooleanCastLike can no longer produce that node. That quietly drops cases like if (new Boolean(foo)) {} from the rule after this perf tweak.
Possible fix
declare_node_union! {
- pub AnyBooleanCastLike = JsUnaryExpression | JsCallExpression
+ pub AnyBooleanCastLike = JsUnaryExpression | JsCallExpression | JsNewExpression
}
impl AnyBooleanCastLike {
pub(crate) fn as_any_js_expression(&self) -> AnyJsExpression {
match self {
Self::JsUnaryExpression(expr) => AnyJsExpression::JsUnaryExpression(expr.clone()),
Self::JsCallExpression(expr) => AnyJsExpression::JsCallExpression(expr.clone()),
+ Self::JsNewExpression(expr) => AnyJsExpression::JsNewExpression(expr.clone()),
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| declare_node_union! { | |
| pub AnyBooleanCastLike = JsUnaryExpression | JsCallExpression | |
| } | |
| impl AnyBooleanCastLike { | |
| pub(crate) fn as_any_js_expression(&self) -> AnyJsExpression { | |
| match self { | |
| Self::JsUnaryExpression(expr) => AnyJsExpression::JsUnaryExpression(expr.clone()), | |
| Self::JsCallExpression(expr) => AnyJsExpression::JsCallExpression(expr.clone()), | |
| } | |
| } | |
| } | |
| impl Rule for NoExtraBooleanCast { | |
| type Query = Ast<AnyJsExpression>; | |
| type Query = Ast<AnyBooleanCastLike>; | |
| declare_node_union! { | |
| pub AnyBooleanCastLike = JsUnaryExpression | JsCallExpression | JsNewExpression | |
| } | |
| impl AnyBooleanCastLike { | |
| pub(crate) fn as_any_js_expression(&self) -> AnyJsExpression { | |
| match self { | |
| Self::JsUnaryExpression(expr) => AnyJsExpression::JsUnaryExpression(expr.clone()), | |
| Self::JsCallExpression(expr) => AnyJsExpression::JsCallExpression(expr.clone()), | |
| Self::JsNewExpression(expr) => AnyJsExpression::JsNewExpression(expr.clone()), | |
| } | |
| } | |
| } | |
| impl Rule for NoExtraBooleanCast { | |
| type Query = Ast<AnyBooleanCastLike>; |
🤖 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/lint/complexity/no_extra_boolean_cast.rs` around
lines 96 - 110, The narrowed union AnyBooleanCastLike removed JsNewExpression
but run() still handles JsNewExpression (see run() and
JsNewExpression::cast_ref), so restore JsNewExpression to the union and ensure
as_any_js_expression maps it to AnyJsExpression::JsNewExpression; update the
declare_node_union to include JsNewExpression and add the matching arm in
as_any_js_expression so NoExtraBooleanCast::run() can still receive and process
new Boolean(...) expressions.
| let (reference, name) = | ||
| global_identifier(&object.as_any_global_identifier_expression()?)?; |
There was a problem hiding this comment.
Handle parenthesised process objects too
Line 66 should normalise object with omit_parentheses() first; otherwise (process).env can evade this rule.
Suggested patch
- let (reference, name) =
- global_identifier(&object.as_any_global_identifier_expression()?)?;
+ let (reference, name) = global_identifier(
+ &object
+ .omit_parentheses()
+ .as_any_global_identifier_expression()?,
+ )?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let (reference, name) = | |
| global_identifier(&object.as_any_global_identifier_expression()?)?; | |
| let (reference, name) = global_identifier( | |
| &object | |
| .omit_parentheses() | |
| .as_any_global_identifier_expression()?, | |
| )?; |
🤖 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/lint/style/no_process_env.rs` around lines 65 -
66, The check should normalize parenthesised objects before extracting the
global identifier: call omit_parentheses() on the object and then pass the
normalized object into as_any_global_identifier_expression() and
global_identifier(); i.e., replace the direct use of object in the
global_identifier(&object.as_any_global_identifier_expression()?) call by first
calling object.omit_parentheses() (then use that normalized object's
as_any_global_identifier_expression() with global_identifier()) so cases like
(process).env are caught.
| AnyJsMemberExpression::JsComputedMemberExpression(expression) => { | ||
| let object = expression.object().ok()?; | ||
| ( | ||
| object.clone(), | ||
| make::js_static_member_expression( | ||
| object, | ||
| make::token(T![.]), | ||
| make::js_name(make::ident("Number")).into(), | ||
| ) | ||
| .into(), | ||
| expression.operator_token().ok()?, | ||
| make::js_name(make::ident(replacement)).into(), | ||
| ), | ||
| ) | ||
| } | ||
| AnyJsExpression::JsComputedMemberExpression(expression) => { | ||
| let object = expression.object().ok()?; | ||
| ( | ||
| object.clone(), | ||
| make::js_static_member_expression( | ||
| object, | ||
| make::token(T![.]), | ||
| make::js_name(make::ident("Number")).into(), | ||
| ), | ||
| ) | ||
| } | ||
| _ => return None, | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Handle ["Infinity"] before prepending Number.
This arm rewrites window["Infinity"] to window.Number["Infinity"], but that property does not exist — the correct target is Number.POSITIVE_INFINITY / Number.NEGATIVE_INFINITY. It also skips the unary +/- parent replacement handled in the other branches, so the current safe fix can change runtime behaviour.
Possible fix
AnyJsMemberExpression::JsComputedMemberExpression(expression) => {
let object = expression.object().ok()?;
- (
- object.clone(),
- make::js_static_member_expression(
- object,
- make::token(T![.]),
- make::js_name(make::ident("Number")).into(),
- ),
- )
+ if global_ident.text() == "Infinity" {
+ let replacement = if let Some(parent) = node.parent::<JsUnaryExpression>() {
+ match parent.operator().ok()? {
+ JsUnaryOperator::Minus => "NEGATIVE_INFINITY",
+ JsUnaryOperator::Plus => "POSITIVE_INFINITY",
+ _ => return None,
+ }
+ } else {
+ "POSITIVE_INFINITY"
+ };
+
+ (
+ node_expr,
+ make::js_static_member_expression(
+ make::js_static_member_expression(
+ object,
+ make::token(T![.]),
+ make::js_name(make::ident("Number")).into(),
+ )
+ .into(),
+ make::token(T![.]),
+ make::js_name(make::ident(replacement)).into(),
+ ),
+ )
+ } else {
+ (
+ object.clone(),
+ make::js_static_member_expression(
+ object,
+ make::token(T![.]),
+ make::js_name(make::ident("Number")).into(),
+ ),
+ )
+ }
}🤖 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/lint/style/use_number_namespace.rs` around lines
213 - 223, Special-case computed member access with property string "Infinity"
inside the AnyJsMemberExpression::JsComputedMemberExpression arm: if the
property is a string literal "Infinity" then do NOT prepend Number as a static
member; instead replace the whole expression with the appropriate numeric member
(Number.POSITIVE_INFINITY or Number.NEGATIVE_INFINITY) and preserve the unary
+/- parent replacement used by the other arms (reuse the same parent-check and
replacement logic). Concretely, detect the property token value "Infinity",
choose make::js_static_member_expression(object? no — replace node entirely with
make::js_static_member_expression(make::js_name(make::ident("Number")),
make::token(T![.]), make::js_name(make::ident("POSITIVE_INFINITY")) ) or
NEGATIVE when a unary - parent is present, and apply the same unary parent
transformation used by the other branches so runtime semantics are unchanged.
| let (reference, object_name) = | ||
| global_identifier(&object.as_any_global_identifier_expression()?)?; |
There was a problem hiding this comment.
Handle parenthesised globals in member-object paths.
Line 135 and Line 155 skip omit_parentheses() before casting, so cases like (window).alert() / (globalThis)["alert"]() can fall through and miss diagnostics.
💡 Suggested patch
fn check_static_member_expression(
member_expr: &JsStaticMemberExpression,
model: &SemanticModel,
) -> Option<&'static str> {
- let object = member_expr.object().ok()?;
+ let object = member_expr.object().ok()?.omit_parentheses();
let (reference, object_name) =
global_identifier(&object.as_any_global_identifier_expression()?)?;
let object_name_text = object_name.text();
@@
fn check_computed_member_expression(
computed_member_expr: &JsComputedMemberExpression,
model: &SemanticModel,
) -> Option<&'static str> {
- let object = computed_member_expr.object().ok()?;
+ let object = computed_member_expr.object().ok()?.omit_parentheses();
let (reference, object_name) =
global_identifier(&object.as_any_global_identifier_expression()?)?;
let object_name_text = object_name.text();Also applies to: 155-156
🤖 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/lint/suspicious/no_alert.rs` around lines 135 -
136, The code is missing omit_parentheses() when locating global identifiers in
member-object paths, so parenthesised globals like (window).alert or
(globalThis)["alert"] are not recognized; update both call sites that use
object.as_any_global_identifier_expression()? and similar at the other
occurrence to first call object.omit_parentheses() (and then
as_any_global_identifier_expression()?) so the global_identifier(...) receives
the unparenthesised expression; adjust the binding that currently does let
(reference, object_name) =
global_identifier(&object.as_any_global_identifier_expression()?)? to use
global_identifier(&object.omit_parentheses().as_any_global_identifier_expression()?)?
(and the analogous change for the second occurrence) ensuring proper error
handling remains.
| let (_, name) = global_identifier( | ||
| &member_expression | ||
| .object() | ||
| .ok()? | ||
| .as_any_global_identifier_expression()?, | ||
| )?; |
There was a problem hiding this comment.
Normalise the object before global lookup
On Line 177, this path skips omit_parentheses(), so (Array).from(...) can be missed. Adding it keeps behaviour aligned with the other migrated rules.
Suggested patch
- let (_, name) = global_identifier(
- &member_expression
- .object()
- .ok()?
- .as_any_global_identifier_expression()?,
- )?;
+ let (_, name) = global_identifier(
+ &member_expression
+ .object()
+ .ok()?
+ .omit_parentheses()
+ .as_any_global_identifier_expression()?,
+ )?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let (_, name) = global_identifier( | |
| &member_expression | |
| .object() | |
| .ok()? | |
| .as_any_global_identifier_expression()?, | |
| )?; | |
| let (_, name) = global_identifier( | |
| &member_expression | |
| .object() | |
| .ok()? | |
| .omit_parentheses() | |
| .as_any_global_identifier_expression()?, | |
| )?; |
🤖 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/lint/suspicious/use_iterable_callback_return.rs`
around lines 175 - 180, The code calling global_identifier currently uses
member_expression.object().ok()?.as_any_global_identifier_expression()? without
normalizing parentheses; update the lookup to normalize the object first by
invoking omit_parentheses() (i.e., use
member_expression.object().ok()?.omit_parentheses().as_any_global_identifier_expression()?)
before passing to global_identifier so cases like (Array).from(...) are detected
consistent with other rules.
Merging this PR will not alter performance
Comparing Footnotes
|
|
Did you notice perf improvements? |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rs (1)
306-322: 💤 Low valueRemove the unused
AnyJsVariableScopeunion type declaration. It's no longer referenced anywhere in the codebase after the refactoring to useScope::is_variable_scope().🤖 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/lint/correctness/no_invalid_use_before_declaration.rs` around lines 306 - 322, Remove the unused union type declaration AnyJsVariableScope: locate the declare_node_union! block that defines AnyJsVariableScope and delete it; verify that no references remain (the codebase now uses Scope::is_variable_scope()), then run a build to ensure no unresolved symbols are introduced and update any imports or re-exports that referenced AnyJsVariableScope if present.
🤖 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.
Nitpick comments:
In
`@crates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rs`:
- Around line 306-322: Remove the unused union type declaration
AnyJsVariableScope: locate the declare_node_union! block that defines
AnyJsVariableScope and delete it; verify that no references remain (the codebase
now uses Scope::is_variable_scope()), then run a build to ensure no unresolved
symbols are introduced and update any imports or re-exports that referenced
AnyJsVariableScope if present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25ef6fb6-931c-4b56-a2f5-e548386fe39d
📒 Files selected for processing (6)
crates/biome_css_analyze/src/lint/style/no_descending_specificity.rscrates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rscrates/biome_js_semantic/src/events.rscrates/biome_js_semantic/src/semantic_model/binding.rscrates/biome_js_semantic/src/semantic_model/builder.rscrates/biome_js_semantic/src/semantic_model/scope.rs
I need to trigger a preview release first. |
80f2bc8 to
e8fe3d7
Compare
|
Generated via AI
|
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.4.14` -> `2.4.15`](https://renovatebot.com/diffs/npm/@biomejs%2fbiome/2.4.14/2.4.15) | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.4.15`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#2415) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.4.14...@biomejs/biome@2.4.15) ##### Patch Changes - [#​9394](biomejs/biome#9394) [`ba3480e`](biomejs/biome@ba3480e) Thanks [@​dyc3](https://github.com/dyc3)! - Added the nursery rule [`useTestHooksInOrder`](https://biomejs.dev/linter/rules/use-test-hooks-in-order) in the `test` domain. The rule enforces that Jest/Vitest lifecycle hooks (`beforeAll`, `beforeEach`, `afterEach`, `afterAll`) are declared in the order they execute, making test setup and teardown easier to reason about. - [#​10254](biomejs/biome#10254) [`e0a54cc`](biomejs/biome@e0a54cc) Thanks [@​dyc3](https://github.com/dyc3)! - Added a new nursery rule [`useVueNextTickPromise`](https://biomejs.dev/linter/rules/use-vue-next-tick-promise/), which enforces Promise syntax when using Vue `nextTick`. For example, the following snippet triggers the rule: ```js import { nextTick } from "vue"; nextTick(() => { updateDom(); }); ``` - [#​10219](biomejs/biome#10219) [`64aee45`](biomejs/biome@64aee45) Thanks [@​dyc3](https://github.com/dyc3)! - Added a new nursery rule [`noVueVOnNumberValues`](https://biomejs.dev/linter/rules/no-vue-v-on-number-values/), that disallows deprecated number modifiers on Vue `v-on` directives. For example, the following snippet triggers the rule: ```vue <input @​keyup.13="submit" /> ``` - [#​10195](biomejs/biome#10195) [`7b8d4e1`](biomejs/biome@7b8d4e1) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useVueValidVFor`](https://biomejs.dev/linter/rules/use-vue-valid-v-for/), which validates Vue `v-for` directives and reports invalid aliases, missing component keys, and keys that do not use iteration variables. - [#​10238](biomejs/biome#10238) [`1110256`](biomejs/biome@1110256) Thanks [@​dyc3](https://github.com/dyc3)! - Added the recommended nursery rule [`noVueImportCompilerMacros`](https://biomejs.dev/linter/rules/no-vue-import-compiler-macros/), which disallows importing Vue compiler macros such as `defineProps` from `vue` because they are automatically available. - [#​10201](biomejs/biome#10201) [`1a08f89`](biomejs/biome@1a08f89) Thanks [@​realknove](https://github.com/realknove)! - Fixed [#​10193](biomejs/biome#10193): `style/useReadonlyClassProperties` no longer reports class properties as readonly-able when they are assigned inside arrow callbacks nested in class property initializers. - [#​9574](biomejs/biome#9574) [`3bd2b6a`](biomejs/biome@3bd2b6a) Thanks [@​Conaclos](https://github.com/Conaclos)! - Fixed [#​9530](biomejs/biome#9530). The diagnostics of [`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/) are now more detailed and more precise. They are also better at localizing where the issue is. - [#​10205](biomejs/biome#10205) [`a704a6c`](biomejs/biome@a704a6c) Thanks [@​Conaclos](https://github.com/Conaclos)! - Fixed [#​10185](biomejs/biome#10185). [\`organizeImports](https://biomejs.dev/assist/actions/organize-imports/) now errors when it encounters an unknown predefined group. The following configuration is now reported as invalid because `:INEXISTENT:` is an unknown predefined group. ```json { "assist": { "actions": { "source": { "organizeImports": { "options": { "groups": [":INEXISTENT:"] } } } } } } ``` - [#​10052](biomejs/biome#10052) [`b565bed`](biomejs/biome@b565bed) Thanks [@​minseong0324](https://github.com/minseong0324)! - Improved [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/): it now flags union annotations whose extra variants are never returned, and suggests the narrower type (e.g. `string | null` → `string`). These functions are now reported because `null` and `number` are included in the return annotations but never returned: ```ts function getUser(): string | null { return "hello"; } // null is never returned function getCode(): string | number { return "hello"; } // number is never returned ``` - [#​10213](biomejs/biome#10213) [`ac30057`](biomejs/biome@ac30057) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​9450](biomejs/biome#9450): HTML and Vue element formatting now preserves child line breaks when an element contains another element child on its own line, instead of collapsing the child element onto the same line. - [#​10275](biomejs/biome#10275) [`9ee6c03`](biomejs/biome@9ee6c03) Thanks [@​solithcy](https://github.com/solithcy)! - Fixed [#​10274](biomejs/biome#10274): Svelte templates with missing expressions no longer parsed as `HtmlBogusElement` - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) now detects misleading return type annotations when object literal properties are initialized with `as const`. This function is now reported because the return annotation widens a property initialized with `as const`: ```ts function f(): { value: string } { return { value: "text" as const }; } ``` - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`noUselessTypeConversion`](https://biomejs.dev/linter/rules/no-useless-type-conversion/) now detects redundant conversions on object literal properties initialized with `as const`. This conversion is now reported because `message.value` is inferred as a string literal: ```ts const message = { value: "text" as const }; String(message.value); ``` - [#​9807](biomejs/biome#9807) [`0ae5840`](biomejs/biome@0ae5840) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useThisInClassMethods`](https://biomejs.dev/linter/rules/use-this-in-class-methods/), based on ESLint's `class-methods-use-this`. The rule now reports instance methods, getters, setters, and function-valued instance fields that do not use `this`, and `biome migrate eslint` preserves the supported `ignoreMethods`, `ignoreOverrideMethods`, and `ignoreClassesWithImplements` options. **Invalid**: ```js class Foo { bar() { // does not use `this`, invalid console.log("Hello Biome"); } } ``` - [#​10258](biomejs/biome#10258) [`e7b18f7`](biomejs/biome@e7b18f7) Thanks [@​ematipico](https://github.com/ematipico)! - Improved linter performance by narrowing the query nodes for several lint rules, reducing how often they are evaluated. - [#​10273](biomejs/biome#10273) [`04e22a1`](biomejs/biome@04e22a1) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10271](biomejs/biome#10271): The HTML parser now correctly parses `of` as text content when in text contexts. - [#​9838](biomejs/biome#9838) [`83f7385`](biomejs/biome@83f7385) Thanks [@​dyc3](https://github.com/dyc3)! - Added the nursery rule [`noBaseToString`](https://biomejs.dev/linter/rules/no-base-to-string/), which reports stringification sites that fall back to Object's default `"[object Object]"` formatting. The rule also supports the `ignoredTypeNames` option. - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`useExhaustiveSwitchCases`](https://biomejs.dev/linter/rules/use-exhaustive-switch-cases/) now checks switch statements over object literal properties initialized with `as const`. This switch is now reported because `status.kind` is inferred as the string literal `"ready"` but no case handles it: ```ts const status = { kind: "ready" as const }; switch (status.kind) { } ``` - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`useStringStartsEndsWith`](https://biomejs.dev/linter/rules/use-string-starts-ends-with/) now detects string index comparisons on object literal properties initialized with `as const`. This comparison is now reported because `message.value` is inferred as a string literal: ```ts const message = { value: "hello" as const }; message.value[0] === "h"; ``` </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:eyJjcmVhdGVkSW5WZXIiOiI0My4xNzMuMyIsInVwZGF0ZWRJblZlciI6IjQzLjE3My4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Server/pulls/12
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.4.14` -> `2.4.15`](https://renovatebot.com/diffs/npm/@biomejs%2fbiome/2.4.14/2.4.15) | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.4.15`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#2415) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.4.14...@biomejs/biome@2.4.15) ##### Patch Changes - [#​9394](biomejs/biome#9394) [`ba3480e`](biomejs/biome@ba3480e) Thanks [@​dyc3](https://github.com/dyc3)! - Added the nursery rule [`useTestHooksInOrder`](https://biomejs.dev/linter/rules/use-test-hooks-in-order) in the `test` domain. The rule enforces that Jest/Vitest lifecycle hooks (`beforeAll`, `beforeEach`, `afterEach`, `afterAll`) are declared in the order they execute, making test setup and teardown easier to reason about. - [#​10254](biomejs/biome#10254) [`e0a54cc`](biomejs/biome@e0a54cc) Thanks [@​dyc3](https://github.com/dyc3)! - Added a new nursery rule [`useVueNextTickPromise`](https://biomejs.dev/linter/rules/use-vue-next-tick-promise/), which enforces Promise syntax when using Vue `nextTick`. For example, the following snippet triggers the rule: ```js import { nextTick } from "vue"; nextTick(() => { updateDom(); }); ``` - [#​10219](biomejs/biome#10219) [`64aee45`](biomejs/biome@64aee45) Thanks [@​dyc3](https://github.com/dyc3)! - Added a new nursery rule [`noVueVOnNumberValues`](https://biomejs.dev/linter/rules/no-vue-v-on-number-values/), that disallows deprecated number modifiers on Vue `v-on` directives. For example, the following snippet triggers the rule: ```vue <input @​keyup.13="submit" /> ``` - [#​10195](biomejs/biome#10195) [`7b8d4e1`](biomejs/biome@7b8d4e1) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useVueValidVFor`](https://biomejs.dev/linter/rules/use-vue-valid-v-for/), which validates Vue `v-for` directives and reports invalid aliases, missing component keys, and keys that do not use iteration variables. - [#​10238](biomejs/biome#10238) [`1110256`](biomejs/biome@1110256) Thanks [@​dyc3](https://github.com/dyc3)! - Added the recommended nursery rule [`noVueImportCompilerMacros`](https://biomejs.dev/linter/rules/no-vue-import-compiler-macros/), which disallows importing Vue compiler macros such as `defineProps` from `vue` because they are automatically available. - [#​10201](biomejs/biome#10201) [`1a08f89`](biomejs/biome@1a08f89) Thanks [@​realknove](https://github.com/realknove)! - Fixed [#​10193](biomejs/biome#10193): `style/useReadonlyClassProperties` no longer reports class properties as readonly-able when they are assigned inside arrow callbacks nested in class property initializers. - [#​9574](biomejs/biome#9574) [`3bd2b6a`](biomejs/biome@3bd2b6a) Thanks [@​Conaclos](https://github.com/Conaclos)! - Fixed [#​9530](biomejs/biome#9530). The diagnostics of [`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/) are now more detailed and more precise. They are also better at localizing where the issue is. - [#​10205](biomejs/biome#10205) [`a704a6c`](biomejs/biome@a704a6c) Thanks [@​Conaclos](https://github.com/Conaclos)! - Fixed [#​10185](biomejs/biome#10185). [\`organizeImports](https://biomejs.dev/assist/actions/organize-imports/) now errors when it encounters an unknown predefined group. The following configuration is now reported as invalid because `:INEXISTENT:` is an unknown predefined group. ```json { "assist": { "actions": { "source": { "organizeImports": { "options": { "groups": [":INEXISTENT:"] } } } } } } ``` - [#​10052](biomejs/biome#10052) [`b565bed`](biomejs/biome@b565bed) Thanks [@​minseong0324](https://github.com/minseong0324)! - Improved [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/): it now flags union annotations whose extra variants are never returned, and suggests the narrower type (e.g. `string | null` → `string`). These functions are now reported because `null` and `number` are included in the return annotations but never returned: ```ts function getUser(): string | null { return "hello"; } // null is never returned function getCode(): string | number { return "hello"; } // number is never returned ``` - [#​10213](biomejs/biome#10213) [`ac30057`](biomejs/biome@ac30057) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​9450](biomejs/biome#9450): HTML and Vue element formatting now preserves child line breaks when an element contains another element child on its own line, instead of collapsing the child element onto the same line. - [#​10275](biomejs/biome#10275) [`9ee6c03`](biomejs/biome@9ee6c03) Thanks [@​solithcy](https://github.com/solithcy)! - Fixed [#​10274](biomejs/biome#10274): Svelte templates with missing expressions no longer parsed as `HtmlBogusElement` - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) now detects misleading return type annotations when object literal properties are initialized with `as const`. This function is now reported because the return annotation widens a property initialized with `as const`: ```ts function f(): { value: string } { return { value: "text" as const }; } ``` - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`noUselessTypeConversion`](https://biomejs.dev/linter/rules/no-useless-type-conversion/) now detects redundant conversions on object literal properties initialized with `as const`. This conversion is now reported because `message.value` is inferred as a string literal: ```ts const message = { value: "text" as const }; String(message.value); ``` - [#​9807](biomejs/biome#9807) [`0ae5840`](biomejs/biome@0ae5840) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useThisInClassMethods`](https://biomejs.dev/linter/rules/use-this-in-class-methods/), based on ESLint's `class-methods-use-this`. The rule now reports instance methods, getters, setters, and function-valued instance fields that do not use `this`, and `biome migrate eslint` preserves the supported `ignoreMethods`, `ignoreOverrideMethods`, and `ignoreClassesWithImplements` options. **Invalid**: ```js class Foo { bar() { // does not use `this`, invalid console.log("Hello Biome"); } } ``` - [#​10258](biomejs/biome#10258) [`e7b18f7`](biomejs/biome@e7b18f7) Thanks [@​ematipico](https://github.com/ematipico)! - Improved linter performance by narrowing the query nodes for several lint rules, reducing how often they are evaluated. - [#​10273](biomejs/biome#10273) [`04e22a1`](biomejs/biome@04e22a1) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10271](biomejs/biome#10271): The HTML parser now correctly parses `of` as text content when in text contexts. - [#​9838](biomejs/biome#9838) [`83f7385`](biomejs/biome@83f7385) Thanks [@​dyc3](https://github.com/dyc3)! - Added the nursery rule [`noBaseToString`](https://biomejs.dev/linter/rules/no-base-to-string/), which reports stringification sites that fall back to Object's default `"[object Object]"` formatting. The rule also supports the `ignoredTypeNames` option. - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`useExhaustiveSwitchCases`](https://biomejs.dev/linter/rules/use-exhaustive-switch-cases/) now checks switch statements over object literal properties initialized with `as const`. This switch is now reported because `status.kind` is inferred as the string literal `"ready"` but no case handles it: ```ts const status = { kind: "ready" as const }; switch (status.kind) { } ``` - [#​10143](biomejs/biome#10143) [`56798a7`](biomejs/biome@56798a7) Thanks [@​minseong0324](https://github.com/minseong0324)! - [`useStringStartsEndsWith`](https://biomejs.dev/linter/rules/use-string-starts-ends-with/) now detects string index comparisons on object literal properties initialized with `as const`. This comparison is now reported because `message.value` is inferred as a string literal: ```ts const message = { value: "hello" as const }; message.value[0] === "h"; ``` </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:eyJjcmVhdGVkSW5WZXIiOiI0My4xNzMuMyIsInVwZGF0ZWRJblZlciI6IjQzLjE3My4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Client/pulls/3
Summary
This PR narrows down the query of some rules. I realised this when testing Biome against big repositories, and realised that some rules were executed a lot, via --profile-rules
This PR reduces the number of nodes queried because of what they need to actually inspect.
I wrote the foundation of the change, and then asked a coding agent to update the rest of the rules.
Test Plan
Green CI
Docs