ReviewOS

also looking at this

stacks/dtsx

fix(extractor): remove the implementation signature, not an overload

#3120
Open mkantautas wants to merge fix/overload-removal-identity into main
2 files +95 -10
packages/dtsx/src/extractor/scanner.tsmodified+15-10
Changes to packages/dtsx/src/extractor/scanner.ts
@@ -126,7 +126,9 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme
126126 const len = source.length
127127 const declarations: Declaration[] = []
128128 const nonExportedTypes = new Map<string, Declaration>()
129 const funcBodyIndices = new Set<number>()
129 // Functions read with a body: the implementation signature of an overload set.
130 // Held by identity, since declarations are inserted before them later.
131 const funcBodies = new Set<Declaration>()
130132 let pos = 0
131133
132134 // Skip BOM (byte order mark)
@@ -2211,12 +2213,7 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme
22112213 const text = `${exportPrefix}${declarationName}${generics}${dtsParams}: ${returnType};`
22122214 const comments = extractLeadingComments(declStart)
22132215
2214 // Record index if this function had a body (implementation signature for overloads)
2215 if (hasBody) {
2216 funcBodyIndices.add(declarations.length)
2217 }
2218
2219 return {
2216 const decl: Declaration = {
22202217 kind: 'function',
22212218 name: name || 'default',
22222219 text,
@@ -2230,6 +2227,9 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme
22302227 start: declStart,
22312228 end: pos,
22322229 }
2230 if (hasBody)
2231 funcBodies.add(decl)
2232 return decl
22332233 }
22342234
22352235 /**
@@ -4057,11 +4057,16 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme
40574057 // Post-process: remove implementation signatures of overloaded functions.
40584058 // For any function name that appears more than once, remove the last declaration
40594059 // that has a body (the implementation signature).
4060 if (funcBodyIndices.size > 0) {
4060 if (funcBodies.size > 0) {
4061 // A default export and a named export of the same name are separate
4062 // functions (`export function make(...)` beside `export default function
4063 // make(...) {}`), so each is its own overload set.
4064 const overloadKey = (decl: Declaration): string => decl.isDefault ? `default ${decl.name}` : decl.name
40614065 const funcNameCounts = new Map<string, number>()
40624066 for (const decl of declarations) {
40634067 if (decl.kind === 'function') {
4064 funcNameCounts.set(decl.name, (funcNameCounts.get(decl.name) || 0) + 1)
4068 const key = overloadKey(decl)
4069 funcNameCounts.set(key, (funcNameCounts.get(key) || 0) + 1)
40654070 }
40664071 }
40674072 // Only remove implementation signatures for overloaded functions (count > 1)
@@ -4076,7 +4081,7 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme
40764081 for (const name of overloadedNames) {
40774082 // Walk backwards to find the last declaration with a body for this name
40784083 for (let i = declarations.length - 1; i >= 0; i--) {
4079 if (declarations[i].kind === 'function' && declarations[i].name === name && funcBodyIndices.has(i)) {
4084 if (declarations[i].kind === 'function' && overloadKey(declarations[i]) === name && funcBodies.has(declarations[i])) {
40804085 toRemove.add(i)
40814086 break
40824087 }
packages/dtsx/test/overload-implementation.test.tsadded+80-0
Changes to packages/dtsx/test/overload-implementation.test.ts
@@ -0,0 +1,80 @@
1/**
2 * An overloaded function lost a public overload and kept its implementation
3 * signature when a local type was pulled into the declaration file:
4 *
5 * interface I { a: number }
6 * export function f(x: string): string
7 * export function f(x: number): number
8 * export function f(x: unknown): unknown { return x }
9 * export const value: I = { a: 1 }
10 *
11 * emitted `f(x: string)` and `f(x: unknown): unknown`, so `f(1)` became
12 * `unknown` and `f(true)` type-checked. The implementation was remembered by
13 * its index in `declarations`, and declaring `I` shifted every index after it,
14 * so the removal hit the overload before the implementation.
15 */
16import { describe, expect, it } from 'bun:test'
17import { mkdtemp, rm, writeFile } from 'node:fs/promises'
18import { tmpdir } from 'node:os'
19import { join } from 'node:path'
20import { runTsc } from './helpers/tsc'
21import { processCode } from './test-utils'
22
23/** Type-check `consumer` against the generated declarations as `./index`. */
24async function checkConsumer(dts: string, consumer: string): Promise<string> {
25 const dir = await mkdtemp(join(tmpdir(), 'dtsx-overloads-'))
26 try {
27 await writeFile(join(dir, 'index.d.ts'), dts)
28 await writeFile(join(dir, 'use.ts'), consumer)
29 return runTsc(dir, ['index.d.ts', 'use.ts']).output
30 }
31 finally {
32 await rm(dir, { recursive: true, force: true })
33 }
34}
35
36describe('overloads alongside a declared local type', () => {
37 it.each([
38 [
39 'local interface before the overloads',
40 'interface I { a: number }\nexport function f(x: string): string\nexport function f(x: number): number\nexport function f(x: unknown): unknown { return x }\nexport const value: I = { a: 1 }\n',
41 ['f(x: string): string;', 'f(x: number): number;'],
42 'f(x: unknown)',
43 'import { f } from \'./index\'\nconst s: string = f(\'a\')\nconst n: number = f(1)\n// @ts-expect-error no public overload takes a boolean\nf(true)\nexport {}\n',
44 ],
45 [
46 'default export',
47 'interface Opts { a: number }\nexport default function make(x: string): Opts\nexport default function make(x: number): number\nexport default function make(x: any): any { return x }\n',
48 ['make(x: string): Opts;', 'make(x: number): number;'],
49 'make(x: any)',
50 'import make from \'./index\'\nconst o: { a: number } = make(\'a\')\nconst n: number = make(1)\n// @ts-expect-error no public overload takes a boolean\nmake(true)\nexport {}\n',
51 ],
52 [
53 'two overloaded functions',
54 'interface A { a: number }\ninterface B { b: number }\nexport function f(x: string): A\nexport function f(x: number): number\nexport function f(x: any): any { return x }\nexport function g(x: string): B\nexport function g(x: number): number\nexport function g(x: any): any { return x }\n',
55 ['f(x: string): A;', 'f(x: number): number;', 'g(x: string): B;', 'g(x: number): number;'],
56 '(x: any)',
57 'import { f, g } from \'./index\'\nconst a: { a: number } = f(\'a\')\nconst b: { b: number } = g(\'b\')\n// @ts-expect-error no public overload takes a boolean\nf(true)\n// @ts-expect-error no public overload takes a boolean\ng(true)\nexport {}\n',
58 ],
59 ])('keeps the public overloads: %s', async (_label, code, overloads, implementation, consumer) => {
60 const output = processCode(code)
61 for (const overload of overloads)
62 expect(output).toContain(overload)
63 expect(output).not.toContain(implementation)
64 expect(await checkConsumer(output, consumer)).toBe('')
65 })
66})
67
68describe('a named and a default export of the same name', () => {
69 it.each([
70 ['named signature, default implementation', 'interface Input { value: number }\ninterface Output { result: number }\nexport function make(input: Input): Output\nexport default function make(input: Input): Output { return { result: input.value } }\n'],
71 ['default signature, named implementation', 'interface Input { value: number }\ninterface Output { result: number }\nexport default function make(input: Input): Output\nexport function make(input: Input): Output { return { result: input.value } }\n'],
72 ['no local types', 'export function make(input: { value: number }): { result: number }\nexport default function make(input: { value: number }): { result: number } { return { result: input.value } }\n'],
73 ])('keeps both: %s', async (_label, code) => {
74 const output = processCode(code)
75 expect(output).toContain('export declare function make(')
76 expect(output).toContain('export default function make(')
77 const consumer = 'import make, { make as named } from \'./index\'\nconst a: number = make({ value: 1 }).result\nconst b: number = named({ value: 1 }).result\nexport {}\n'
78 expect(await checkConsumer(output, consumer)).toBe('')
79 })
80})