diff --git a/.changeset/resolve-record-type.md b/.changeset/resolve-record-type.md new file mode 100644 index 000000000000..252c1aea15a8 --- /dev/null +++ b/.changeset/resolve-record-type.md @@ -0,0 +1,5 @@ +--- +"@biomejs/biome": patch +--- + +Fixed [#6606](https://github.com/biomejs/biome/issues/6606): The type inference engine now resolves `Record` types, synthesizing them as object types with index signatures. This improves accuracy for type-aware lint rules such as `noFloatingPromises`, `noMisusedPromises`, `useAwaitThenable`, and `useArraySortCompare` when operating on Record-typed values. diff --git a/crates/biome_js_analyze/src/lint/nursery/use_await_thenable.rs b/crates/biome_js_analyze/src/lint/nursery/use_await_thenable.rs index 400101bd9a53..772a84f67306 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_await_thenable.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_await_thenable.rs @@ -64,7 +64,9 @@ impl Rule for UseAwaitThenable { // Uncomment the following line for debugging convenience: //let printed = format!("type of {expression:?} = {ty:?}"); - (ty.is_inferred() && !ty.is_promise_instance()).then_some(()) + let is_maybe_promise = + ty.is_promise_instance() || ty.has_variant(|ty| ty.is_promise_instance()); + (ty.is_inferred() && !is_maybe_promise).then_some(()) } fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts new file mode 100644 index 000000000000..c8fc35665be5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts @@ -0,0 +1,49 @@ +/* should not generate diagnostics */ + +// Optional chaining on a possibly-undefined function property +type MaybeCallbacks = { + onSuccess?: () => Promise; + onError?: () => void; +}; +function handleCallbacks(cbs: MaybeCallbacks) { + cbs.onError?.(); +} + +// Optional chaining with nullish coalescing — result is not a Promise +const maybeObj: { fetch?: () => string } | undefined = undefined; +maybeObj?.fetch?.() ?? "default"; + +// Optional chaining on method returning non-Promise +type Api = { + getName?: () => string; +}; +function callApi(api: Api) { + api.getName?.(); +} + +// Record with string keys, values are non-Promise functions +const handlers: Record void) | undefined> = {}; +handlers?.["someHandler"]?.(); + +// Nested optional chaining on non-Promise types +type Nested = { + inner?: { + doWork?: () => number; + }; +}; +function nested(n: Nested) { + n.inner?.doWork?.(); +} + +// Optional call on union type that doesn't include Promise +type MaybeFunc = (() => string) | undefined; +const maybeFn: MaybeFunc = undefined; +maybeFn?.(); + +// Nullish coalescing with optional chaining — result is not a Promise +const config: { getValue?: () => string } | null = null; +const result = config?.getValue?.() ?? "fallback"; + +// Logical AND with optional chaining — short-circuit, no Promise +const obj: { run?: () => void } | undefined = undefined; +obj && obj.run?.(); diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts.snap new file mode 100644 index 000000000000..707e6a43b3c0 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts.snap @@ -0,0 +1,57 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue6606.ts +--- +# Input +```ts +/* should not generate diagnostics */ + +// Optional chaining on a possibly-undefined function property +type MaybeCallbacks = { + onSuccess?: () => Promise; + onError?: () => void; +}; +function handleCallbacks(cbs: MaybeCallbacks) { + cbs.onError?.(); +} + +// Optional chaining with nullish coalescing — result is not a Promise +const maybeObj: { fetch?: () => string } | undefined = undefined; +maybeObj?.fetch?.() ?? "default"; + +// Optional chaining on method returning non-Promise +type Api = { + getName?: () => string; +}; +function callApi(api: Api) { + api.getName?.(); +} + +// Record with string keys, values are non-Promise functions +const handlers: Record void) | undefined> = {}; +handlers?.["someHandler"]?.(); + +// Nested optional chaining on non-Promise types +type Nested = { + inner?: { + doWork?: () => number; + }; +}; +function nested(n: Nested) { + n.inner?.doWork?.(); +} + +// Optional call on union type that doesn't include Promise +type MaybeFunc = (() => string) | undefined; +const maybeFn: MaybeFunc = undefined; +maybeFn?.(); + +// Nullish coalescing with optional chaining — result is not a Promise +const config: { getValue?: () => string } | null = null; +const result = config?.getValue?.() ?? "fallback"; + +// Logical AND with optional chaining — short-circuit, no Promise +const obj: { run?: () => void } | undefined = undefined; +obj && obj.run?.(); + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts new file mode 100644 index 000000000000..a04d8b3921d3 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts @@ -0,0 +1,30 @@ +/* should generate diagnostics */ + +// Optional chaining on a method that returns Promise — the call itself is not optional +type AsyncApi = { + fetchData: () => Promise; +}; +async function callAsyncApi(api: AsyncApi) { + api.fetchData(); +} + +// Non-optional call on async method with optional chaining elsewhere +type Service = { + client?: { + request: () => Promise; + }; +}; +async function callService(svc: Service) { + // The request() call is NOT optional — if client exists, this is a floating Promise + svc.client?.request(); +} + +// Record with promise-returning values — optional chaining doesn't suppress the diagnostic +type PromiseHandlers = Record Promise) | undefined>; +const promiseHandlers: PromiseHandlers = {}; +promiseHandlers?.["someHandler"]?.(); + +// Record with unknown-returning values combined with Promise.reject — unknown could be a Promise +const optionalObject: Record unknown) | undefined> = {}; +optionalObject?.nonExistentMethod?.() || + Promise.reject("optional chaining bypass"); diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts.snap new file mode 100644 index 000000000000..0d55a5cfdce9 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts.snap @@ -0,0 +1,124 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue6606_invalid.ts +--- +# Input +```ts +/* should generate diagnostics */ + +// Optional chaining on a method that returns Promise — the call itself is not optional +type AsyncApi = { + fetchData: () => Promise; +}; +async function callAsyncApi(api: AsyncApi) { + api.fetchData(); +} + +// Non-optional call on async method with optional chaining elsewhere +type Service = { + client?: { + request: () => Promise; + }; +}; +async function callService(svc: Service) { + // The request() call is NOT optional — if client exists, this is a floating Promise + svc.client?.request(); +} + +// Record with promise-returning values — optional chaining doesn't suppress the diagnostic +type PromiseHandlers = Record Promise) | undefined>; +const promiseHandlers: PromiseHandlers = {}; +promiseHandlers?.["someHandler"]?.(); + +// Record with unknown-returning values combined with Promise.reject — unknown could be a Promise +const optionalObject: Record unknown) | undefined> = {}; +optionalObject?.nonExistentMethod?.() || + Promise.reject("optional chaining bypass"); + +``` + +# Diagnostics +``` +issue6606_invalid.ts:8:2 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 6 │ }; + 7 │ async function callAsyncApi(api: AsyncApi) { + > 8 │ api.fetchData(); + │ ^^^^^^^^^^^^^^^^ + 9 │ } + 10 │ + + 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. + + i Unsafe fix: Add await operator. + + 8 │ → await·api.fetchData(); + │ ++++++ + +``` + +``` +issue6606_invalid.ts:19:2 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 17 │ async function callService(svc: Service) { + 18 │ // The request() call is NOT optional — if client exists, this is a floating Promise + > 19 │ svc.client?.request(); + │ ^^^^^^^^^^^^^^^^^^^^^^ + 20 │ } + 21 │ + + 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. + + i Unsafe fix: Add await operator. + + 19 │ → await·svc.client?.request(); + │ ++++++ + +``` + +``` +issue6606_invalid.ts:25: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. + + 23 │ type PromiseHandlers = Record Promise) | undefined>; + 24 │ const promiseHandlers: PromiseHandlers = {}; + > 25 │ promiseHandlers?.["someHandler"]?.(); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 26 │ + 27 │ // Record with unknown-returning values combined with Promise.reject — unknown could be a Promise + + 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. + + +``` + +``` +issue6606_invalid.ts:29: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. + + 27 │ // Record with unknown-returning values combined with Promise.reject — unknown could be a Promise + 28 │ const optionalObject: Record unknown) | undefined> = {}; + > 29 │ optionalObject?.nonExistentMethod?.() || + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 30 │ Promise.reject("optional chaining bypass"); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 31 │ + + 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. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts new file mode 100644 index 000000000000..94b76ff0c70f --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts @@ -0,0 +1,19 @@ +/* should generate diagnostics */ + +// Record with async function values used in void-return context +const handlers: Record Promise) | undefined> = {}; +[1, 2, 3].forEach(async (n) => { + await handlers[n.toString()]?.(); +}); + +// Record with Promise values used directly in conditionals +const cache: Record> = {}; +if (cache["key"]) { + console.log("cached"); +} + +while (cache["other"]) { + break; +} + +const val = cache["key"] ? "yes" : "no"; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts.snap new file mode 100644 index 000000000000..50975335e5d5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts.snap @@ -0,0 +1,112 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: recordInvalid.ts +--- +# Input +```ts +/* should generate diagnostics */ + +// Record with async function values used in void-return context +const handlers: Record Promise) | undefined> = {}; +[1, 2, 3].forEach(async (n) => { + await handlers[n.toString()]?.(); +}); + +// Record with Promise values used directly in conditionals +const cache: Record> = {}; +if (cache["key"]) { + console.log("cached"); +} + +while (cache["other"]) { + break; +} + +const val = cache["key"] ? "yes" : "no"; + +``` + +# Diagnostics +``` +recordInvalid.ts:5:19 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i This function returns a Promise, but no return value was expected. + + 3 │ // Record with async function values used in void-return context + 4 │ const handlers: Record Promise) | undefined> = {}; + > 5 │ [1, 2, 3].forEach(async (n) => { + │ ^^^^^^^^^^^^^^ + > 6 │ await handlers[n.toString()]?.(); + > 7 │ }); + │ ^ + 8 │ + 9 │ // Record with Promise values used directly in conditionals + + i This may not have the desired result if you expect the Promise to be `await`-ed. + + 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. + + +``` + +``` +recordInvalid.ts:11:5 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i A Promise was found where a conditional was expected. + + 9 │ // Record with Promise values used directly in conditionals + 10 │ const cache: Record> = {}; + > 11 │ if (cache["key"]) { + │ ^^^^^^^^^^^^ + 12 │ console.log("cached"); + 13 │ } + + i A Promise is always truthy, so this is most likely a mistake. + + i You may have intended to `await` the Promise instead. + + 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. + + +``` + +``` +recordInvalid.ts:15:8 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i A Promise was found where a conditional was expected. + + 13 │ } + 14 │ + > 15 │ while (cache["other"]) { + │ ^^^^^^^^^^^^^^ + 16 │ break; + 17 │ } + + i A Promise is always truthy, so this is most likely a mistake. + + i You may have intended to `await` the Promise instead. + + 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. + + +``` + +``` +recordInvalid.ts:19:13 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i A Promise was found where a conditional was expected. + + 17 │ } + 18 │ + > 19 │ const val = cache["key"] ? "yes" : "no"; + │ ^^^^^^^^^^^^ + 20 │ + + i A Promise is always truthy, so this is most likely a mistake. + + i You may have intended to `await` the Promise instead. + + 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. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordValid.ts b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordValid.ts new file mode 100644 index 000000000000..1d4d51ce8a4c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordValid.ts @@ -0,0 +1,22 @@ +/* should not generate diagnostics */ + +// Record with non-Promise function values used in conditionals +const actions: Record void) | undefined> = {}; +if (actions["save"]) { + actions["save"](); +} + +// Record with boolean values used in conditionals +const flags: Record = {}; +if (flags["enabled"]) { + console.log("enabled"); +} + +// Record with Promise values properly awaited before conditional +const fetchers: Record Promise) | undefined> = {}; +async function checkFetcher() { + const result = await fetchers["check"]?.(); + if (result) { + console.log("ok"); + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordValid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordValid.ts.snap new file mode 100644 index 000000000000..fd6844d2639e --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordValid.ts.snap @@ -0,0 +1,30 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: recordValid.ts +--- +# Input +```ts +/* should not generate diagnostics */ + +// Record with non-Promise function values used in conditionals +const actions: Record void) | undefined> = {}; +if (actions["save"]) { + actions["save"](); +} + +// Record with boolean values used in conditionals +const flags: Record = {}; +if (flags["enabled"]) { + console.log("enabled"); +} + +// Record with Promise values properly awaited before conditional +const fetchers: Record Promise) | undefined> = {}; +async function checkFetcher() { + const result = await fetchers["check"]?.(); + if (result) { + console.log("ok"); + } +} + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordInvalid.ts b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordInvalid.ts new file mode 100644 index 000000000000..683f8c8ae40c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordInvalid.ts @@ -0,0 +1,6 @@ +/* should generate diagnostics */ + +// Record with array values sorted without a comparator +const groups: Record = {}; +groups["a"].sort(); +groups["b"].toSorted(); diff --git a/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordInvalid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordInvalid.ts.snap new file mode 100644 index 000000000000..33ee9fbd1b2b --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordInvalid.ts.snap @@ -0,0 +1,56 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: recordInvalid.ts +--- +# Input +```ts +/* should generate diagnostics */ + +// Record with array values sorted without a comparator +const groups: Record = {}; +groups["a"].sort(); +groups["b"].toSorted(); + +``` + +# Diagnostics +``` +recordInvalid.ts:5:1 lint/nursery/useArraySortCompare ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Compare function missing. + + 3 │ // Record with array values sorted without a comparator + 4 │ const groups: Record = {}; + > 5 │ groups["a"].sort(); + │ ^^^^^^^^^^^^^^^^^^ + 6 │ groups["b"].toSorted(); + 7 │ + + i When called without a compare function, Array#sort() and Array#toSorted() converts all non-undefined array elements into strings and then compares said strings based off their UTF-16 code units. + + i Add a compare function to prevent unexpected sorting. + + 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. + + +``` + +``` +recordInvalid.ts:6:1 lint/nursery/useArraySortCompare ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Compare function missing. + + 4 │ const groups: Record = {}; + 5 │ groups["a"].sort(); + > 6 │ groups["b"].toSorted(); + │ ^^^^^^^^^^^^^^^^^^^^^^ + 7 │ + + i When called without a compare function, Array#sort() and Array#toSorted() converts all non-undefined array elements into strings and then compares said strings based off their UTF-16 code units. + + i Add a compare function to prevent unexpected sorting. + + 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. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordValid.ts b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordValid.ts new file mode 100644 index 000000000000..957a59ead277 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordValid.ts @@ -0,0 +1,6 @@ +/* should not generate diagnostics */ + +// Record with array values sorted with a comparator +const groups: Record = {}; +groups["a"].sort((a, b) => a - b); +groups["b"].toSorted((a, b) => a - b); diff --git a/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordValid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordValid.ts.snap new file mode 100644 index 000000000000..54deb0ccf3c2 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useArraySortCompare/recordValid.ts.snap @@ -0,0 +1,14 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: recordValid.ts +--- +# Input +```ts +/* should not generate diagnostics */ + +// Record with array values sorted with a comparator +const groups: Record = {}; +groups["a"].sort((a, b) => a - b); +groups["b"].toSorted((a, b) => a - b); + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordInvalid.ts b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordInvalid.ts new file mode 100644 index 000000000000..a556cf07c1b9 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordInvalid.ts @@ -0,0 +1,15 @@ +/* should generate diagnostics */ + +// Awaiting a Record value that is a string (not a Promise) +const config: Record = {}; +await config["key"]; + +// Awaiting a Record function value that returns a non-Promise +const getters: Record number> = {}; +await getters["count"](); + +// Awaiting a Record function value with optional chaining that returns a non-Promise +// The ?.() call on (() => number) | undefined resolves to number | undefined, +// which is not a Promise and should be flagged. +const computers: Record number) | undefined> = {}; +await computers["sum"]?.(); diff --git a/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordInvalid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordInvalid.ts.snap new file mode 100644 index 000000000000..98511ba0b62c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordInvalid.ts.snap @@ -0,0 +1,86 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: recordInvalid.ts +--- +# Input +```ts +/* should generate diagnostics */ + +// Awaiting a Record value that is a string (not a Promise) +const config: Record = {}; +await config["key"]; + +// Awaiting a Record function value that returns a non-Promise +const getters: Record number> = {}; +await getters["count"](); + +// Awaiting a Record function value with optional chaining that returns a non-Promise +// The ?.() call on (() => number) | undefined resolves to number | undefined, +// which is not a Promise and should be flagged. +const computers: Record number) | undefined> = {}; +await computers["sum"]?.(); + +``` + +# Diagnostics +``` +recordInvalid.ts:5:1 lint/nursery/useAwaitThenable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i `await` used on a non-Promise value. + + 3 │ // Awaiting a Record value that is a string (not a Promise) + 4 │ const config: Record = {}; + > 5 │ await config["key"]; + │ ^^^^^^^^^^^^^^^^^^^ + 6 │ + 7 │ // Awaiting a Record function value that returns a non-Promise + + i This may happen if you accidentally used `await` on a synchronous value. + + i Please ensure the value is not a custom "thenable" implementation before removing the `await`: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#thenables + + 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. + + +``` + +``` +recordInvalid.ts:9:1 lint/nursery/useAwaitThenable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i `await` used on a non-Promise value. + + 7 │ // Awaiting a Record function value that returns a non-Promise + 8 │ const getters: Record number> = {}; + > 9 │ await getters["count"](); + │ ^^^^^^^^^^^^^^^^^^^^^^^^ + 10 │ + 11 │ // Awaiting a Record function value with optional chaining that returns a non-Promise + + i This may happen if you accidentally used `await` on a synchronous value. + + i Please ensure the value is not a custom "thenable" implementation before removing the `await`: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#thenables + + 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. + + +``` + +``` +recordInvalid.ts:15:1 lint/nursery/useAwaitThenable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i `await` used on a non-Promise value. + + 13 │ // which is not a Promise and should be flagged. + 14 │ const computers: Record number) | undefined> = {}; + > 15 │ await computers["sum"]?.(); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ + 16 │ + + i This may happen if you accidentally used `await` on a synchronous value. + + i Please ensure the value is not a custom "thenable" implementation before removing the `await`: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#thenables + + 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. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordValid.ts b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordValid.ts new file mode 100644 index 000000000000..ec998be00897 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordValid.ts @@ -0,0 +1,16 @@ +/* should not generate diagnostics */ + +// Awaiting a Record value that is a Promise +const cache: Record> = {}; +await cache["key"]; + +// Awaiting the result of a Record function value that returns a Promise (optional call) +// The ?.() on (() => Promise) | undefined resolves to Promise | undefined; +// the Promise variant means this await is valid. +const fetchers: Record Promise) | undefined> = {}; +await fetchers["getData"]?.(); + +// Awaiting a Record value typed as Promise via type alias +type AsyncHandlers = Record Promise>; +const asyncHandlers: AsyncHandlers = {}; +await asyncHandlers["run"](); diff --git a/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordValid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordValid.ts.snap new file mode 100644 index 000000000000..57f53d89a705 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useAwaitThenable/recordValid.ts.snap @@ -0,0 +1,24 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: recordValid.ts +--- +# Input +```ts +/* should not generate diagnostics */ + +// Awaiting a Record value that is a Promise +const cache: Record> = {}; +await cache["key"]; + +// Awaiting the result of a Record function value that returns a Promise (optional call) +// The ?.() on (() => Promise) | undefined resolves to Promise | undefined; +// the Promise variant means this await is valid. +const fetchers: Record Promise) | undefined> = {}; +await fetchers["getData"]?.(); + +// Awaiting a Record value typed as Promise via type alias +type AsyncHandlers = Record Promise>; +const asyncHandlers: AsyncHandlers = {}; +await asyncHandlers["run"](); + +``` diff --git a/crates/biome_js_type_info/src/flattening/expressions.rs b/crates/biome_js_type_info/src/flattening/expressions.rs index 095ccf2a9466..54d9db05863c 100644 --- a/crates/biome_js_type_info/src/flattening/expressions.rs +++ b/crates/biome_js_type_info/src/flattening/expressions.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::hash::{Hash, Hasher}; use biome_rowan::Text; @@ -6,8 +7,8 @@ use rustc_hash::FxHasher; use crate::{ CallArgumentType, DestructureField, Function, FunctionParameter, Literal, MAX_FLATTEN_DEPTH, - Resolvable, ResolvedTypeData, ResolvedTypeMember, ResolverId, TypeData, TypeMember, - TypeReference, TypeResolver, TypeofCallExpression, TypeofDestructureExpression, + Resolvable, ResolvedTypeData, ResolvedTypeId, ResolvedTypeMember, ResolverId, TypeData, + TypeMember, TypeReference, TypeResolver, TypeofCallExpression, TypeofDestructureExpression, TypeofExpression, conditionals::{ ConditionalType, reference_to_falsy_subset_of, reference_to_non_nullish_subset_of, @@ -348,17 +349,19 @@ pub(super) fn flattened_expression( } } -fn flattened_call( - expr: &TypeofCallExpression, +/// Resolves a callee type through layers of indirection (`InstanceOf`, +/// `Interface`, `Object`) until a `Function` is found, bounded by +/// [`MAX_FLATTEN_DEPTH`] iterations. +/// +/// Returns `None` if no function can be reached. +fn resolve_callee_to_function( callee: TypeData, resolver: &mut dyn TypeResolver, -) -> Option { +) -> Option> { let mut callee = callee; for _ in 0..MAX_FLATTEN_DEPTH { match callee { - TypeData::Function(function) => { - return flattened_function_call(expr, &function, resolver); - } + TypeData::Function(function) => return Some(function), TypeData::InstanceOf(instance) => { let instance_callee = resolver.resolve_and_get(&instance.ty)?; callee = if instance_callee.is_function() { @@ -382,10 +385,64 @@ fn flattened_call( _ => break, } } - None } +fn flattened_call( + expr: &TypeofCallExpression, + callee: TypeData, + resolver: &mut dyn TypeResolver, +) -> Option { + match callee { + TypeData::Union(union) => { + // When calling a union-typed callee (e.g. `(() => Promise) | undefined`), + // resolve the call on each callable variant and collect return types. + let mut return_types = Vec::new(); + for variant in union.types() { + let Some(resolved) = resolver.resolve_and_get(variant) else { + continue; + }; + match resolved.as_raw_data() { + // Optional call (`?.`): an `undefined` or `null` callee + // short-circuits to `undefined`, so preserve it in the + // result union instead of silently dropping it. + TypeData::Undefined | TypeData::Null => { + return_types.push(TypeData::Undefined); + continue; + } + TypeData::Unknown | TypeData::TypeofExpression(_) => { + continue; + } + _ => {} + } + if let Some(function) = resolve_callee_to_function(resolved.to_data(), resolver) + && let Some(result) = flattened_function_call(expr, &function, resolver) + { + return_types.push(result); + } + } + match return_types.len() { + 0 => None, + 1 => Some(return_types.into_iter().next().unwrap()), + _ => { + let refs: Vec<_> = return_types + .into_iter() + .map(|ty| { + let id = resolver.register_type(Cow::Owned(ty)); + TypeReference::from(ResolvedTypeId::new(resolver.level(), id)) + }) + .collect(); + Some(TypeData::union_of(resolver, refs.into_boxed_slice())) + } + } + } + callee => { + let function = resolve_callee_to_function(callee, resolver)?; + flattened_function_call(expr, &function, resolver) + } + } +} + fn flattened_destructure( expr: &TypeofDestructureExpression, resolver: &mut dyn TypeResolver, diff --git a/crates/biome_js_type_info/src/resolver.rs b/crates/biome_js_type_info/src/resolver.rs index a21172b3ab94..d72312a30473 100644 --- a/crates/biome_js_type_info/src/resolver.rs +++ b/crates/biome_js_type_info/src/resolver.rs @@ -5,9 +5,9 @@ use biome_js_type_info_macros::Resolvable; use biome_rowan::Text; use crate::{ - GLOBAL_UNKNOWN_ID, NUM_PREDEFINED_TYPES, ScopeId, TypeData, TypeId, TypeImportQualifier, - TypeInstance, TypeMember, TypeMemberKind, TypeReference, TypeReferenceQualifier, TypeofValue, - Union, + GLOBAL_UNKNOWN_ID, NUM_PREDEFINED_TYPES, Object, ScopeId, TypeData, TypeId, + TypeImportQualifier, TypeInstance, TypeMember, TypeMemberKind, TypeReference, + TypeReferenceQualifier, TypeofValue, Union, globals::{GLOBAL_RESOLVER_ID, GLOBAL_UNDEFINED_ID, UNKNOWN_ID, global_type_name}, }; @@ -754,38 +754,59 @@ impl Resolvable for TypeReference { match resolved_id { Some(resolved_id) => Some(Self::Resolved(resolved_id)), None if qualifier.has_known_type_parameters() => Some({ - // If we can't resolve the qualifier as is, attempt to - // resolve it without type parameters. If it can be - // resolved that way, we create an instantiation for it - // and resolve to there. - resolver - .resolve_qualifier(&qualifier.without_type_parameters()) - .and_then(|resolved_id| { - let resolved = resolver - .get_by_resolved_id(resolved_id) - .map(|data| data.to_data()); - let parameters = - resolved.as_ref().and_then(|data| data.type_parameters())?; - let resolved_id: ResolvedTypeId = resolver.register_and_resolve( - TypeData::instance_of(TypeInstance { - ty: resolved_id.into(), - type_parameters: Self::merge_parameters( - parameters, - &qualifier.type_parameters, - ), - }), - ); - Some(resolved_id.into()) - }) - .unwrap_or_else(|| { - Self::from(TypeReferenceQualifier { - path: qualifier.path.clone(), - type_parameters: self.resolved_params(resolver), - scope_id: qualifier.scope_id, - type_only: qualifier.type_only, - excluded_binding_id: qualifier.excluded_binding_id, + // Handle Record by synthesizing an object type + // with an index signature: { [key: K]: V } + if qualifier.is_record() && qualifier.type_parameters.len() == 2 { + let params = self.resolved_params(resolver); + let key_type = params[0].clone(); + let value_type = params[1].clone(); + let resolved_id: ResolvedTypeId = + resolver.register_and_resolve(TypeData::Object(Box::new(Object { + prototype: None, + members: [TypeMember { + kind: TypeMemberKind::IndexSignature(key_type), + ty: value_type, + }] + .into(), + }))); + Self::Resolved(resolved_id) + } else { + // If we can't resolve the qualifier as is, attempt to + // resolve it without type parameters. If it can be + // resolved that way, we create an instantiation for it + // and resolve to there. + resolver + .resolve_qualifier(&qualifier.without_type_parameters()) + .and_then(|resolved_id| { + let resolved = resolver + .get_by_resolved_id(resolved_id) + .map(|data| data.to_data()); + let parameters = resolved + .as_ref() + .and_then(|data| data.type_parameters())?; + let resolved_params = self.resolved_params(resolver); + let resolved_id: ResolvedTypeId = resolver + .register_and_resolve(TypeData::instance_of( + TypeInstance { + ty: resolved_id.into(), + type_parameters: Self::merge_parameters( + parameters, + &resolved_params, + ), + }, + )); + Some(resolved_id.into()) }) - }) + .unwrap_or_else(|| { + Self::from(TypeReferenceQualifier { + path: qualifier.path.clone(), + type_parameters: self.resolved_params(resolver), + scope_id: qualifier.scope_id, + type_only: qualifier.type_only, + excluded_binding_id: qualifier.excluded_binding_id, + }) + }) + } }), None => None, } diff --git a/crates/biome_js_type_info/src/type_data.rs b/crates/biome_js_type_info/src/type_data.rs index cf452f844b27..60231e01fa19 100644 --- a/crates/biome_js_type_info/src/type_data.rs +++ b/crates/biome_js_type_info/src/type_data.rs @@ -1427,6 +1427,17 @@ impl TypeReferenceQualifier { self.path.is_identifier("Promise") } + /// Checks whether this type qualifier references a `Record` type. + /// + /// This method simply checks whether the reference is for a literal + /// `Record`, without considering whether another symbol named `Record` is + /// in scope. It can be used _after_ type resolution has failed to find a + /// `Record` symbol in scope, but should not be used _instead of_ such type + /// resolution. + pub fn is_record(&self) -> bool { + self.path.is_identifier("Record") + } + /// Checks whether this type qualifier references the `RegExp` type. /// /// This method simply checks whether the reference is for a literal