fix(lint): handle forwardRef callbacks in useHookAtTopLevel#9648
fix(lint): handle forwardRef callbacks in useHookAtTopLevel#9648raashish1601 wants to merge 3 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: bd86ae1 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 |
github-actions
bot
added
A-Linter
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe lint rule use_hook_at_top_level now classifies React components/hooks using semantic information: Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs`:
- Line 141: The code is using a Result pattern for function.binding() but that
method returns an Option; change the pattern from `let Ok(binding) =
function.binding()` to `let Some(binding) = function.binding()` and adjust the
corresponding else branch handling if needed in the function (the match around
the `function.binding()` call in use_hook_at_top_level.rs); ensure any
error/unreachable logic remains correct for the None case rather than treating
it like a Result::Err.
- Around line 162-177: The current code misuses SyntaxNode::parent generics and
AnyJsCallArgument APIs: replace the generic parent::<AnyJsCallArgument>() call
with a plain parent() followed by casting the returned SyntaxNode to
AnyJsCallArgument (e.g., via AnyJsCallArgument::cast or pattern match) to get an
argument value, and instead of calling as_any_js_expression() (which doesn't
exist on AnyJsCallArgument) extract the inner expression by pattern-matching the
AnyJsCallArgument variants or using the provided accessor (e.g., match on
AnyJsCallArgument::Expr or call a method like AnyJsCallArgument::expression())
so you obtain argument_expression whose omit_parentheses().syntax() can be
compared to expression.syntax(); keep the existing ancestor search using
JsCallExpression::cast unchanged.
- Around line 179-188: The compiler can't infer callee's concrete type here; add
an explicit type annotation on the let binding so the returned value from
call_expression.callee() has a concrete type before calling
callee.omit_parentheses() and is_react_call_api. Change the binding to the form
`let Ok(callee): Result<TheConcreteCalleeType, _> = call_expression.callee()
else { return false; };` (or `let Ok(callee): TheConcreteCalleeType = ...` if
the function returns the type directly), using the actual return type of
call_expression.callee() as defined in its signature, then keep the subsequent
call to callee.omit_parentheses() and is_react_call_api unchanged.
🪄 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: a996b240-0e80-4926-8c9d-c8f3804cabf4
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rscrates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js
| let Self::AnyJsFunction(function) = self else { | ||
| return false; | ||
| }; | ||
| let Ok(binding) = function.binding() else { |
There was a problem hiding this comment.
function.binding() returns Option, not Result.
The pipeline failure confirms this: use let Some(binding) = ... instead of let Ok(binding) = ....
🔧 Proposed fix
- let Ok(binding) = function.binding() else {
+ let Some(binding) = function.binding() else {📝 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 Ok(binding) = function.binding() else { | |
| let Some(binding) = function.binding() else { |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 141-141: Rust compile error E0308: mismatched types. function.binding() has type Option<AnyJsBinding>, but code pattern expects Result via let Ok(binding) = ... else. Expected Option<AnyJsBinding>, found Result<_, _>.
🪛 GitHub Actions: Lint rule docs
[error] 141-141: Rust compile error E0308 (mismatched types): function.binding() has type Option<AnyJsBinding> but code uses let Ok(binding) = ... else expecting a Result. Expected Option<AnyJsBinding>, found Result<_, _>.
🪛 GitHub Actions: Pull request Node.js
[error] 141-141: Rust compiler error E0308: mismatched types in use_hook_at_top_level.rs. let Ok(binding) = function.binding() else { ... } expects Option<AnyJsBinding> but found Result<_, _>.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs` at
line 141, The code is using a Result pattern for function.binding() but that
method returns an Option; change the pattern from `let Ok(binding) =
function.binding()` to `let Some(binding) = function.binding()` and adjust the
corresponding else branch handling if needed in the function (the match around
the `function.binding()` call in use_hook_at_top_level.rs); ensure any
error/unreachable logic remains correct for the None case rather than treating
it like a Result::Err.
| let Some(argument) = expression.syntax().parent::<AnyJsCallArgument>() else { | ||
| return false; | ||
| }; | ||
| let Some(argument_expression) = argument.as_any_js_expression() else { | ||
| return false; | ||
| }; | ||
| if argument_expression.omit_parentheses().syntax() != expression.syntax() { | ||
| return false; | ||
| } | ||
|
|
||
| let Some(call_expression) = argument | ||
| .syntax() | ||
| .ancestors() | ||
| .find_map(JsCallExpression::cast) | ||
| else { | ||
| return false; |
There was a problem hiding this comment.
Several API misuses causing compilation failures.
The parent() method on SyntaxNode doesn't accept generic arguments. You need to call parent() and then cast separately.
Additionally, once you fix the parent call, argument will be AnyJsCallArgument, which doesn't have as_any_js_expression(). You likely want to extract the inner expression via pattern matching or a dedicated method.
🔧 Proposed fix
- let Some(argument) = expression.syntax().parent::<AnyJsCallArgument>() else {
- return false;
- };
- let Some(argument_expression) = argument.as_any_js_expression() else {
- return false;
- };
- if argument_expression.omit_parentheses().syntax() != expression.syntax() {
- return false;
- }
-
- let Some(call_expression) = argument
- .syntax()
- .ancestors()
- .find_map(JsCallExpression::cast)
+ let Some(argument) = expression
+ .syntax()
+ .parent()
+ .and_then(AnyJsCallArgument::cast)
else {
return false;
};
+ let AnyJsCallArgument::AnyJsExpression(argument_expression) = &argument else {
+ return false;
+ };
+ if argument_expression.omit_parentheses().syntax() != expression.syntax() {
+ return false;
+ }
+
+ let Some(call_expression) = argument.syntax().ancestors().find_map(JsCallExpression::cast) else {
+ return false;
+ };📝 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 Some(argument) = expression.syntax().parent::<AnyJsCallArgument>() else { | |
| return false; | |
| }; | |
| let Some(argument_expression) = argument.as_any_js_expression() else { | |
| return false; | |
| }; | |
| if argument_expression.omit_parentheses().syntax() != expression.syntax() { | |
| return false; | |
| } | |
| let Some(call_expression) = argument | |
| .syntax() | |
| .ancestors() | |
| .find_map(JsCallExpression::cast) | |
| else { | |
| return false; | |
| let Some(argument) = expression | |
| .syntax() | |
| .parent() | |
| .and_then(AnyJsCallArgument::cast) | |
| else { | |
| return false; | |
| }; | |
| let AnyJsCallArgument::AnyJsExpression(argument_expression) = &argument else { | |
| return false; | |
| }; | |
| if argument_expression.omit_parentheses().syntax() != expression.syntax() { | |
| return false; | |
| } | |
| let Some(call_expression) = argument.syntax().ancestors().find_map(JsCallExpression::cast) else { | |
| return false; | |
| }; |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 162-162: Rust compile error E0107: method takes 0 generic arguments but 1 was supplied. Call expression.syntax().parent::<AnyJsCallArgument>(); parent() is defined as pub fn parent(&self) -> Option<Self> (no generics).
[error] 165-165: Rust compile error E0599: no method named as_any_js_expression found for SyntaxNode<JsLanguage> at argument.as_any_js_expression().
[error] 168-168: Rust compile error E0282: type annotations needed. Cannot infer type at argument_expression.omit_parentheses().syntax() != expression.syntax().
[error] 173-173: Rust compile error E0599: no method named syntax found for SyntaxNode<JsLanguage> at .syntax() on argument.
🪛 GitHub Actions: Lint rule docs
[error] 162-162: Rust compile error E0107: expression.syntax().parent::<AnyJsCallArgument>() supplies 1 generic argument, but method parent takes 0 generic arguments.
[error] 165-165: Rust compile error E0599: no method named as_any_js_expression found for SyntaxNode<JsLanguage>.
[error] 168-168: Rust compile error E0282: type annotations needed at argument_expression.omit_parentheses().syntax() != expression.syntax() (cannot infer type).
[error] 172-173: Rust compile error E0599: no method named syntax found for SyntaxNode<JsLanguage> at .syntax() call in let Some(call_expression) = argument .syntax() ...
🪛 GitHub Actions: Pull request Node.js
[error] 162-162: Rust compiler error E0107: method takes 0 generic arguments but 1 was supplied. expression.syntax().parent::<AnyJsCallArgument>() should be called without generics; parent() is pub fn parent(&self) -> Option<Self>.
[error] 165-165: Rust compiler error E0599: no method named as_any_js_expression found for SyntaxNode<JsLanguage> in the current scope at .as_any_js_expression().
[error] 168-168: Rust compiler error E0282: type annotations needed. Cannot infer type for argument_expression usage at if argument_expression.omit_parentheses().syntax() != expression.syntax() { ... }.
[error] 172-173: Rust compiler error E0599: no method named syntax found for struct SyntaxNode<JsLanguage> at .syntax() on call_expression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs` around
lines 162 - 177, The current code misuses SyntaxNode::parent generics and
AnyJsCallArgument APIs: replace the generic parent::<AnyJsCallArgument>() call
with a plain parent() followed by casting the returned SyntaxNode to
AnyJsCallArgument (e.g., via AnyJsCallArgument::cast or pattern match) to get an
argument value, and instead of calling as_any_js_expression() (which doesn't
exist on AnyJsCallArgument) extract the inner expression by pattern-matching the
AnyJsCallArgument variants or using the provided accessor (e.g., match on
AnyJsCallArgument::Expr or call a method like AnyJsCallArgument::expression())
so you obtain argument_expression whose omit_parentheses().syntax() can be
compared to expression.syntax(); keep the existing ancestor search using
JsCallExpression::cast unchanged.
| let Ok(callee) = call_expression.callee() else { | ||
| return false; | ||
| }; | ||
|
|
||
| is_react_call_api( | ||
| &callee.omit_parentheses(), | ||
| model, | ||
| ReactLibrary::React, | ||
| "forwardRef", | ||
| ) |
There was a problem hiding this comment.
Type annotation needed for callee.
Once the earlier errors are fixed, the compiler should be able to infer the type here. However, if it still struggles, you can add an explicit type annotation.
🔧 Optional explicit annotation
- let Ok(callee) = call_expression.callee() else {
+ let Ok(callee): Result<AnyJsExpression, _> = call_expression.callee() else {📝 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 Ok(callee) = call_expression.callee() else { | |
| return false; | |
| }; | |
| is_react_call_api( | |
| &callee.omit_parentheses(), | |
| model, | |
| ReactLibrary::React, | |
| "forwardRef", | |
| ) | |
| let Ok(callee): Result<AnyJsExpression, _> = call_expression.callee() else { | |
| return false; | |
| }; | |
| is_react_call_api( | |
| &callee.omit_parentheses(), | |
| model, | |
| ReactLibrary::React, | |
| "forwardRef", | |
| ) |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 179-179: Rust compile error E0282: type annotations needed. Cannot infer type at let Ok(callee) = call_expression.callee() else { ... }.
[error] 184-184: Rust compile error E0282: type annotations needed. Cannot infer type at &callee.omit_parentheses(),.
🪛 GitHub Actions: Lint rule docs
[error] 179-179: Rust compile error E0282: type annotations needed at let Ok(callee) = call_expression.callee() else { ... } (cannot infer type).
[error] 184-184: Rust compile error E0282: type annotations needed at &callee.omit_parentheses() (cannot infer type).
🪛 GitHub Actions: Pull request Node.js
[error] 179-179: Rust compiler error E0282: type annotations needed. Cannot infer type for let Ok(callee) = call_expression.callee() else { ... }.
[error] 184-184: Rust compiler error E0282: type annotations needed. Cannot infer type at &callee.omit_parentheses().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs` around
lines 179 - 188, The compiler can't infer callee's concrete type here; add an
explicit type annotation on the let binding so the returned value from
call_expression.callee() has a concrete type before calling
callee.omit_parentheses() and is_react_call_api. Change the binding to the form
`let Ok(callee): Result<TheConcreteCalleeType, _> = call_expression.callee()
else { return false; };` (or `let Ok(callee): TheConcreteCalleeType = ...` if
the function returns the type directly), using the actual return type of
call_expression.callee() as defined in its signature, then keep the subsequent
call to callee.omit_parentheses() and is_react_call_api unchanged.
Conaclos
added
the
M-Likely Agent
3 participants
Summary
forwardReforReact.forwardRefas component render callbacks foruseHookAtTopLevelforwardRefcallbacks in both formsCloses #9195.
Testing
rustfmt crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rscargo test -p biome_js_analyze --test spec_tests useHookAtTopLevel(fails in this worker checkout because the machine runs out of disk space during dependency compilation)