Skip to content

Commit 4bf9b21

Browse files
authored
fix(inference): argument and type parameters inference (#10992)
1 parent a5e4bab commit 4bf9b21

18 files changed

Lines changed: 3280 additions & 1765 deletions

File tree

.changeset/rude-bats-fall.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Fixed [`noMisusedPromises`](https://biomejs.dev/linter/rules/no-misused-promises/): The rule now reports Promise-returning callbacks where a synchronous callback is expected when calls use tuple spreads or tuple rest parameters, including generic and deeply nested tuples, and when constructor signatures come from interface or object types. Recursive or excessively nested tuple spreads use a conservative fallback so analysis terminates.
6+
7+
For example, the following callback is now reported.
8+
9+
```ts
10+
declare function consume(...args: [number, () => void]): void;
11+
const prefix: [number] = [1];
12+
13+
consume(...prefix, async () => {});
14+
```

.claude/skills/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ This applies to all agents, all skills, and all contributions. Keep code and doc
4646
| **[biome-developer](./biome-developer/SKILL.md)** | General development best practices, common gotchas, Biome-specific patterns | Any agent |
4747
| **[testing-codegen](./testing-codegen/SKILL.md)** | Run tests, manage snapshots, generate code | Any agent |
4848
| **[changeset](./changeset/SKILL.md)** | Create and write proper changesets for the CHANGELOG | Any agent |
49+
| **[doc-comments](./doc-comments/SKILL.md)** | Write comments and rustdoc addressed to developers, not end users | Any agent |
4950
| **[pull-request](./pull-request/SKILL.md)** | Create PRs with proper titles, descriptions, and branch targeting | Any agent |
5051
| **[type-inference](./type-inference/SKILL.md)** | Work with module graph and type inference system | `biome-lint-engineer` |
5152
| **[diagnostics-development](./diagnostics-development/SKILL.md)** | Create user-friendly error messages and diagnostics | Any agent |
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
---
2+
name: doc-comments
3+
description: How to write inline comments, rustdoc, and module documentation in the Biome codebase. The audience is Biome developers reading the source, not end users. Use whenever writing or editing `//` comments, `///` item docs, or `//!` module docs — including comments added incidentally while fixing bugs or implementing features.
4+
compatibility: Designed for coding agents working on the Biome codebase (github.com/biomejs/biome).
5+
---
6+
7+
## Purpose
8+
9+
Comments and doc comments in this repository are read by contributors, months
10+
or years after they were written, with none of the context you have right now.
11+
This skill defines who that reader is, what each kind of comment is for, and
12+
which patterns are banned.
13+
14+
**Scope boundary:** rustdoc inside `declare_lint_rule!` / `declare_assist_rule!`
15+
blocks is end-user documentation — it is generated into the website. This skill
16+
does not apply there; see [lint-rule-development](../lint-rule-development/SKILL.md).
17+
18+
## The Reader
19+
20+
Write for a Biome contributor who is competent in Rust but has **no access to
21+
your current context**: not this conversation, not the pull request, not the
22+
issue, not the diff. They see only the repository at HEAD.
23+
24+
Two consequences follow directly:
25+
26+
1. **Never narrate change history.** Words like "now", "previously",
27+
"no longer", "the new approach" are meaningless at HEAD, where only one
28+
approach exists. State how the code works, not how it came to be.
29+
2. **Never address the reviewer.** A comment that argues your change is
30+
correct ("this properly handles X") belongs in the PR description, not in
31+
the source. The comment must justify the code as it stands, permanently.
32+
33+
## Three Kinds of Documentation, Three Different Jobs
34+
35+
| Kind | Job | Contains |
36+
| ---- | --- | -------- |
37+
| `//!` module docs | Explanation | Why the module exists, core concepts and terminology, how the pieces relate, design rationale |
38+
| `///` item docs | Reference | The contract: behavior, inputs and outputs, invariants, panics, errors. Neutral and factual |
39+
| `//` inline comments | Rationale | Only what the code cannot say: constraints, workarounds (with issue links), non-obvious coupling, why the obvious alternative is wrong |
40+
41+
Do not mix the jobs. Implementation details do not belong in `///` docs — put
42+
them as `//` comments inside the body. The contract does not belong scattered
43+
across inline comments — put it on the item.
44+
45+
## The Deletion Test
46+
47+
Before writing any comment, ask: **does this state something the reader cannot
48+
recover from the code itself?**
49+
50+
- If the information is already carried by names, types, or structure, do not
51+
write the comment. If the name fails to carry it, improve the name.
52+
- Information that legitimately needs a comment: an invariant, a rationale, a
53+
coupling to code elsewhere, a workaround with a link, surprising behavior of
54+
a dependency, a term of art the module defines.
55+
56+
When editing later, the same test applies in reverse: a comment that no longer
57+
passes it should be deleted, not left to rot.
58+
59+
## Banned Patterns
60+
61+
**Narrating the next line.** Delete these on sight:
62+
63+
```rust
64+
// Increment the generation counter
65+
generation += 1;
66+
```
67+
68+
**Change-history narration.** Rewrite as present-tense rationale:
69+
70+
```rust
71+
// BAD: We now intern types instead of cloning them.
72+
// GOOD: Interning avoids cloning these types on every lookup.
73+
```
74+
75+
**Reviewer-addressed justification.** Move the argument to the PR:
76+
77+
```rust
78+
// BAD: This correctly handles the overload case from the bug report.
79+
// GOOD: Overloads are matched by arity before parameter types, so a
80+
// partial-arity call cannot select the wrong candidate.
81+
```
82+
83+
**Restated rustdoc.** A `///` doc that rewords the item name says nothing:
84+
85+
```rust
86+
// BAD:
87+
/// Handles the type inference.
88+
fn infer_types(...)
89+
90+
// GOOD:
91+
/// Infers the type of `expr` in the scope of `module`, returning
92+
/// `TypeData::Unknown` when the expression references an unresolved import.
93+
fn infer_types(...)
94+
```
95+
96+
**Vague hedging.** "Some cases", "various reasons", "handles edge cases",
97+
"etc."either name them or drop the sentence.
98+
99+
**Emojis.** Banned everywhere in this repository, comments included.
100+
101+
**Ad-hoc section banners** (`// ----- helpers -----`, `// ==== TYPES ====`).
102+
For grouping in long files, use the region comment pattern below instead.
103+
104+
## Region Comments
105+
106+
Long files group related items with paired region markers:
107+
108+
```rust
109+
// #region FILE-LEVEL METHODS
110+
...
111+
// #endregion
112+
```
113+
114+
This is an established convention across the codebase (`biome_service`,
115+
`biome_module_graph`, `biome_rowan`, the parsers). The `Workspace` trait in
116+
[`crates/biome_service/src/workspace.rs`](../../../crates/biome_service/src/workspace.rs)
117+
uses it to group its methods (`PROJECT-LEVEL METHODS`, `FILE-LEVEL METHODS`,
118+
`SEARCH-RELATED METHODS`). Editors fold on these markers, which is the point:
119+
they exist for navigation, not documentation.
120+
121+
Rules:
122+
123+
- Every `// #region` has a matching `// #endregion`. An unpaired marker breaks
124+
editor folding silently.
125+
- The name states what the group contains. It can be a plain label
126+
(`Shared helpers`) or anchored to a function (`#region parse_thematic_break_parts`)
127+
when the region holds one entry point and its private support code.
128+
- Use regions only where they earn their keep: files or `impl`/`trait` blocks
129+
long enough that folding helps. A file that fits on two screens does not
130+
need them.
131+
- A region name is organization, not documentation. It never substitutes for
132+
rustdoc on the items inside it.
133+
134+
## Editing Existing Code
135+
136+
- Preserve existing doc comments. If your change alters behavior, extend or
137+
correct the specific prosenever replace it with generic text. Deleting
138+
hard-won context is worse than leaving a comment slightly stale.
139+
- Match the surrounding density. A heavily documented module deserves the same
140+
level on new items; do not blanket a sparse module with comments.
141+
142+
## Exemplar
143+
144+
The `//!` module docs at the top of
145+
[`crates/biome_service/src/workspace.rs`](../../../crates/biome_service/src/workspace.rs)
146+
show the target register. They define a term the rest of the module depends on
147+
("open documents") and give its meaning in both the LSP and CLI contexts; they
148+
explain a design decision the signatures alone would make confusing (the
149+
workspace is stateful, yet every method takes `&self`, because the trait must
150+
be thread-safe and caching happens internally); and they state the error
151+
philosophy once, at the top, instead of repeating it on every method.
152+
Everything is present tense; nothing mentions how the design evolved or
153+
defends a change.
154+
155+
## Self-Check Before Finishing
156+
157+
After completing any task that touched comments, re-read **only the comments
158+
in your diff**, in isolation from the code changes:
159+
160+
1. Does each one pass the deletion test?
161+
2. Does any reference the conversation, the change itself, or the reviewer?
162+
3. Would a reader without access to the diff understand each one?
163+
164+
Fix or delete what fails. Deletion is the default; a missing comment is
165+
cheaper than a misleading one.
166+
167+
## References
168+
169+
- [Diátaxis](https://diataxis.fr/) — the framework behind the
170+
explanation / reference / rationale split above.
171+
- [lint-rule-development](../lint-rule-development/SKILL.md) — for rule
172+
rustdoc, which is end-user documentation.

.claude/skills/type-inference/SKILL.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ TypeData::TypeofExpression(TypeofExpression::Addition {
104104
3. Converts to `TypeReference::Resolved` if found locally
105105
4. Converts to `TypeReference::Import` if from import statement
106106
5. Falls back to globals (like `Array`, `Promise`)
107-
6. Uses `TypeReference::Unknown` if nothing found
107+
6. Uses `TypeReference::unknown()` if nothing is found
108108

109109
**Where**: Implemented in `js_module_info/collector.rs`
110110

@@ -119,9 +119,13 @@ TypeData::TypeofExpression(TypeofExpression::Addition {
119119
2. Resolves `TypeReference::Import` by following imports
120120
3. Converts to `TypeReference::Resolved` after following imports
121121

122-
**Where**: Implemented in `js_module_info/module_resolver.rs`
122+
**Where**: The Salsa-backed implementation starts at
123+
`db/queries/type_inference.rs::infer_module_types` and uses helpers under
124+
`db/type_inference/`. `js_module_info/module_resolver.rs` contains the legacy
125+
`TypeResolver`-based path.
123126

124-
**Limitation**: Results cannot be cached (would become stale on file changes)
127+
**Caching**: `infer_module_types` is tracked by Salsa. Imported module results
128+
are dependencies, so Salsa invalidates affected importers after a change.
125129

126130
## Working with Type Resolvers
127131

@@ -287,7 +291,7 @@ let raw_data: &TypeData = resolved_data.as_raw_data();
287291
- **Resolver context**: Keep `ResolvedTypeData` when possible, don't extract raw `TypeData` early
288292
- **Performance**: Type vectors are fast - iterate directly instead of recursive traversal
289293
- **IDE focus**: All design decisions prioritize instant IDE updates over CLI performance
290-
- **No caching**: Full inference results can't be cached (would become stale)
294+
- **Caching**: Salsa memoizes full-inference query results and invalidates them through tracked module dependencies
291295
- **Globals**: Currently hardcoded, eventually should use TypeScript's `.d.ts` files
292296

293297
## Common Patterns

AGENTS.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,17 @@ cargo test
218218

219219
Read files in full before making wide-ranging changes, before editing files you have not already fully inspected, and when the user asks you to investigate or audit something. Do not rely only on search snippets for broad changes.
220220

221+
### 7. Comments and Doc Comments
222+
223+
These rules apply to **every** comment you write, including ones added incidentally while fixing a bug. Full guidance with examples: [`.claude/skills/doc-comments/SKILL.md`](./.claude/skills/doc-comments/SKILL.md).
224+
225+
- Write for a contributor reading the code at HEAD, months later, with no access to this conversation, the PR, or the diff.
226+
- Never narrate change history ("now", "previously", "no longer") and never address the reviewer ("this correctly handles..."). State how the code works, not how it came to be or why the change is right.
227+
- Deletion test: a comment must state something the reader cannot recover from the code. If names or types already carry it, don't write it.
228+
- `///` docs state the contract (behavior, invariants, panics); `//!` docs explain why the module exists and its terminology; `//` comments carry rationale only.
229+
- When your change alters documented behavior, extend or correct the existing prose — never replace specific docs with generic text.
230+
- Exception: rustdoc inside `declare_lint_rule!` is end-user documentation for the website; these rules don't apply there.
231+
221232
## Available Resources
222233

223234
### Skills (Procedural Knowledge)
@@ -226,6 +237,7 @@ Located in `.claude/skills/`, these provide step-by-step workflows:
226237

227238
- **biome-developer** - General development best practices and common gotchas
228239
- **changeset** - Creating and writing proper changesets
240+
- **doc-comments** - Writing comments and rustdoc addressed to developers
229241
- **eslint-migrate-options** - Implementing ESLint-to-Biome rule option migrators
230242
- **lint-rule-development** - Creating and testing lint rules
231243
- **formatter-development** - Implementing formatters
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/* should generate diagnostics */
2+
3+
type Prefix<T> = [T];
4+
5+
declare function consumeTuple(
6+
...args: [...Prefix<number>, () => void]
7+
): void;
8+
const tuplePrefix: Prefix<number> = [1];
9+
consumeTuple(...tuplePrefix, async () => {});
10+
11+
type Recursive<T> = [...Recursive<T>];
12+
13+
declare function consumeRecursive(
14+
...args: [...Recursive<number>, () => void]
15+
): void;
16+
consumeRecursive(async () => {});
17+
18+
interface InterfaceConstructor {
19+
new (callback: () => void): object;
20+
}
21+
declare const InterfaceConsumer: InterfaceConstructor;
22+
new InterfaceConsumer(async () => {});
23+
24+
declare const ObjectConsumer: {
25+
new (callback: () => void): object;
26+
};
27+
new ObjectConsumer(async () => {});
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: invalidComplexCallbacks.ts
4+
---
5+
# Input
6+
```ts
7+
/* should generate diagnostics */
8+
9+
type Prefix<T> = [T];
10+
11+
declare function consumeTuple(
12+
...args: [...Prefix<number>, () => void]
13+
): void;
14+
const tuplePrefix: Prefix<number> = [1];
15+
consumeTuple(...tuplePrefix, async () => {});
16+
17+
type Recursive<T> = [...Recursive<T>];
18+
19+
declare function consumeRecursive(
20+
...args: [...Recursive<number>, () => void]
21+
): void;
22+
consumeRecursive(async () => {});
23+
24+
interface InterfaceConstructor {
25+
new (callback: () => void): object;
26+
}
27+
declare const InterfaceConsumer: InterfaceConstructor;
28+
new InterfaceConsumer(async () => {});
29+
30+
declare const ObjectConsumer: {
31+
new (callback: () => void): object;
32+
};
33+
new ObjectConsumer(async () => {});
34+
35+
```
36+
37+
# Diagnostics
38+
```
39+
invalidComplexCallbacks.ts:9:30 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
40+
41+
i This function returns a Promise, but no return value was expected.
42+
43+
7 │ ): void;
44+
8 │ const tuplePrefix: Prefix<number> = [1];
45+
> 9 │ consumeTuple(...tuplePrefix, async () => {});
46+
│ ^^^^^^^^^^^^^^
47+
10 │
48+
11 │ type Recursive<T> = [...Recursive<T>];
49+
50+
i This may not have the desired result if you expect the Promise to be `await`-ed.
51+
52+
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.
53+
54+
55+
```
56+
57+
```
58+
invalidComplexCallbacks.ts:16:18 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
59+
60+
i This function returns a Promise, but no return value was expected.
61+
62+
14 │ ...args: [...Recursive<number>, () => void]
63+
15 │ ): void;
64+
> 16 │ consumeRecursive(async () => {});
65+
│ ^^^^^^^^^^^^^^
66+
17 │
67+
18 │ interface InterfaceConstructor {
68+
69+
i This may not have the desired result if you expect the Promise to be `await`-ed.
70+
71+
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.
72+
73+
74+
```
75+
76+
```
77+
invalidComplexCallbacks.ts:22:23 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
78+
79+
i This function returns a Promise, but no return value was expected.
80+
81+
20 │ }
82+
21 │ declare const InterfaceConsumer: InterfaceConstructor;
83+
> 22 │ new InterfaceConsumer(async () => {});
84+
│ ^^^^^^^^^^^^^^
85+
23 │
86+
24 │ declare const ObjectConsumer: {
87+
88+
i This may not have the desired result if you expect the Promise to be `await`-ed.
89+
90+
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.
91+
92+
93+
```
94+
95+
```
96+
invalidComplexCallbacks.ts:27:20 lint/nursery/noMisusedPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
97+
98+
i This function returns a Promise, but no return value was expected.
99+
100+
25 │ new (callback: () => void): object;
101+
26};
102+
> 27 │ new ObjectConsumer(async () => {});
103+
│ ^^^^^^^^^^^^^^
104+
28 │
105+
106+
i This may not have the desired result if you expect the Promise to be `await`-ed.
107+
108+
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.
109+
110+
111+
```

0 commit comments

Comments
 (0)