Skip to content

Commit 8192451

Browse files
authored
fix(core): infer for-of and for-in loop variables (#6809)
1 parent eebc48e commit 8192451

File tree

13 files changed

+273
-43
lines changed

13 files changed

+273
-43
lines changed

.changeset/iterable-items-infer.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Fixed [#6796](https://github.com/biomejs/biome/issues/6796): Fixed a false positive that happened in `noFloatingPromises` when calling functions that were declared as part of `for ... of` syntax inside `async` functions.
6+
7+
Instead, the variables declared inside `for ... of` loops are now correctly
8+
inferred if the expression being iterated evaluates to an `Array` (support for other iterables will follow later).
9+
10+
**Invalid example**
11+
12+
```tsx
13+
const txStatements: Array<(tx) => Promise<any>> = [];
14+
15+
db.transaction((tx: any) => {
16+
for (const stmt of txStatements) {
17+
// We correctly flag this resolves to a `Promise`:
18+
stmt(tx)
19+
}
20+
});
21+
```
22+
23+
**Valid example**
24+
25+
```tsx
26+
async function valid(db) {
27+
const txStatements: Array<(tx: any) => void> = [(tx) => tx.insert().run()];
28+
29+
db.transaction((tx: any) => {
30+
for (const stmt of txStatements) {
31+
// We don't flag a false positive here anymore:
32+
stmt(tx)
33+
}
34+
});
35+
}
36+
```

.changeset/sync-sinuses-stutter.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"@biomejs/biome": patch
33
---
44

5-
Fixed [#6796](https://github.com/biomejs/biome/issues/6796): `noFloatingPromises` will no longer suggest to add `await` keyword inside synchronous callbacks nested inside `async` functions.
5+
`noFloatingPromises` will no longer suggest to add `await` keyword inside synchronous callbacks nested inside `async` functions.

.markdownlint.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"no-duplicate-heading": null,
1818
"blanks-around-lists": null,
1919
"no-multiple-blanks": null,
20+
"no-emphasis-as-heading": null,
2021
"blanks-around-fences": null,
2122
"blanks-around-headings": null,
2223
"heading-increment": null,

crates/biome_js_analyze/tests/quick_test.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,15 @@ fn project_layout_with_top_level_dependencies(dependencies: Dependencies) -> Arc
2626
#[test]
2727
fn quick_test() {
2828
const FILENAME: &str = "dummyFile.ts";
29-
const SOURCE: &str = r#"const condition = Math.random() > -1; // Always true, but dynamic to linter
30-
condition ? Promise.reject("ternary bypass") : null;
31-
"#;
29+
const SOURCE: &str = r#"async function doStuff(db) {
30+
const txStatements: Array<(tx: any) => Promise<number>> = [(tx) => tx.insert().run()];
31+
32+
db.transaction((tx: any) => {
33+
for (const stmt of txStatements) {
34+
stmt(tx)
35+
}
36+
});
37+
}"#;
3238

3339
let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default());
3440

crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalidSyncCallbackInsideAsyncFn.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,13 @@ async function doStuff(db) {
77
}
88
});
99
}
10+
11+
async function doStuff2(db) {
12+
const txStatements: Array<Promise<(tx: any) => void>> = [];
13+
14+
db.transaction((tx: any) => {
15+
for (const stmt of txStatements) {
16+
stmt
17+
}
18+
});
19+
}

crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalidSyncCallbackInsideAsyncFn.ts.snap

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ async function doStuff(db) {
1414
});
1515
}
1616
17+
async function doStuff2(db) {
18+
const txStatements: Array<Promise<(tx: any) => void>> = [];
19+
20+
db.transaction((tx: any) => {
21+
for (const stmt of txStatements) {
22+
stmt
23+
}
24+
});
25+
}
26+
1727
```
1828
1929
# Diagnostics
@@ -33,3 +43,20 @@ invalidSyncCallbackInsideAsyncFn.ts:6:13 lint/nursery/noFloatingPromises ━━
3343
3444
3545
```
46+
47+
```
48+
invalidSyncCallbackInsideAsyncFn.ts:16:13 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━
49+
50+
i A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior.
51+
52+
14 │ db.transaction((tx: any) => {
53+
15for (const stmt of txStatements) {
54+
> 16 │ stmt
55+
│ ^^^^
56+
17 │ }
57+
18});
58+
59+
i This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator.
60+
61+
62+
```
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/* should not generate diagnostics */
2+
3+
async function doStuff(db) {
4+
const txStatements: Array<(tx: any) => void> = [(tx) => tx.insert().run()];
5+
6+
db.transaction((tx: any) => {
7+
for (const stmt of txStatements) {
8+
stmt(tx)
9+
}
10+
});
11+
}
12+
13+
async function doStuff2(db) {
14+
const txStatements: Array<Promise<(tx: any) => void>> = [];
15+
16+
db.transaction(async (tx: any) => {
17+
for await (const stmt of txStatements) {
18+
stmt(tx)
19+
}
20+
});
21+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: validSyncCallbackInsideAsyncFn.ts
4+
---
5+
# Input
6+
```ts
7+
/* should not generate diagnostics */
8+
9+
async function doStuff(db) {
10+
const txStatements: Array<(tx: any) => void> = [(tx) => tx.insert().run()];
11+
12+
db.transaction((tx: any) => {
13+
for (const stmt of txStatements) {
14+
stmt(tx)
15+
}
16+
});
17+
}
18+
19+
async function doStuff2(db) {
20+
const txStatements: Array<Promise<(tx: any) => void>> = [];
21+
22+
db.transaction(async (tx: any) => {
23+
for await (const stmt of txStatements) {
24+
stmt(tx)
25+
}
26+
});
27+
}
28+
29+
```

crates/biome_js_type_info/src/flattening/expressions.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,26 @@ pub(super) fn flattened_expression(
119119
})
120120
}
121121
}
122+
TypeofExpression::IterableValueOf(expr) => {
123+
let ty = resolver.resolve_and_get(&expr.ty)?;
124+
match ty.as_raw_data() {
125+
TypeData::InstanceOf(instance)
126+
if instance.ty == GLOBAL_ARRAY_ID.into()
127+
&& instance.has_known_type_parameters() =>
128+
{
129+
instance
130+
.type_parameters
131+
.first()
132+
.map(|param| ty.apply_module_id_to_reference(param))
133+
.and_then(|param| resolver.resolve_and_get(&param))
134+
.map(ResolvedTypeData::to_data)
135+
}
136+
_ => {
137+
// TODO: Handle other iterable types
138+
None
139+
}
140+
}
141+
}
122142
TypeofExpression::LogicalAnd(expr) => {
123143
let left = resolver.resolve_and_get(&expr.left)?;
124144
let conditional = ConditionalType::from_resolved_data(left, resolver);

crates/biome_js_type_info/src/format_type_info.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,16 @@ impl Format<FormatTypeContext> for TypeofExpression {
474474
)
475475
}
476476
},
477+
Self::IterableValueOf(expr) => {
478+
write!(
479+
f,
480+
[&format_args![&group(&format_args![
481+
text("iterable_value_of"),
482+
soft_line_break_or_space(),
483+
&expr.ty
484+
])]]
485+
)
486+
}
477487
Self::LogicalAnd(expr) => {
478488
write!(
479489
f,

0 commit comments

Comments
 (0)