fix(lint): respect selected overloads in noFloatingPromises#9635
fix(lint): respect selected overloads in noFloatingPromises#9635raashish1601 wants to merge 4 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 8183249 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
WalkthroughThe PR updates the Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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: 1
🤖 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/nursery/no_floating_promises.rs`:
- Around line 532-550: argument_matches_parameter currently returns true for
non-callback parameters which lets overload resolution ignore non-callback
mismatches; change the early-return in argument_matches_parameter (where
parameter_callback_returns_promise(parameter) is None) from returning true to
returning false, and update function_overload_matches_call so it tracks whether
any callback parameter matched (e.g., count or bool flag) and only accepts an
overload if at least one callback parameter matched along with all other
required checks; keep using parameter_callback_returns_promise and
expression_function_returns_promise_like for detection, and add snapshot tests
that exercise overload disambiguation with non-callback parameters to prevent
regressions.
🪄 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: 5d1d4720-05e7-4d64-9071-3a6dd0aa4489
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/contributor-14-no-floating-promises-overloads.mdcrates/biome_js_analyze/src/lint/nursery/no_floating_promises.rscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_invalid.tscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_valid.ts
Merging this PR will not alter performance
Comparing 
Footnotes
|
dyc3
added
the
M-Likely Agent
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/nursery/no_floating_promises.rs`:
- Around line 497-518: The current overload acceptance only returns
Some(OverloadMatch) when matched_callback_parameters > 0, which rejects
overloads matched solely by non-callback parameters; update the condition at the
end of the loop so the function returns an OverloadMatch when either
matched_callback_parameters > 0 or matched_non_callback_parameters > 0 (i.e.,
use (matched_callback_parameters > 0 || matched_non_callback_parameters >
0).then_some(...)) so pure non-callback matches are accepted; ensure this
touches the block constructing OverloadMatch (references:
matched_callback_parameters, matched_non_callback_parameters,
unknown_non_callback_parameters, OverloadMatch) and add a unit test covering
callback-free overloads (e.g., overloads distinguished by string literals) to
prevent regression.
- Around line 604-629: The callback_type_returns_promise logic only inspects
syntax; change it to use the semantic Type API by threading the existing
RuleContext into parameter_callback_returns_promise and
callback_type_returns_promise so you can convert the AnyTsType to a semantic
Type and call is_promise_instance()/has_variant() (similar to the usage at line
600) instead of relying on ts_type_is_promise_like; update
ts_type_is_promise_like to use semantic checks or remove it in favor of semantic
Type checks, and add regression tests for type-aliased callbacks (e.g., type
AsyncCb = () => Promise<void>) and globalThis.Promise-qualified types to prevent
regressions.
🪄 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: 4ce9a663-46c6-4add-9036-7193b6aca8fb
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_invalid.tscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_valid.ts
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_valid.ts
| let mut matched_callback_parameters = 0; | ||
| let mut matched_non_callback_parameters = 0; | ||
| let mut unknown_non_callback_parameters = 0; | ||
|
|
||
| for (index, argument) in arguments.args().iter().enumerate() { | ||
| let Ok(argument) = argument else { | ||
| return None; | ||
| }; | ||
| let parameter = parameters.get(index).or(rest_parameter)?; | ||
|
|
||
| match argument_matches_parameter(ctx, &argument, parameter)? { | ||
| ParameterMatch::Callback => matched_callback_parameters += 1, | ||
| ParameterMatch::NonCallback => matched_non_callback_parameters += 1, | ||
| ParameterMatch::Unknown => unknown_non_callback_parameters += 1, | ||
| } | ||
| } | ||
|
|
||
| (matched_callback_parameters > 0).then_some(OverloadMatch { | ||
| matched_callback_parameters, | ||
| matched_non_callback_parameters, | ||
| unknown_non_callback_parameters, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the exact code at lines 497-518
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '497,518p'Repository: biomejs/biome
Length of output: 1078
🏁 Script executed:
# Get more context around the function
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '490,530p'Repository: biomejs/biome
Length of output: 2082
🏁 Script executed:
# Find test files for this lint rule
find crates/biome_js_analyze/tests -name "*floating*" -o -name "*no_floating*"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Check if there are snapshot tests for this rule
find crates/biome_js_analyze/tests/specs -type d -name "*nursery*" | head -5Repository: biomejs/biome
Length of output: 100
🏁 Script executed:
# List test files for no_floating_promises rule
ls -la crates/biome_js_analyze/tests/specs/nursery/ | grep -i floatingRepository: biomejs/biome
Length of output: 193
🏁 Script executed:
# Check the structure of test files
find crates/biome_js_analyze/tests/specs/nursery/no_floating_promises -type fRepository: biomejs/biome
Length of output: 156
🏁 Script executed:
# Examine the function signature and understand what it does
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '480,520p'Repository: biomejs/biome
Length of output: 1716
🏁 Script executed:
# Look for where this function is called to understand context
rg "function_overload_matches_call" crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -A 2 -B 2Repository: biomejs/biome
Length of output: 434
🏁 Script executed:
# Check the OverloadMatch struct and how it's used
rg "struct OverloadMatch" crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -A 10Repository: biomejs/biome
Length of output: 389
🏁 Script executed:
# Check the actual test directory structure
ls -la crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/Repository: biomejs/biome
Length of output: 5099
🏁 Script executed:
# Get the function signature and understand it better
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '470,490p'Repository: biomejs/biome
Length of output: 831
🏁 Script executed:
# Check how the review comment describes the scenario - check the logic around line 514
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '514,520p'Repository: biomejs/biome
Length of output: 298
🏁 Script executed:
# Find the matching_overload_returns_promise_like function
rg "matching_overload_returns_promise_like" crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -A 5 -B 2Repository: biomejs/biome
Length of output: 538
🏁 Script executed:
# Check how overload matches are used and what criteria should match
rg "OverloadMatch" crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -B 3 -A 3 | head -60Repository: biomejs/biome
Length of output: 826
🏁 Script executed:
# Check the test files related to issue 9568 which seems to handle non-callback scenarios
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_valid.tsRepository: biomejs/biome
Length of output: 406
🏁 Script executed:
# Check the invalid non-callback test
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_invalid.tsRepository: biomejs/biome
Length of output: 403
🏁 Script executed:
# Look at the function definition to understand what it's supposed to do
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '460,475p'Repository: biomejs/biome
Length of output: 655
🏁 Script executed:
# Check how this function is being used - where is the check applied?
rg "function_overload_matches_call" crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -B 5 -A 5Repository: biomejs/biome
Length of output: 920
🏁 Script executed:
# Understand the full logic of how overloads are selected
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '420,460p'Repository: biomejs/biome
Length of output: 1684
🏁 Script executed:
# Look at the complete matching_overload_returns_promise_like function
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '400,425p'Repository: biomejs/biome
Length of output: 1191
🏁 Script executed:
# Check the order of overload selection - how are best matches chosen?
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '405,425p'Repository: biomejs/biome
Length of output: 929
🏁 Script executed:
# Let's look for any tests with overloads that have NO callback parameters
rg "export function.*void.*;" crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/ -A 5 | head -50Repository: biomejs/biome
Length of output: 3811
🏁 Script executed:
# Check if there's any test file that tests non-callback-only overloads (overloads with purely non-callback parameter matches)
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -name "*.ts" -exec grep -l "export function.*mode.*void" {} \;Repository: biomejs/biome
Length of output: 248
🏁 Script executed:
# Let me look at the issue9568 test files more carefully - check if the first overload has only non-callback params
cat -n crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_valid.tsRepository: biomejs/biome
Length of output: 504
🏁 Script executed:
# Search for test files with overload patterns that don't have callback parameters
rg "export function.*\(" crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/ -A 2 | grep -v "cb:" | grep -v "callback" | head -40Repository: biomejs/biome
Length of output: 825
🏁 Script executed:
# Check if the specificity function correctly handles cases with only non-callback matches
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '430,448p'Repository: biomejs/biome
Length of output: 872
🏁 Script executed:
# Let's look at the specificity implementation more carefully
rg "fn specificity" crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -A 10Repository: biomejs/biome
Length of output: 348
🏁 Script executed:
# Create a simple Python script to understand the logic flow
python3 << 'EOF'
# The condition at line 514 is: (matched_callback_parameters > 0).then_some(...)
# This returns Some(match) only if matched_callback_parameters > 0
# The specificity tuple is: (matched_callback_parameters, matched_non_callback_parameters, unknown_non_callback_parameters)
# Consider a test case like:
# export function foo(mode: "sync"): void;
# export function foo(mode: "async"): Promise<void>;
# foo("sync");
# For the first overload:
# - mode: "sync" matches the literal "sync" -> ParameterMatch::NonCallback
# - matched_callback_parameters = 0
# - matched_non_callback_parameters = 1
# - Result: (0 > 0) => False => None is returned
# For the second overload:
# - mode: "async" does NOT match "sync" -> return None from function_overload_matches_call
# So neither overload matches, and the function falls back to the implementation signature
# This is the bug the review is pointing out
print("If an overload is selected purely by non-callback parameters (like literal string matching),")
print("line 514's condition (matched_callback_parameters > 0) will return None,")
print("causing all overloads to be filtered out, falling back to the implementation signature.")
print("\nThe fix would be to also accept non-callback-only matches:")
print("(matched_callback_parameters > 0 || matched_non_callback_parameters > 0).then_some(...)")
EOFRepository: biomejs/biome
Length of output: 458
🏁 Script executed:
# Check if there are any existing test files that test overloads WITHOUT callback parameters
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -name "*.ts" -exec grep -L "cb:" {} \; | head -10Repository: biomejs/biome
Length of output: 1693
🏁 Script executed:
# Let's examine one of those files to confirm
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts | head -50Repository: biomejs/biome
Length of output: 1292
🏁 Script executed:
# Check if any test file has overload-like patterns without callbacks
rg "export function.*mode.*:" crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/*.ts | head -20Repository: biomejs/biome
Length of output: 744
Accept non-callback-only overload matches at line 514.
The condition (matched_callback_parameters > 0) filters out overloads selected purely by non-callback parameter matches (e.g., literal string arguments). For overloads like f("sync"): void vs f("async"): Promise<void>, the first overload would be rejected even when it perfectly matches the call arguments, forcing a fallback to the implementation signature and causing false positives.
Change line 514 to accept overloads with any matched parameters:
(matched_callback_parameters > 0 || matched_non_callback_parameters > 0).then_some(...)
Add a test case with callback-free overloads (e.g., overloads distinguished only by string literal arguments) to prevent regression, per coding guidelines requiring bug fixes to include tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs` around
lines 497 - 518, The current overload acceptance only returns
Some(OverloadMatch) when matched_callback_parameters > 0, which rejects
overloads matched solely by non-callback parameters; update the condition at the
end of the loop so the function returns an OverloadMatch when either
matched_callback_parameters > 0 or matched_non_callback_parameters > 0 (i.e.,
use (matched_callback_parameters > 0 || matched_non_callback_parameters >
0).then_some(...)) so pure non-callback matches are accepted; ensure this
touches the block constructing OverloadMatch (references:
matched_callback_parameters, matched_non_callback_parameters,
unknown_non_callback_parameters, OverloadMatch) and add a unit test covering
callback-free overloads (e.g., overloads distinguished by string literals) to
prevent regression.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/nursery/no_floating_promises.rs`:
- Around line 388-396: The current overload discovery fails for imported
functions because model.binding(&reference) returns an import specifier (e.g.,
JsNamedImportSpecifier) and from_binding_declaration() only handles local
function declarations; update from_binding_declaration() (or add a helper used
by it) to detect import specifiers and trace them to their original declaration:
when you encounter a JsNamedImportSpecifier (or other import binding) use the
semantic model/module resolution APIs to follow the import specifier to its
source module and locate the real function declaration (and any overloads)
before collecting overloads; keep the existing behavior for direct function
declarations so both local and imported overloaded functions are handled.
🪄 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: 762d39b1-5917-4ba8-b5e9-c49b71386ac3
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_alias_invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_alias_valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_alias_invalid.tscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_alias_valid.ts
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_alias_valid.ts
| let model = ctx.get_service::<SemanticModel>()?; | ||
| let reference = call | ||
| .callee() | ||
| .ok()? | ||
| .omit_parentheses() | ||
| .as_js_identifier_expression()? | ||
| .name() | ||
| .ok()?; | ||
| let declaration = model.binding(&reference)?.tree().declaration()?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are any tests for imported overloaded functions
rg -l 'import.*schedule|import.*overload' crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# First, locate and examine the actual file
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '380,430p'Repository: biomejs/biome
Length of output: 2184
🏁 Script executed:
#!/bin/bash
# Check what test files exist for this rule
find crates/biome_js_analyze/tests/specs/nursery -name "*no_floating_promises*" -o -name "*noFloatingPromises*" | head -20Repository: biomejs/biome
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Search more broadly for test cases with imports in the test directory
rg 'import.*from|import \{' crates/biome_js_analyze/tests/specs/nursery/ -l | grep -i floating | head -10Repository: biomejs/biome
Length of output: 914
🏁 Script executed:
#!/bin/bash
# Check one of the test files to understand the test structure
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts | head -50Repository: biomejs/biome
Length of output: 1292
🏁 Script executed:
#!/bin/bash
# Look for any test files with import statements
head -30 crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/validReexportedPromise/index.tsRepository: biomejs/biome
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Search for how model.binding works in the codebase
rg 'fn binding' crates/biome_js_semantic -A 3 | head -40Repository: biomejs/biome
Length of output: 2589
🏁 Script executed:
#!/bin/bash
# Search for function overloads in test files
rg 'function.*\(' crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/ -B 1 | grep -A 1 'function' | head -40Repository: biomejs/biome
Length of output: 2155
🏁 Script executed:
#!/bin/bash
# Look at the returnPromiseResult file to see if it has overloads
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/returnPromiseResult.tsRepository: biomejs/biome
Length of output: 260
🏁 Script executed:
#!/bin/bash
# Search the entire codebase for function overload test patterns
rg 'declare function|overload' crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/ -A 2Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check SemanticModel to understand what tree().declaration() returns for imports
rg 'fn tree' crates/biome_js_semantic/src/semantic_model/binding.rs -A 10Repository: biomejs/biome
Length of output: 475
🏁 Script executed:
#!/bin/bash
# Check the issue9568_valid.ts test completely
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_valid.tsRepository: biomejs/biome
Length of output: 384
🏁 Script executed:
#!/bin/bash
# Check if there's a test with IMPORTED overloaded functions
rg -A 10 'import.*bestEffort|import.*function' crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_valid.tsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for how declarations are resolved from imports
rg 'fn declaration' crates/biome_js_semantic/src/semantic_model -A 8 | head -60Repository: biomejs/biome
Length of output: 1664
🏁 Script executed:
#!/bin/bash
# Look for any test with imported overloaded functions (import + use overloads)
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -name "*.ts" ! -name "*.snap" -exec cat {} \; | grep -B 5 -A 5 'import.*from'Repository: biomejs/biome
Length of output: 1647
🏁 Script executed:
#!/bin/bash
# Check if SemanticModel resolves imports to source or stops at import
rg 'pub fn declaration\(' crates/biome_js_semantic -B 2 -A 15 | head -80Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Examine what AnyJsIdentifierBinding represents and if it handles imports specially
ast-grep --pattern 'pub fn tree\(\) -> $_ {
$$$
}'Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check if AnyJsIdentifierBinding has a declaration() method
rg 'fn declaration\(' crates/biome_js_syntax -A 10 | grep -B 3 -A 10 'AnyJsIdentifierBinding\|pub fn declaration'Repository: biomejs/biome
Length of output: 8714
🏁 Script executed:
#!/bin/bash
# Look at the collect_adjacent_function_overloads function to understand what it expects
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '427,480p'Repository: biomejs/biome
Length of output: 2222
🏁 Script executed:
#!/bin/bash
# Check what happens when a binding is an import specifier
rg 'JsImportSpecifier\|declaration' crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -B 2 -A 2Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look at the declaration helper function to understand what it returns
rg -B 5 'fn declaration\(parent' crates/biome_js_syntax/src/binding_ext.rs -A 20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check what AnyJsBindingDeclaration can be (including imports)
rg 'pub enum AnyJsBindingDeclaration' crates/biome_js_syntax/src/generated -A 30Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Verify the claim: when you import a function, what does model.binding().tree().declaration() return?
rg 'JsImportNamespaceClause\|JsImportSpecifier' crates/biome_js_syntax/src -l | head -5Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for the declaration function implementation
rg -A 30 'fn declaration\(' crates/biome_js_syntax/src/binding_ext.rsRepository: biomejs/biome
Length of output: 3858
🏁 Script executed:
#!/bin/bash
# Find what AnyJsBindingDeclaration includes - search the generated file
find crates/biome_js_syntax/src -name "*.rs" -exec grep -l "AnyJsBindingDeclaration" {} \;Repository: biomejs/biome
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Check the generated file directly for the enum
grep -A 50 'enum AnyJsBindingDeclaration' crates/biome_js_syntax/src/generated/unions.rs | head -60Repository: biomejs/biome
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Find AnyJsBindingDeclaration definition
rg 'pub.*AnyJsBindingDeclaration' crates/biome_js_syntax/src -B 2 -A 20 | head -100Repository: biomejs/biome
Length of output: 6669
🏁 Script executed:
#!/bin/bash
# Check what happens when an import specifier is the binding declaration
cat -n crates/biome_js_syntax/src/binding_ext.rs | sed -n '1,100p'Repository: biomejs/biome
Length of output: 5678
🏁 Script executed:
#!/bin/bash
# Check what AnyPotentialFunctionOverloadSignature can be
rg 'enum AnyPotentialFunctionOverloadSignature|pub.*AnyPotentialFunctionOverloadSignature' crates/biome_js_analyze/src -B 2 -A 20 | head -80Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look at from_binding_declaration to see if it handles imports
rg 'from_binding_declaration' crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -B 3 -A 15Repository: biomejs/biome
Length of output: 1660
🏁 Script executed:
#!/bin/bash
# Show the complete from_binding_declaration implementation
rg 'fn from_binding_declaration' crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs -A 30Repository: biomejs/biome
Length of output: 1460
🏁 Script executed:
#!/bin/bash
# Now check if there's any test that imports an overloaded function and calls it
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_valid.tsRepository: biomejs/biome
Length of output: 384
🏁 Script executed:
#!/bin/bash
# Search for tests that import from another module and test overloads
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -type f -name "*.ts" ! -name "*.snap" -print0 | xargs -0 grep -l 'import.*from'Repository: biomejs/biome
Length of output: 558
🏁 Script executed:
#!/bin/bash
# Check if invalidReexportedPromise or validReexportedPromise test imported overloads
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalidReexportedPromise/index.tsRepository: biomejs/biome
Length of output: 245
🏁 Script executed:
#!/bin/bash
# Check the other one
cat crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/validReexportedPromise/index.tsRepository: biomejs/biome
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Check if there are any other test files in those directories
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalidReexportedPromise -type fRepository: biomejs/biome
Length of output: 665
🏁 Script executed:
#!/bin/bash
# Search all test files for any that have multiple function signatures (overloads)
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -type f -name "*.ts" ! -name "*.snap" -exec grep -l 'declare function\|function.*\n.*function' {} \;Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check if there's a test directory with imported overloaded functions
find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -type dRepository: biomejs/biome
Length of output: 293
🏁 Script executed:
#!/bin/bash
# Let me search for test files with declared functions (TypeScript overloads)
rg 'declare function' crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/ -lRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Create a comprehensive search for any test with imported functions that have overloads
for file in $(find crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises -type f -name "*.ts" ! -name "*.snap"); do
if grep -q "import.*from" "$file"; then
echo "=== File: $file ==="
cat "$file"
echo ""
fi
doneRepository: biomejs/biome
Length of output: 10078
🏁 Script executed:
#!/bin/bash
# Let me trace through what actually happens with an import
# Check if SemanticModel actually does cross-module resolution
rg 'pub fn binding' crates/biome_js_semantic/src/semantic_model/model.rs -A 5Repository: biomejs/biome
Length of output: 511
🏁 Script executed:
#!/bin/bash
# Check what happens when you call model.binding on an imported identifier
# See if there's special handling for imports
rg 'import\|Import' crates/biome_js_semantic/src/semantic_model/model.rs | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Final verification: trace the exact path for an imported function
# Check what happens step by step
cat -n crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs | sed -n '384,397p'Repository: biomejs/biome
Length of output: 661
🏁 Script executed:
#!/bin/bash
# Check if there's any cross-module type resolution in semantic model
rg 'cross.module\|cross_module\|inter.module\|cross.file' crates/biome_js_semantic -iRepository: biomejs/biome
Length of output: 39
Imported overloaded functions won't have overloads discovered.
model.binding(&reference) resolves to the import specifier itself (e.g., JsNamedImportSpecifier), not the original function declaration. The from_binding_declaration() helper only handles function declarations, not imports, so it returns None and overload collection fails silently.
This means calls to imported overloaded functions skip overload checking:
import { schedule } from './scheduler';
schedule("sync", cb); // Overloads in scheduler.ts won't be checkedConsider extending from_binding_declaration() to trace imports to their source declarations, or document this limitation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs` around
lines 388 - 396, The current overload discovery fails for imported functions
because model.binding(&reference) returns an import specifier (e.g.,
JsNamedImportSpecifier) and from_binding_declaration() only handles local
function declarations; update from_binding_declaration() (or add a helper used
by it) to detect import specifiers and trace them to their original declaration:
when you encounter a JsNamedImportSpecifier (or other import binding) use the
semantic model/module resolution APIs to follow the import specifier to its
source module and locate the real function declaration (and any overloads)
before collecting overloads; keep the existing behavior for direct function
declarations so both local and imported overloaded functions are handled.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs (1)
388-396: Imported overloaded functions fall back to semantic type checking.When the callee is imported,
model.binding(&reference)?.tree().declaration()returns the import specifier rather than the original declaration. Sincefrom_binding_declaration()doesn't handle imports, overload collection silently fails and the rule falls back to the semanticis_promise_instance()check at lines 195-196.This is a reasonable fallback but worth documenting. Consider adding a TODO or a brief comment explaining this limitation for future contributors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs` around lines 388 - 396, The current lookup using model.binding(&reference)?.tree().declaration() can return an import specifier for imported callees, causing from_binding_declaration() to not collect overloads and the rule to silently fall back to the semantic is_promise_instance() check; add a brief TODO/comment adjacent to the callee/reference/declaration logic (near the get_service::<SemanticModel>() and from_binding_declaration() usage) explaining that imported overloaded functions resolve to import specifiers, that from_binding_declaration() doesn't handle imports and thus overload collection fails, and note this as a known limitation for future improvement (e.g., resolve imports to original declarations before collecting overloads).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs`:
- Around line 388-396: The current lookup using
model.binding(&reference)?.tree().declaration() can return an import specifier
for imported callees, causing from_binding_declaration() to not collect
overloads and the rule to silently fall back to the semantic
is_promise_instance() check; add a brief TODO/comment adjacent to the
callee/reference/declaration logic (near the get_service::<SemanticModel>() and
from_binding_declaration() usage) explaining that imported overloaded functions
resolve to import specifiers, that from_binding_declaration() doesn't handle
imports and thus overload collection fails, and note this as a known limitation
for future improvement (e.g., resolve imports to original declarations before
collecting overloads).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c368d5a-af2d-464a-b447-ec2aceffbdfb
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_only_invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_only_valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_only_invalid.tscrates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_only_valid.ts
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue9568_non_callback_only_valid.ts
3 participants
Summary
oFloatingPromises to recover adjacent TypeScript function overloads from the bound declaration
Testing