-
-
Notifications
You must be signed in to change notification settings - Fork 963
feat(inference): infer Recored<K, V>
#9383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
3a46637
feat(inference): resolve `Record<V, K>`
ematipico e2f38fb
Merge branch 'main' into feat/record-floating-promises
ematipico ab46aff
address feedback
ematipico 397ecc5
address feedback 2
ematipico 8944cf0
address feedback 3
ematipico 7d98550
address feedback 4
ematipico File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#6606](https://github.com/biomejs/biome/issues/6606): The type inference engine now resolves `Record<K, V>` 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 49 additions & 0 deletions
49
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| /* should not generate diagnostics */ | ||
|
|
||
| // Optional chaining on a possibly-undefined function property | ||
| type MaybeCallbacks = { | ||
| onSuccess?: () => Promise<void>; | ||
| 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<string, (() => 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?.(); |
57 changes: 57 additions & 0 deletions
57
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606.ts.snap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<void>; | ||
| 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<string, (() => 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?.(); | ||
|
|
||
| ``` |
30 changes: 30 additions & 0 deletions
30
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<string>; | ||
| }; | ||
| async function callAsyncApi(api: AsyncApi) { | ||
| api.fetchData(); | ||
| } | ||
|
|
||
| // Non-optional call on async method with optional chaining elsewhere | ||
| type Service = { | ||
| client?: { | ||
| request: () => Promise<void>; | ||
| }; | ||
| }; | ||
| 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<string, (() => Promise<void>) | undefined>; | ||
| const promiseHandlers: PromiseHandlers = {}; | ||
| promiseHandlers?.["someHandler"]?.(); | ||
|
|
||
| // Record with unknown-returning values combined with Promise.reject — unknown could be a Promise | ||
| const optionalObject: Record<string, (() => unknown) | undefined> = {}; | ||
| optionalObject?.nonExistentMethod?.() || | ||
| Promise.reject("optional chaining bypass"); | ||
124 changes: 124 additions & 0 deletions
124
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue6606_invalid.ts.snap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<string>; | ||
| }; | ||
| async function callAsyncApi(api: AsyncApi) { | ||
| api.fetchData(); | ||
| } | ||
|
|
||
| // Non-optional call on async method with optional chaining elsewhere | ||
| type Service = { | ||
| client?: { | ||
| request: () => Promise<void>; | ||
| }; | ||
| }; | ||
| 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<string, (() => Promise<void>) | undefined>; | ||
| const promiseHandlers: PromiseHandlers = {}; | ||
| promiseHandlers?.["someHandler"]?.(); | ||
|
|
||
| // Record with unknown-returning values combined with Promise.reject — unknown could be a Promise | ||
| const optionalObject: Record<string, (() => 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<string, (() => Promise<void>) | 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<string, (() => 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. | ||
|
|
||
|
|
||
| ``` |
19 changes: 19 additions & 0 deletions
19
crates/biome_js_analyze/tests/specs/nursery/noMisusedPromises/recordInvalid.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| /* should generate diagnostics */ | ||
|
|
||
| // Record with async function values used in void-return context | ||
| const handlers: Record<string, (() => Promise<void>) | undefined> = {}; | ||
| [1, 2, 3].forEach(async (n) => { | ||
| await handlers[n.toString()]?.(); | ||
| }); | ||
|
|
||
| // Record with Promise values used directly in conditionals | ||
| const cache: Record<string, Promise<string>> = {}; | ||
| if (cache["key"]) { | ||
| console.log("cached"); | ||
| } | ||
|
|
||
| while (cache["other"]) { | ||
| break; | ||
| } | ||
|
|
||
| const val = cache["key"] ? "yes" : "no"; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.