fix(lint/noFloatingPromises): select the matching overload (#10586)

Author: IxxyDev

.changeset/fix-no-floating-promises-overloads.md

@@ -0,0 +1,17 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed [#9568](https://github.com/biomejs/biome/issues/9568): [`noFloatingPromises`](https://biomejs.dev/linter/rules/no-floating-promises/) no longer reports a false positive when calling an overloaded function and the selected overload does not return a promise.
+
+```ts
+function bestEffort(cb: () => Promise<number>): Promise<number>;
+function bestEffort(cb: () => number): number;
+function bestEffort(cb: () => number | Promise<number>): Promise<number> | number {
+	return cb() as Promise<number> | number;
+}
+
+// This resolves to the second overload, which returns `number`, so it is no
+// longer flagged as a floating promise.
+bestEffort(() => 42);
+```

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

@@ -0,0 +1,15 @@
+/* should not generate diagnostics */
+
+function bestEffort<T>(cb: () => Promise<T>): Promise<T | undefined>;
+function bestEffort<T>(cb: () => T): T | undefined;
+function bestEffort<T>(cb: (() => T) | (() => Promise<T>)): Promise<T | undefined> | T | undefined {
+	return undefined;
+}
+
+function syncWork(): number {
+	return 42;
+}
+
+// Generic overload exactly like the reported issue: selects the second
+// overload (sync), so it must NOT be flagged.
+bestEffort(syncWork);

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

@@ -0,0 +1,23 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: issue9568_generic.ts
+---
+# Input
+```ts
+/* should not generate diagnostics */
+
+function bestEffort<T>(cb: () => Promise<T>): Promise<T | undefined>;
+function bestEffort<T>(cb: () => T): T | undefined;
+function bestEffort<T>(cb: (() => T) | (() => Promise<T>)): Promise<T | undefined> | T | undefined {
+	return undefined;
+}
+
+function syncWork(): number {
+	return 42;
+}
+
+// Generic overload exactly like the reported issue: selects the second
+// overload (sync), so it must NOT be flagged.
+bestEffort(syncWork);
+
+```

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

@@ -0,0 +1,13 @@
+function bestEffort(cb: () => Promise<number>): Promise<number>;
+function bestEffort(cb: () => number): number;
+function bestEffort(cb: () => number | Promise<number>): Promise<number> | number {
+	return cb() as Promise<number> | number;
+}
+
+async function asyncWork(): Promise<number> {
+	return 42;
+}
+
+// Resolves to the first overload (async callback) -> returns a promise,
+// so this floating call must still be flagged.
+bestEffort(asyncWork);

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

@@ -0,0 +1,40 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: issue9568_invalid.ts
+---
+# Input
+```ts
+function bestEffort(cb: () => Promise<number>): Promise<number>;
+function bestEffort(cb: () => number): number;
+function bestEffort(cb: () => number | Promise<number>): Promise<number> | number {
+	return cb() as Promise<number> | number;
+}
+
+async function asyncWork(): Promise<number> {
+	return 42;
+}
+
+// Resolves to the first overload (async callback) -> returns a promise,
+// so this floating call must still be flagged.
+bestEffort(asyncWork);
+
+```
+
+# Diagnostics
+```
+issue9568_invalid.ts:13:1 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  i A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior.
+  
+    11 │ // Resolves to the first overload (async callback) -> returns a promise,
+    12 │ // so this floating call must still be flagged.
+  > 13 │ bestEffort(asyncWork);
+       │ ^^^^^^^^^^^^^^^^^^^^^^
+    14 │ 
+  
+  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.
+  
+  i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information.
+  
+
+```

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

@@ -0,0 +1,15 @@
+/* should not generate diagnostics */
+
+function bestEffort(cb: () => Promise<number>): Promise<number>;
+function bestEffort(cb: () => number): number;
+function bestEffort(cb: () => number | Promise<number>): Promise<number> | number {
+	return cb() as Promise<number> | number;
+}
+
+function syncWork(): number {
+	return 42;
+}
+
+// Resolves to the second overload (sync callback) -> returns `number`,
+// which is not a promise, so this must NOT be flagged.
+bestEffort(syncWork);

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

@@ -0,0 +1,23 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: issue9568_valid.ts
+---
+# Input
+```ts
+/* should not generate diagnostics */
+
+function bestEffort(cb: () => Promise<number>): Promise<number>;
+function bestEffort(cb: () => number): number;
+function bestEffort(cb: () => number | Promise<number>): Promise<number> | number {
+	return cb() as Promise<number> | number;
+}
+
+function syncWork(): number {
+	return 42;
+}
+
+// Resolves to the second overload (sync callback) -> returns `number`,
+// which is not a promise, so this must NOT be flagged.
+bestEffort(syncWork);
+
+```

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

@@ -0,0 +1,16 @@
+/* should not generate diagnostics */
+
+interface SyncCb { (): void }
+interface AsyncCb { (): Promise<void> }
+
+function run(cb: AsyncCb): Promise<void>;
+function run(cb: SyncCb): void;
+function run(cb: SyncCb | AsyncCb): Promise<void> | void {
+	const result = cb();
+	return result instanceof Promise ? result : undefined;
+}
+
+const sync: SyncCb = () => {};
+
+// SyncCb argument selects the second overload (returns `void`), must NOT be flagged.
+run(sync);

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

@@ -0,0 +1,25 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+assertion_line: 149
+expression: overloadCallableInterface.ts
+---
+# Input
+```ts
+/* should not generate diagnostics */
+
+interface SyncCb { (): void }
+interface AsyncCb { (): Promise<void> }
+
+function run(cb: AsyncCb): Promise<void>;
+function run(cb: SyncCb): void;
+function run(cb: SyncCb | AsyncCb): Promise<void> | void {
+	const result = cb();
+	return result instanceof Promise ? result : undefined;
+}
+
+const sync: SyncCb = () => {};
+
+// SyncCb argument selects the second overload (returns `void`), must NOT be flagged.
+run(sync);
+
+```

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

@@ -0,0 +1,14 @@
+/* should not generate diagnostics */
+
+function onlyAsync(cb: () => Promise<void>): Promise<void>;
+function onlyAsync(cb: () => Promise<void>, retries: number): Promise<void>;
+function onlyAsync(cb: () => Promise<void>, retries?: number): Promise<void> {
+	return cb();
+}
+
+function syncCb(): void {}
+
+// Neither overload accepts a synchronous callback, so the call resolves to no
+// signature. Its type is unknown rather than the first overload's
+// `Promise<void>`, so it must NOT be flagged as a floating promise.
+onlyAsync(syncCb);