Extract to typedef from (invalid) type with comments in JS file causes assertion failure

[DanielRosenwasser]

// @filename: foo.js
type Foo = [|{
    /**
     * 
     */
    oops: string;
}|];

// @filename: bar.js
type Bar = [|string | /* oops */ boolean|];

Request "extract to typedef" on each[|region|].

Response received: getEditsForRefactor (194). Request took 2 ms. Success: false . Message: Error processing request. Debug Failure. False expression: Node must have a real position for this operation
Error: Debug Failure. False expression: Node must have a real position for this operation
    at NodeObject.assertHasRealPosition ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:162180:22)
    at NodeObject.getStart ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:162186:18)
    at processChildNode ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:145804:47)
    at processChildNodes ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:145905:48)
    at [NODE_MODULES_DIR]/typescript/lib/tsserver.js:145793:21
    at visitNodes ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:30193:24)
    at Object.forEachChild ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:30677:24)
    at processNode ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:145790:20)
    at formatSpanWorker ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:145590:17)
    at [NODE_MODULES_DIR]/typescript/lib/tsserver.js:145553:140
    at Object.getFormattingScanner ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:144180:23)
    at Object.formatNodeGivenIndentation ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:145553:31)
    at getFormattedTextOfNode ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:147971:45)
    at format ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:147942:52)
    at computeNewText ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:147945:23)
    at [NODE_MODULES_DIR]/typescript/lib/tsserver.js:147909:39
    at Object.mapDefined ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:605:30)
    at [NODE_MODULES_DIR]/typescript/lib/tsserver.js:147907:42
    at Object.mapDefined ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:605:30)
    at Object.getTextChangesFromChanges ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:147893:27)
    at ChangeTracker.getChanges ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:147832:45)
    at Function.ChangeTracker.with ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:147252:32)
    at Object.getRefactorEditsToExtractType [as getEditsForAction] ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:159993:62)
    at Object.getEditsForRefactor ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:148557:41)
    at Object.getEditsForRefactor ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:164314:32)
    at IpcIOSession.Session.getEditsForRefactor ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:175446:59)
    at Session.handlers.ts.Map.ts.getEntries._a.<computed> ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:174066:61)
    at [NODE_MODULES_DIR]/typescript/lib/tsserver.js:175829:88
    at IpcIOSession.Session.executeWithRequestId ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:175820:28)
    at IpcIOSession.Session.executeCommand ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:175829:33)
    at IpcIOSession.Session.onMessage ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:175855:35)
    at process.<anonymous> ([NODE_MODULES_DIR]/typescript/lib/tsserver.js:178490:31)
    at process.emit (node:events:390:28)
    at emit (node:internal/child_process:917:12)
    at processTicksAndRejections (node:internal/process/task_queues:84:21)

[rbuckton]

I was encountering a similar error in #48112 due to new synthetic nodes being added as a result of processing text changes. I fixed the case I ran into using this: https://github.com/microsoft/TypeScript/pull/48112/files?show-viewed-files=true&file-filters%5B%5D=#diff-386d93db50a938da0c4af7bb293ac679b5da235cebeeee9adbc040fbf8aa222cR1128-R1136

I'll have to check whether the issues are related.

[DanielRosenwasser]

That link seems to have issues on my end - can you hard-link to the line range of the file?

[rbuckton]

TypeScript/src/services/textChanges.ts

Lines 1128 to 1136 in 01b9a2d

	const textChangesTransformationContext: TransformationContext = {
	...nullTransformationContext,
	factory: createNodeFactory(
	nullTransformationContext.factory.flags | NodeFactoryFlags.NoParenthesizerRules,
	nullTransformationContext.factory.baseFactory),
	};
	export function assignPositionsToNode(node: Node): Node {
	const visited = visitEachChild(node, assignPositionsToNode, textChangesTransformationContext, assignPositionsToNodeArray, assignPositionsToNode);

[rbuckton]

The problem is that the nullTransformationContext uses a default NodeFactory that does auto-parenthesization, and our auto-parenthesization for types is fairly aggressive, so we sometimes introduce a new parenthesized type when visiting on 1136. Since this visitor is only supposed to create a proxy for existing nodes for the purpose of setting positions, its not designed to handle net-new nodes.

[DanielRosenwasser]

Is it a parenthesization issue? It seems more like it's specific to putting a comment inside of a comment. The fix that @MQuy has at #48545 of just dropping the comments fixes the specific tests at a shallow level, but would fail any deeper.

Is there a way to turn off comment trivia emit for an entire subtree? That would probably be an ideal fix, but admittedly I'm not familiar with the transform pipeline anymore.

[rbuckton]

Is it a parenthesization issue? It seems more like it's specific to putting a comment inside of a comment. The fix that @MQuy has at #48545 of just dropping the comments fixes the specific tests at a shallow level, but would fail any deeper.

I thought it may have been due to a ParenthesizedType node being silently added to the type, but that does not seem to be the case here.

Is there a way to turn off comment trivia emit for an entire subtree? That would probably be an ideal fix, but admittedly I'm not familiar with the transform pipeline anymore.

Yes, you can set EmitFlags.NoComments | EmitFlags.NoNestedComments to disable comments for the node and its subtree.

[DanielRosenwasser]

Thank you @MQuy!

[DanielRosenwasser]

Looks like there's still an issue when triggering the command from inside of a comment (#48593).