Skip to content

'Extract type' from property signature with erroneous initializer causes assertion failure #52751

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

Closed
DanielRosenwasser opened this issue Feb 14, 2023 · 4 comments · Fixed by #52803
Assignees
Labels
Bug A bug in TypeScript Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Help Wanted You can do this Old-Crawler-Detected Detected by an older crawler (aka "fuzzer") running random TSServer operations on public code.

Comments

@DanielRosenwasser
Copy link
Member

type Foo = [|{
    x: string = someString;
}|]

Request an "extract to type alias" refactoring on the specified [|range|]:

 Debug Failure. False expression: Token end is child end
Error: Debug Failure. False expression: Token end is child end
    at processChildNode (typescript/lib/tsserver.js:165873:17)
    at typescript/lib/tsserver.js:165804:9
    at visitNode2 (typescript/lib/tsserver.js:29284:18)
    at forEachChildInPropertySignature (typescript/lib/tsserver.js:29340:167)
    at forEachChild (typescript/lib/tsserver.js:29798:35)
    at processNode (typescript/lib/tsserver.js:165801:5)
    at processChildNode (typescript/lib/tsserver.js:165880:7)
    at processChildNodes (typescript/lib/tsserver.js:165923:32)
    at typescript/lib/tsserver.js:165817:9
    at visitNodes (typescript/lib/tsserver.js:29289:14)
    at forEachChildInTypeLiteral (typescript/lib/tsserver.js:29399:12)
    at forEachChild (typescript/lib/tsserver.js:29798:35)
    at processNode (typescript/lib/tsserver.js:165801:5)
    at processChildNode (typescript/lib/tsserver.js:165880:7)
    at typescript/lib/tsserver.js:165804:9
    at visitNode2 (typescript/lib/tsserver.js:29284:18)
    at forEachChildInTypeAliasDeclaration (typescript/lib/tsserver.js:29575:144)
    at forEachChild (typescript/lib/tsserver.js:29798:35)
    at processNode (typescript/lib/tsserver.js:165801:5)
    at formatSpanWorker (typescript/lib/tsserver.js:165609:5)
    at typescript/lib/tsserver.js:165549:105
    at getFormattingScanner (typescript/lib/tsserver.js:164365:15)
    at Object.formatNodeGivenIndentation (typescript/lib/tsserver.js:165549:10)
    at getFormattedTextOfNode (typescript/lib/tsserver.js:163762:43)
    at format (typescript/lib/tsserver.js:163742:27)
    at computeNewText (typescript/lib/tsserver.js:163743:219)
    at typescript/lib/tsserver.js:163704:25
    at mapDefined (typescript/lib/tsserver.js:2627:22)
    at typescript/lib/tsserver.js:163702:28
    at mapDefined (typescript/lib/tsserver.js:2627:22)
    at Object.getTextChangesFromChanges (typescript/lib/tsserver.js:163696:12)
    at ChangeTracker.getChanges (typescript/lib/tsserver.js:163585:35)
    at ChangeTracker.with (typescript/lib/tsserver.js:163083:20)
    at Object.getRefactorEditsToExtractType [as getEditsForAction] (typescript/lib/tsserver.js:156988:60)
    at Object.getEditsForRefactor (typescript/lib/tsserver.js:156469:31)
    at Object.getEditsForRefactor2 [as getEditsForRefactor] (typescript/lib/tsserver.js:135592:32)
    at IpcIOSession.getEditsForRefactor (typescript/lib/tsserver.js:178779:49)
    at getEditsForRefactor (typescript/lib/tsserver.js:177034:43)
@DanielRosenwasser DanielRosenwasser changed the title 'Extract type' from property signature causes assertion failure 'Extract type' from property signature with erroneous initializer causes assertion failure Feb 14, 2023
@DanielRosenwasser
Copy link
Member Author

As usual, this has to do with the visitor with visitEachChild not actually visiting the initializer property on PropertySignatures, even though forEachChild does.

And as usual, it's awkward because updatePropertySignature doesn't give control over it. It really should either drop the property signature entirely or provide a mechanism to update it. I'd probably lean towards the latter

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Help Wanted You can do this Old-Crawler-Detected Detected by an older crawler (aka "fuzzer") running random TSServer operations on public code. labels Feb 14, 2023
@DanielRosenwasser DanielRosenwasser added the Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". label Feb 14, 2023
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 5.0.1 milestone Feb 14, 2023
@rbuckton
Copy link
Contributor

initializer on a property signature is a grammar error and isn't part of a syntactically valid AST. Supporting it in visitEachChild would require adding it to the updatePropertySignature factory method, which I'm not a fan of. I may look into an alternate approach, as I'd eventually like to find a way to remove these grammar-error-only node properties from nodes to reduce memory overhead.

@rbuckton
Copy link
Contributor

I don't think the issue is with visitEachChild, since new positions are assigned by the printer via a TextChangesWriter.

@rbuckton
Copy link
Contributor

I don't think the issue is with visitEachChild, since new positions are assigned by the printer via a TextChangesWriter.

Ah, never mind. Both the emitter and visitEachChild need to be fixed for this to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Help Wanted You can do this Old-Crawler-Detected Detected by an older crawler (aka "fuzzer") running random TSServer operations on public code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants