ReviewOS
Reload

also looking at this

stacks/dtsx

fix(inference): leave defaults out of annotated destructured arrow parameters

#3127
Open mkantautas wants to merge fix/arrow-destructuring-defaults into main
2 files +248 -2
packages/dtsx/src/processor/type-inference.tsmodified+150-2
@@ -1580,7 +1580,12 @@ export function cleanParameterDefaults(params: string): string {
15801580 }
15811581 }
15821582 }
1583 if (!hasRealEqual) return stripped
1583 if (!hasRealEqual) {
1584 // No parameter has a default of its own, but a destructuring pattern may
1585 // hold some; those are taken out in place.
1586 const cleanedInner = stripPatternDefaultsInList(inner)
1587 return cleanedInner === inner ? stripped : (hadParens ? `(${cleanedInner})` : cleanedInner)
1588 }
15841589
15851590 // Split parameters by comma at depth 0, preserving whitespace around commas
15861591 const paramParts: string[] = []
@@ -1728,7 +1733,7 @@ function cleanSingleParam(param: string): string {
17281733
17291734 if (colonIdx !== -1 && (equalIdx === -1 || colonIdx < equalIdx)) {
17301735 // Has type annotation: name[?]: type [= default]
1731 const name = param.slice(0, colonIdx).trim()
1736 const name = stripPatternDefaults(param.slice(0, colonIdx).trim())
17321737 const type = equalIdx !== -1
17331738 ? param.slice(colonIdx + 1, equalIdx).trim()
17341739 : param.slice(colonIdx + 1).trim()
@@ -1756,6 +1761,149 @@ function cleanSingleParam(param: string): string {
17561761 return param
17571762}
17581763
1764/**
1765 * `params` with the defaults taken out of each annotated destructuring pattern
1766 * (`{ a = 1 }: T, b: U` is `{ a }: T, b: U`) and every other character as
1767 * written. A list holding a `/` (a comment or a regex, whose text could pass
1768 * for parameters) or a template literal is returned as it is.
1769 */
1770function stripPatternDefaultsInList(params: string): string {
1771 if (params.includes('/') || params.includes('`'))
1772 return params
1773 let result = ''
1774 let copied = 0
1775 let depth = 0
1776 let patternStart = -1
1777 let atParamStart = true
1778 for (let i = 0; i < params.length; i++) {
1779 const ch = params.charCodeAt(i)
1780 if (ch === 34 || ch === 39) {
1781 i = skipQuotedValue(params, i, ch) - 1
1782 atParamStart = false
1783 continue
1784 }
1785 if (atParamStart && ch > 32) {
1786 patternStart = depth === 0 && (ch === 91 || ch === 123) ? i : -1
1787 atParamStart = false
1788 }
1789 if (ch === 40 || ch === 60 || ch === 91 || ch === 123) {
1790 depth++
1791 }
1792 else if (ch === 41 || ch === 93 || ch === 125 || (ch === 62 && params.charCodeAt(i - 1) !== 61)) {
1793 if (--depth === 0 && patternStart !== -1 && (ch === 93 || ch === 125)) {
1794 const pattern = params.slice(patternStart, i + 1)
1795 const cleaned = stripPatternDefaults(pattern)
1796 if (cleaned !== pattern && params.slice(i + 1).trimStart().startsWith(':')) {
1797 result += params.slice(copied, patternStart) + cleaned
1798 copied = i + 1
1799 }
1800 patternStart = -1
1801 }
1802 }
1803 else if (ch === 44 && depth === 0) {
1804 atParamStart = true
1805 }
1806 }
1807 return copied === 0 ? params : result + params.slice(copied)
1808}
1809
1810/**
1811 * A parameter's destructuring pattern without its default values, which a
1812 * declaration may not have (TS2371): `{ a = 1, b: [c = 'x'] = [] }` is
1813 * `{ a, b: [c] }`. Only a pattern of names, string and number keys, nested
1814 * patterns and rest elements, whose defaults are each a name, a number, a
1815 * string, `[]` or `{}`, is rewritten. Any other text is returned as it is.
1816 */
1817function stripPatternDefaults(name: string): string {
1818 let i = 0
1819 let out = ''
1820 const space = (): string => {
1821 const start = i
1822 while (i < name.length && name.charCodeAt(i) <= 32) i++
1823 return name.slice(start, i)
1824 }
1825 const word = (dots: boolean): string => {
1826 const start = i
1827 while (i < name.length && (isWordChar(name.charCodeAt(i)) || (dots && name[i] === '.'))) i++
1828 return name.slice(start, i)
1829 }
1830 const quoted = (): string => {
1831 const quote = name.charCodeAt(i)
1832 const end = skipQuotedValue(name, i, quote)
1833 if (end > name.length || end - 1 === i || name.charCodeAt(end - 1) !== quote) return ''
1834 const text = name.slice(i, end)
1835 i = end
1836 return text
1837 }
1838 const literal = (): boolean => {
1839 if (name[i] === '"' || name[i] === '\'') return quoted() !== ''
1840 if (name.startsWith('[]', i) || name.startsWith('{}', i)) { i += 2; return true }
1841 if (name[i] === '-') i++
1842 return word(true) !== ''
1843 }
1844 const target = (): boolean => {
1845 if (name[i] === '{' || name[i] === '[') return pattern()
1846 const id = word(false)
1847 out += id
1848 return isIdentifierName(id)
1849 }
1850 const element = (inObject: boolean): boolean => {
1851 if (name.startsWith('...', i)) {
1852 out += '...'
1853 i += 3
1854 out += space()
1855 return target()
1856 }
1857 if (inObject) {
1858 const key = name[i] === '"' || name[i] === '\'' ? quoted() : word(false)
1859 if (!key) return false
1860 out += key
1861 const afterKey = i
1862 const gap = space()
1863 if (name[i] === ':') {
1864 out += `${gap}:`
1865 i++
1866 out += space()
1867 if (!target()) return false
1868 }
1869 else {
1870 i = afterKey
1871 if (!isIdentifierName(key)) return false
1872 }
1873 }
1874 else if (!target()) {
1875 return false
1876 }
1877 const afterTarget = i
1878 space()
1879 if (name[i] === '=' && name[i + 1] !== '=' && name[i + 1] !== '>') {
1880 i++
1881 space()
1882 return literal()
1883 }
1884 i = afterTarget
1885 return true
1886 }
1887 const pattern = (): boolean => {
1888 const open = name[i]
1889 const close = open === '{' ? '}' : ']'
1890 out += open
1891 i++
1892 for (;;) {
1893 out += space()
1894 if (name[i] === close) { out += close; i++; return true }
1895 if (open === '[' && name[i] === ',') { out += ','; i++; continue } // a hole
1896 if (!element(open === '{')) return false
1897 out += space()
1898 if (name[i] === ',') { out += ','; i++; continue }
1899 if (name[i] === close) { out += close; i++; return true }
1900 return false
1901 }
1902 }
1903 if (name[0] !== '{' && name[0] !== '[') return name
1904 return pattern() && i === name.length ? out : name
1905}
1906
17591907/**
17601908 * Find the index after any leading block comments and whitespace in a key.
17611909 * Used to locate the actual identifier so modifiers like `async` can be
packages/dtsx/test/arrow-destructuring-defaults.test.tsadded+98-0
@@ -0,0 +1,98 @@
1/**
2 * An arrow function's destructured parameter kept its default values:
3 *
4 * export const glob = ({ cwd, gitignore = true }: GlobOptions) => ...
5 *
6 * emitted `({ cwd, gitignore = true }: GlobOptions) => ...`, and TypeScript
7 * rejects an initializer in a declaration (TS2371). knip's `util/glob` came out
8 * this way. A function declaration's parameters already lost them.
9 */
10import { describe, expect, it } from 'bun:test'
11import { mkdtemp, rm, writeFile } from 'node:fs/promises'
12import { tmpdir } from 'node:os'
13import { join } from 'node:path'
14import { runTsc } from './helpers/tsc'
15import { processCode } from './test-utils'
16
17/** Type-check `consumer` against the generated declarations as `./index`. */
18async function checkConsumer(dts: string, consumer: string): Promise<string> {
19 const dir = await mkdtemp(join(tmpdir(), 'dtsx-pattern-defaults-'))
20 try {
21 await writeFile(join(dir, 'index.d.ts'), dts)
22 await writeFile(join(dir, 'use.ts'), consumer)
23 return runTsc(dir, ['index.d.ts', 'use.ts']).output
24 }
25 finally {
26 await rm(dir, { recursive: true, force: true })
27 }
28}
29
30describe('destructured arrow parameters with defaults', () => {
31 it.each([
32 [
33 'object pattern',
34 'export interface GlobOptions { cwd: string, dir?: string, gitignore?: boolean }\nexport const glob = ({ cwd, dir = cwd, gitignore = true }: GlobOptions): string => dir + gitignore\n',
35 '({ cwd, dir, gitignore }: GlobOptions) => string',
36 'import { glob } from \'./index\'\nconst s: string = glob({ cwd: \'.\' })\nexport {}\n',
37 ],
38 [
39 'array pattern',
40 'export const first = ([x = 0, y]: [number?, number?]): number => x\n',
41 '([x, y]: [number?, number?]) => number',
42 'import { first } from \'./index\'\nconst n: number = first([])\nexport {}\n',
43 ],
44 [
45 'nested pattern with its own default',
46 'export const pick = ({ o: { p = \'z\' } = {} }: { o?: { p?: string } }): string => p\n',
47 '({ o: { p } }: { o?: { p?: string } }) => string',
48 'import { pick } from \'./index\'\nconst s: string = pick({})\nexport {}\n',
49 ],
50 [
51 'pattern with a parameter default',
52 'export const add = (n = 5, { m = 2 }: { m?: number } = {}): number => n + m\n',
53 '(n?: number, { m }?: { m?: number }) => number',
54 'import { add } from \'./index\'\nconst a: number = add()\nconst b: number = add(1, { m: 3 })\nexport {}\n',
55 ],
56 [
57 'defaults that are names, numbers, strings and empty literals, and a rest element',
58 'export interface Opts { a?: number, b?: number, c?: string, d?: string, e?: string[], f?: object, g?: boolean }\nconst base = { c: \'x\' }\nexport const opts = ({ a = 1.5, b = -2, c = base.c, d = \'q\', e = [], f = {}, ...rest }: Opts): number => a + b\n',
59 '({ a, b, c, d, e, f, ...rest }: Opts) => number',
60 'import { opts } from \'./index\'\nconst n: number = opts({})\nexport {}\n',
61 ],
62 [
63 'two patterns around a generic type',
64 'export const f = ({ a = 1 }: { a?: number }, b: Map<string, { c: number }>, { d = 2 }: { d?: number }): number => a\n',
65 '({ a }: { a?: number }, b: Map<string, { c: number }>, { d }: { d?: number }) => number',
66 'import { f } from \'./index\'\nconst n: number = f({}, new Map(), {})\nexport {}\n',
67 ],
68 ])('leaves the defaults out: %s', async (_label, code, signature, consumer) => {
69 const output = processCode(code)
70 expect(output).toContain(signature)
71 expect(await checkConsumer(output, consumer)).toBe('')
72 })
73
74 it.each([
75 ['a line comment holding `=`', 'export const f = ({ x, // seconds = ms / 1000\n y }: { x: number, y: number }): number => 1\n', '// seconds = ms / 1000\n y }'],
76 ['a default with type arguments', 'export const f = ({ map = new Map<string, number>() }: { map?: Map<string, number> }): number => 1\n', '{ map = new Map<string, number>() }'],
77 ['a computed key with an assignment', 'let key: string | undefined\nexport const f = ({ [key ??= \'x\']: x }: Record<string, number>): number => x\n', '[key ??= \'x\']: x'],
78 ['a default with operators, a string holding `=` and an arrow', 'export const f = ({ s = \'x=y\', eq = 1 === 1, cb = (q: number) => q }: { s?: string, eq?: boolean, cb?: (q: number) => number }): string => s\n', '{ s = \'x=y\', eq = 1 === 1, cb = (q: number) => q }'],
79 ['comparisons in two patterns', 'type B = { b?: boolean }\nexport const f = ({ a = 1 < 2 }, { b = 2 > 1 }: B): boolean => !!a && !!b\n', '({ a = 1 < 2 }, { b = 2 > 1 }: B)'],
80 ['a computed key with a default', 'let key: \'a\' | \'b\' = \'a\'\nexport const f = ({ [key = \'b\']: value }: { a: string, b: number }, n = 1): typeof value => value\n', '{ [key = \'b\']: value }'],
81 ['a regex default holding a comma', 'export const f = ({ re = /a,b/ }: { re?: RegExp }): number => 1\n', '{ re = /a,b/ }'],
82 ['a regex default holding `,b=`', 'export const f = ({ re = /a,b=c/ }: { re?: RegExp }): boolean => re.test(\'a\')\n', '{ re = /a,b=c/ }'],
83 ['a default holding a block comment', 'export const f = ({ n = 1 /* a,b=c */ }: { n?: number }): number => n\n', '{ n = 1 /* a,b=c */ }'],
84 ['a default holding a nested template literal', 'export const f = ({ t = `${`a,b=c`}` }: { t?: string }): string => t\n', '{ t = `${`a,b=c`}` }'],
85 ['a pattern without a type, whose defaults give its type', 'export const f = ({ verbose = false }): boolean => verbose\n', '{ verbose = false }'],
86 ['a parameter beside it with a block comment', 'export const f = ({ a = 1 }: { a?: number }, /* note */ b: string): number => a\n', '({ a = 1 }: { a?: number }, /* note */ b: string)'],
87 ['a regex that looks like another parameter', 'export const f = ({ re = /}, { b = 2 }:B/ }: { re?: RegExp }): number => 1\n', '{ re = /}, { b = 2 }:B/ }'],
88 ['a parameter beside it with a nested template literal type', 'export const f = ({ x = 1 }: { x?: number }, query: `?${`id=${number}`}`): number => x\n', '({ x = 1 }: { x?: number }, query: `?${`id=${number}`}`)'],
89 ['a parameter beside it with `//` in a comment', 'export const f = ({ x = 1 }: { x?: number }, /** See https://example.com */ y: number): number => x + y\n', '/** See https://example.com */ y: number'],
90 ])('leaves a pattern it cannot cut cleanly as written: %s', (_label, code, kept) => {
91 expect(processCode(code)).toContain(kept)
92 })
93
94 it('keeps an unannotated pattern beside it as written', () => {
95 expect(processCode('export const f = ({ op = \'<\' }: { op?: string }, { valid = 1 > 0, verbose = false }): string => op\n'))
96 .toContain('({ op }: { op?: string }, { valid = 1 > 0, verbose = false }) => string')
97 })
98})