-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix crash when a call signature's type parameter cannot be reused #4846
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
Open
Nicolaev Eduard (nikeedw)
wants to merge
4
commits into
microsoft:main
Choose a base branch
from
nikeedw:fix-4748-nil-type-parameter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+269
−1
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b68e59c
Fix crash when a call signature's type parameter cannot be reused
nikeedw 620995b
Take the type parameter symbol straight off the declaration
nikeedw 854aac7
Update baseline for deduplicated declaration diagnostics
nikeedw a6e2822
Merge remote-tracking branch 'origin/main' into update-branch
nikeedw 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
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
226 changes: 226 additions & 0 deletions
226
...e/tsc/declarationEmit/dts-signature-update-with-a-type-parameter-that-cannot-be-reused.js
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,226 @@ | ||
| currentDirectory::/home/src/workspaces/project | ||
| useCaseSensitiveFileNames::true | ||
| Input:: | ||
| //// [/home/src/workspaces/project/a.ts] *new* | ||
| declare const brand: unique symbol; | ||
| const state = { name: "", count: 0, [brand]: true }; | ||
| export const api = { | ||
| setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => { | ||
| state[key] = value; | ||
| }, | ||
| }; | ||
| //// [/home/src/workspaces/project/b.ts] *new* | ||
| import { api } from "./a"; | ||
| export const merged = { ...api }; | ||
| //// [/home/src/workspaces/project/tsconfig.json] *new* | ||
| { | ||
| "compilerOptions": { | ||
| "strict": true, | ||
| "incremental": true, | ||
| "skipLibCheck": true, | ||
| "skipDefaultLibCheck": true, | ||
| }, | ||
| } | ||
|
|
||
| tsgo | ||
| ExitStatus:: Success | ||
| Output:: | ||
| //// [/home/src/tslibs/TS/Lib/lib.es2025.full.d.ts] *Lib* | ||
| /// <reference no-default-lib="true"/> | ||
| interface Boolean {} | ||
| interface Function {} | ||
| interface CallableFunction {} | ||
| interface NewableFunction {} | ||
| interface IArguments {} | ||
| interface Number { toExponential: any; } | ||
| interface Object {} | ||
| interface RegExp {} | ||
| interface String { charAt: any; } | ||
| interface Array<T> { length: number; [n: number]: T; } | ||
| interface ReadonlyArray<T> {} | ||
| interface SymbolConstructor { | ||
| (desc?: string | number): symbol; | ||
| for(name: string): symbol; | ||
| readonly toStringTag: symbol; | ||
| } | ||
| declare var Symbol: SymbolConstructor; | ||
| interface Symbol { | ||
| readonly [Symbol.toStringTag]: string; | ||
| } | ||
| declare const console: { log(msg: any): void; }; | ||
| //// [/home/src/workspaces/project/a.js] *new* | ||
| const state = { name: "", count: 0, [brand]: true }; | ||
| export const api = { | ||
| setField: (key, value) => { | ||
| state[key] = value; | ||
| }, | ||
| }; | ||
|
|
||
| //// [/home/src/workspaces/project/b.js] *new* | ||
| import { api } from "./a"; | ||
| export const merged = { ...api }; | ||
|
|
||
| //// [/home/src/workspaces/project/tsconfig.tsbuildinfo] *new* | ||
| {"version":"FakeTSVersion","root":[[2,3]],"fileNames":["lib.es2025.full.d.ts","./a.ts","./b.ts"],"fileInfos":[{"version":"8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };","affectsGlobalScope":true,"impliedNodeFormat":1},"420f0324953654bb26e57da1857dc6a2-declare const brand: unique symbol;\nconst state = { name: \"\", count: 0, [brand]: true };\nexport const api = {\n setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => {\n state[key] = value;\n },\n};","de62ab78d5ba64a5b325bfebf0f2e8d3-import { api } from \"./a\";\nexport const merged = { ...api };"],"fileIdsList":[[2]],"options":{"skipLibCheck":true,"strict":true,"skipDefaultLibCheck":true},"referencedMap":[[3,1]]} | ||
| //// [/home/src/workspaces/project/tsconfig.tsbuildinfo.readable.baseline.txt] *new* | ||
| { | ||
| "version": "FakeTSVersion", | ||
| "root": [ | ||
| { | ||
| "files": [ | ||
| "./a.ts", | ||
| "./b.ts" | ||
| ], | ||
| "original": [ | ||
| 2, | ||
| 3 | ||
| ] | ||
| } | ||
| ], | ||
| "fileNames": [ | ||
| "lib.es2025.full.d.ts", | ||
| "./a.ts", | ||
| "./b.ts" | ||
| ], | ||
| "fileInfos": [ | ||
| { | ||
| "fileName": "lib.es2025.full.d.ts", | ||
| "version": "8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };", | ||
| "signature": "8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };", | ||
| "affectsGlobalScope": true, | ||
| "impliedNodeFormat": "CommonJS", | ||
| "original": { | ||
| "version": "8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };", | ||
| "affectsGlobalScope": true, | ||
| "impliedNodeFormat": 1 | ||
| } | ||
| }, | ||
| { | ||
| "fileName": "./a.ts", | ||
| "version": "420f0324953654bb26e57da1857dc6a2-declare const brand: unique symbol;\nconst state = { name: \"\", count: 0, [brand]: true };\nexport const api = {\n setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => {\n state[key] = value;\n },\n};", | ||
| "signature": "420f0324953654bb26e57da1857dc6a2-declare const brand: unique symbol;\nconst state = { name: \"\", count: 0, [brand]: true };\nexport const api = {\n setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => {\n state[key] = value;\n },\n};", | ||
| "impliedNodeFormat": "CommonJS" | ||
| }, | ||
| { | ||
| "fileName": "./b.ts", | ||
| "version": "de62ab78d5ba64a5b325bfebf0f2e8d3-import { api } from \"./a\";\nexport const merged = { ...api };", | ||
| "signature": "de62ab78d5ba64a5b325bfebf0f2e8d3-import { api } from \"./a\";\nexport const merged = { ...api };", | ||
| "impliedNodeFormat": "CommonJS" | ||
| } | ||
| ], | ||
| "fileIdsList": [ | ||
| [ | ||
| "./a.ts" | ||
| ] | ||
| ], | ||
| "options": { | ||
| "skipLibCheck": true, | ||
| "strict": true, | ||
| "skipDefaultLibCheck": true | ||
| }, | ||
| "referencedMap": { | ||
| "./b.ts": [ | ||
| "./a.ts" | ||
| ] | ||
| }, | ||
| "size": 1378 | ||
| } | ||
|
|
||
| tsconfig.json:: | ||
| SemanticDiagnostics:: | ||
| *refresh* /home/src/tslibs/TS/Lib/lib.es2025.full.d.ts | ||
| *refresh* /home/src/workspaces/project/a.ts | ||
| *refresh* /home/src/workspaces/project/b.ts | ||
| Signatures:: | ||
|
|
||
|
|
||
| Edit [0]:: modify b.ts | ||
| //// [/home/src/workspaces/project/b.ts] *modified* | ||
| import { api } from "./a"; | ||
| export const merged = { ...api }; | ||
| export const touched = 1; | ||
|
|
||
| tsgo | ||
| ExitStatus:: Success | ||
| Output:: | ||
| //// [/home/src/workspaces/project/b.js] *modified* | ||
| import { api } from "./a"; | ||
| export const merged = { ...api }; | ||
| export const touched = 1; | ||
|
|
||
| //// [/home/src/workspaces/project/tsconfig.tsbuildinfo] *modified* | ||
| {"version":"FakeTSVersion","root":[[2,3]],"fileNames":["lib.es2025.full.d.ts","./a.ts","./b.ts"],"fileInfos":[{"version":"8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };","affectsGlobalScope":true,"impliedNodeFormat":1},"420f0324953654bb26e57da1857dc6a2-declare const brand: unique symbol;\nconst state = { name: \"\", count: 0, [brand]: true };\nexport const api = {\n setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => {\n state[key] = value;\n },\n};",{"version":"6fa50051a3b923ced64e589b1e168966-import { api } from \"./a\";\nexport const merged = { ...api };\nexport const touched = 1;","signature":"49d11e9dbd07ad5b91e829e73838e2a1-export declare const merged: {\n setField: <K extends \"count\" | \"name\" | unique symbol>(key: K, value: ({\n name: string;\n count: number;\n [brand]: boolean;\n })[K]) => void;\n};\nexport declare const touched = 1;\n\n(40,6): error2527: The_inferred_type_of_0_references_an_inaccessible_1_type_A_type_annotation_is_necessary_2527\nmerged\nunique symbol\n\n(40,6): error4023: Exported_variable_0_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named_4023\nmerged\nbrand\n\"/home/src/workspaces/project/a\"\n","impliedNodeFormat":1}],"fileIdsList":[[2]],"options":{"skipLibCheck":true,"strict":true,"skipDefaultLibCheck":true},"referencedMap":[[3,1]]} | ||
| //// [/home/src/workspaces/project/tsconfig.tsbuildinfo.readable.baseline.txt] *modified* | ||
| { | ||
| "version": "FakeTSVersion", | ||
| "root": [ | ||
| { | ||
| "files": [ | ||
| "./a.ts", | ||
| "./b.ts" | ||
| ], | ||
| "original": [ | ||
| 2, | ||
| 3 | ||
| ] | ||
| } | ||
| ], | ||
| "fileNames": [ | ||
| "lib.es2025.full.d.ts", | ||
| "./a.ts", | ||
| "./b.ts" | ||
| ], | ||
| "fileInfos": [ | ||
| { | ||
| "fileName": "lib.es2025.full.d.ts", | ||
| "version": "8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };", | ||
| "signature": "8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };", | ||
| "affectsGlobalScope": true, | ||
| "impliedNodeFormat": "CommonJS", | ||
| "original": { | ||
| "version": "8859c12c614ce56ba9a18e58384a198f-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ninterface SymbolConstructor {\n (desc?: string | number): symbol;\n for(name: string): symbol;\n readonly toStringTag: symbol;\n}\ndeclare var Symbol: SymbolConstructor;\ninterface Symbol {\n readonly [Symbol.toStringTag]: string;\n}\ndeclare const console: { log(msg: any): void; };", | ||
| "affectsGlobalScope": true, | ||
| "impliedNodeFormat": 1 | ||
| } | ||
| }, | ||
| { | ||
| "fileName": "./a.ts", | ||
| "version": "420f0324953654bb26e57da1857dc6a2-declare const brand: unique symbol;\nconst state = { name: \"\", count: 0, [brand]: true };\nexport const api = {\n setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => {\n state[key] = value;\n },\n};", | ||
| "signature": "420f0324953654bb26e57da1857dc6a2-declare const brand: unique symbol;\nconst state = { name: \"\", count: 0, [brand]: true };\nexport const api = {\n setField: <K extends keyof typeof state>(key: K, value: (typeof state)[K]): void => {\n state[key] = value;\n },\n};", | ||
| "impliedNodeFormat": "CommonJS" | ||
| }, | ||
| { | ||
| "fileName": "./b.ts", | ||
| "version": "6fa50051a3b923ced64e589b1e168966-import { api } from \"./a\";\nexport const merged = { ...api };\nexport const touched = 1;", | ||
| "signature": "49d11e9dbd07ad5b91e829e73838e2a1-export declare const merged: {\n setField: <K extends \"count\" | \"name\" | unique symbol>(key: K, value: ({\n name: string;\n count: number;\n [brand]: boolean;\n })[K]) => void;\n};\nexport declare const touched = 1;\n\n(40,6): error2527: The_inferred_type_of_0_references_an_inaccessible_1_type_A_type_annotation_is_necessary_2527\nmerged\nunique symbol\n\n(40,6): error4023: Exported_variable_0_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named_4023\nmerged\nbrand\n\"/home/src/workspaces/project/a\"\n", | ||
| "impliedNodeFormat": "CommonJS", | ||
| "original": { | ||
| "version": "6fa50051a3b923ced64e589b1e168966-import { api } from \"./a\";\nexport const merged = { ...api };\nexport const touched = 1;", | ||
| "signature": "49d11e9dbd07ad5b91e829e73838e2a1-export declare const merged: {\n setField: <K extends \"count\" | \"name\" | unique symbol>(key: K, value: ({\n name: string;\n count: number;\n [brand]: boolean;\n })[K]) => void;\n};\nexport declare const touched = 1;\n\n(40,6): error2527: The_inferred_type_of_0_references_an_inaccessible_1_type_A_type_annotation_is_necessary_2527\nmerged\nunique symbol\n\n(40,6): error4023: Exported_variable_0_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named_4023\nmerged\nbrand\n\"/home/src/workspaces/project/a\"\n", | ||
| "impliedNodeFormat": 1 | ||
| } | ||
| } | ||
| ], | ||
| "fileIdsList": [ | ||
| [ | ||
| "./a.ts" | ||
| ] | ||
| ], | ||
| "options": { | ||
| "skipLibCheck": true, | ||
| "strict": true, | ||
| "skipDefaultLibCheck": true | ||
| }, | ||
| "referencedMap": { | ||
| "./b.ts": [ | ||
| "./a.ts" | ||
| ] | ||
| }, | ||
| "size": 2037 | ||
| } | ||
|
|
||
| tsconfig.json:: | ||
| SemanticDiagnostics:: | ||
| *refresh* /home/src/workspaces/project/b.ts | ||
| Signatures:: | ||
| (computed .d.ts) /home/src/workspaces/project/b.ts |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a
compilertest intestdata\tests\cases\compilerand not a full stack test, since it's not concerned with incremental/watch/CLI stuff.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that first and verified against the unpatched code — as a compiler test this repro is green, because it never reaches the failing path. The shape necessarily carries declaration diagnostics (TS2527/TS4023), and declaration emit skips printing a file that has them:
The malformed list is still built — the diagnostics come from that same transform — but only
EmitOnlyForcedDtsprints in spite of them, and its sole production caller is the incremental d.ts signature computation inaffectedfileshandler.go. The compiler runner sets neitherforceEmitnoremitOnly, so the same two files undercases/compilerpass with and without the fix, while this scenario panics without it.Agreed the defect itself is in the node builder/printer, not in anything incremental — incremental is just the only path that prints the result. If there's a diagnostics-free shape that makes type-parameter reuse fail (I went looking via the silent
markError(nil)paths in the reuse visitor and couldn't build one), I'll gladly move this to a compiler test; otherwise I'd prefer to keep the incremental regression test, since it's the only harness that reaches the crash.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. Unfortunate. I'd have suggested a fourslash regression test as a fallback, but our current
fourslashAPIs are a bit lacking compared to the old ones and don't have LS emit functions exposed to the test runner right now (eg,baselineGetEmitOutput/getEmitOutput).I'm just not a fun of stuffing regression tests into these unit test files, but I guess this'll do for now.