also looking at this
fix(extractor): keep a protected constructor protected
#3125
2 files
+77
-2
| @@ -2723,12 +2723,13 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 2723 | 2723 | let isOverride = false |
| 2724 | 2724 | let isAccessor = false |
| 2725 | 2725 | let isAsync = false |
| 2726 | let protectedEnd = -1 | |
| 2726 | 2727 | |
| 2727 | 2728 | const modLoop = true |
| 2728 | 2729 | while (modLoop) { |
| 2729 | 2730 | skipWhitespaceAndComments() |
| 2730 | 2731 | if (matchWord('private')) { isPrivate = true; pos += 7; continue } |
| 2731 | if (matchWord('protected')) { isProtected = true; pos += 9; continue } | |
| 2732 | if (matchWord('protected')) { isProtected = true; pos += 9; protectedEnd = pos; continue } | |
| 2732 | 2733 | if (matchWord('public')) { pos += 6; continue } |
| 2733 | 2734 | if (matchWord('static')) { isStatic = true; pos += 6; continue } |
| 2734 | 2735 | if (matchWord('abstract')) { isAbstract = true; pos += 8; continue } |
| @@ -2769,6 +2770,9 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 2769 | 2770 | |
| 2770 | 2771 | // Detect member type |
| 2771 | 2772 | if (matchWord('constructor')) { |
| 2773 | // A `protected` with a line break after it is a field of that name | |
| 2774 | // (TypeScript reads a modifier only on the same line as what it modifies). | |
| 2775 | const isProtectedConstructor = isProtected && !/[\n\r\u2028\u2029]/.test(source.slice(protectedEnd, pos)) | |
| 2772 | 2776 | pos += 11 |
| 2773 | 2777 | skipWhitespaceAndComments() |
| 2774 | 2778 | const rawParams = extractParamList() |
| @@ -2785,8 +2789,10 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 2785 | 2789 | pos++ |
| 2786 | 2790 | } |
| 2787 | 2791 | |
| 2792 | // A protected constructor stays protected: public, it would let any | |
| 2793 | // caller construct a class only its subclasses may. | |
| 2788 | 2794 | const dtsParams = buildDtsParams(rawParams) |
| 2789 | members.push(` constructor${dtsParams};`) | |
| 2795 | members.push(` ${isProtectedConstructor ? 'protected ' : ''}constructor${dtsParams};`) | |
| 2790 | 2796 | } |
| 2791 | 2797 | else if (matchWord('get')) { |
| 2792 | 2798 | pos += 3 |
| @@ -0,0 +1,69 @@ | ||
| 1 | /** | |
| 2 | * A protected constructor was emitted as a public one: | |
| 3 | * | |
| 4 | * export class Singleton { | |
| 5 | * protected constructor(name: string) {} | |
| 6 | * static create(): Singleton { ... } | |
| 7 | * } | |
| 8 | * | |
| 9 | * emitted `constructor(name: string);`, so `new Singleton('x')` type-checked | |
| 10 | * against the declarations although only subclasses and the class itself may | |
| 11 | * construct it. | |
| 12 | */ | |
| 13 | import { describe, expect, it } from 'bun:test' | |
| 14 | import { mkdtemp, rm, writeFile } from 'node:fs/promises' | |
| 15 | import { tmpdir } from 'node:os' | |
| 16 | import { join } from 'node:path' | |
| 17 | import { runTsc } from './helpers/tsc' | |
| 18 | import { processCode } from './test-utils' | |
| 19 | ||
| 20 | /** Type-check `consumer` against the generated declarations as `./index`. */ | |
| 21 | async function checkConsumer(dts: string, consumer: string): Promise<string> { | |
| 22 | const dir = await mkdtemp(join(tmpdir(), 'dtsx-protected-constructor-')) | |
| 23 | try { | |
| 24 | await writeFile(join(dir, 'index.d.ts'), dts) | |
| 25 | await writeFile(join(dir, 'use.ts'), consumer) | |
| 26 | return runTsc(dir, ['index.d.ts', 'use.ts']).output | |
| 27 | } | |
| 28 | finally { | |
| 29 | await rm(dir, { recursive: true, force: true }) | |
| 30 | } | |
| 31 | } | |
| 32 | ||
| 33 | describe('protected constructor', () => { | |
| 34 | it.each([ | |
| 35 | [ | |
| 36 | 'plain', | |
| 37 | 'export class Singleton {\n protected constructor(name: string) {}\n static create(): Singleton { return new Singleton(\'x\') }\n}\n', | |
| 38 | 'protected constructor(name: string);', | |
| 39 | 'import { Singleton } from \'./index\'\nconst s: Singleton = Singleton.create()\nexport class Named extends Singleton {\n constructor() { super(\'named\') }\n}\nnew Named()\n// @ts-expect-error the constructor is protected\nnew Singleton(\'x\')\nexport {}\n', | |
| 40 | ], | |
| 41 | [ | |
| 42 | 'with a parameter property', | |
| 43 | 'export class Entity {\n protected constructor(public readonly id: number) {}\n}\n', | |
| 44 | 'protected constructor(id: number);', | |
| 45 | 'import { Entity } from \'./index\'\nexport class User extends Entity {\n constructor() { super(1) }\n}\nconst id: number = new User().id\n// @ts-expect-error the constructor is protected\nnew Entity(1)\nexport {}\n', | |
| 46 | ], | |
| 47 | [ | |
| 48 | 'overloads', | |
| 49 | 'export class Point {\n protected constructor(x: number, y: number)\n protected constructor(xy: [number, number])\n protected constructor(a: any, b?: any) {}\n}\n', | |
| 50 | 'protected constructor(x: number, y: number);', | |
| 51 | 'import { Point } from \'./index\'\nexport class Origin extends Point {\n constructor() { super(0, 0) }\n}\n// @ts-expect-error the constructor is protected\nnew Point(0, 0)\nexport {}\n', | |
| 52 | ], | |
| 53 | ])('stays protected: %s', async (_label, code, constructor, consumer) => { | |
| 54 | const output = processCode(code) | |
| 55 | expect(output).toContain(constructor) | |
| 56 | expect(await checkConsumer(output, consumer)).toBe('') | |
| 57 | }) | |
| 58 | ||
| 59 | it.each([ | |
| 60 | ['public modifier', 'export class C {\n public constructor(a: string) {}\n}\n'], | |
| 61 | ['a field named protected on the line before', 'export class C {\n protected\n constructor(a: string) { this.protected = a }\n}\n'], | |
| 62 | ['a line break inside a comment after protected', 'export class C {\n protected /*\n */ constructor(a: string) { this.protected = a }\n}\n'], | |
| 63 | ])('leaves a public constructor public: %s', async (_label, code) => { | |
| 64 | const output = processCode(code) | |
| 65 | expect(output).toContain(' constructor(a: string);') | |
| 66 | expect(output).not.toContain('protected constructor') | |
| 67 | expect(await checkConsumer(output, 'import { C } from \'./index\'\nnew C(\'x\')\nexport {}\n')).toBe('') | |
| 68 | }) | |
| 69 | }) | |