Fix panic runtime error on ast.KindTupleTypes

[hamidrezahanafi]

Fixes #1501
Fixes #1408

Root cause from my debugging?
The declaration transformer is processing AST nodes that originated from @types/react/index.d.ts (the large file), but it's trying to use those node positions with a completely different source file (types.ts) as the context.
This is what I suspected - cross-file node processing during incremental compilation.
The incremental build system mixes nodes from different files, but the transformer was always using tx.state.currentSourceFile for position calculations instead of the node's actual source file.

What's the fix?
When handling TupleType nodes, we now compute line/character information against the node’s original parse location by using original := EmitContext().MostOriginal(input) and then the originalSourceFile derived from that node. If the original source file is available and the tuple spans a single line there, we set EFSingleLine. This ensures formatting and position calculations are based on the correct file, regardless of whether the node originated in the current file or a dependency, eliminating mis-formatting and position errors during declaration emit.

[jakebailey]

Do you have a test which reproduces this for the PR? It seems like you know the conditions, so laying out a compiler test for it would be helpful.

[hamidrezahanafi]

Do you have a test which reproduces this for the PR? It seems like you know the conditions, so laying out a compiler test for it would be helpful.

I tried super hard to find a test that failing and many combinations, still no luck of a compiler test that would fail.
Based on my observations, the failures are on the basically where we again didn't import React directly and use React. in the code.
Also any chance that we can merge this fix without test to unblock?

[jakebailey]

We typically don't do that, but we could.

What I'm not understanding is why this particular fix is needed; the original code says:

if (isTupleTypeNode(input) && (getLineAndCharacterOfPosition(currentSourceFile, input.pos).line === getLineAndCharacterOfPosition(currentSourceFile, input.end).line)) {
	setEmitFlags(input, EmitFlags.SingleLine);
}

Which is equivalent to the original code, and that sort of tells me the problem is elsewhere, e.g. maybe we have forgotten to set the text on the intermediate SourceFile?

[hamidrezahanafi]

We typically don't do that, but we could.

What I'm not understanding is why this particular fix is needed; the original code says:

if (isTupleTypeNode(input) && (getLineAndCharacterOfPosition(currentSourceFile, input.pos).line === getLineAndCharacterOfPosition(currentSourceFile, input.end).line)) {
	setEmitFlags(input, EmitFlags.SingleLine);
}

Which is equivalent to the original code, and that sort of tells me the problem is elsewhere, e.g. maybe we have forgotten to set the text on the intermediate SourceFile?

Even the tx.state.currentSourceFile is different than ast.GetSourceFileOfNode(input) and this only happens when I don't import React from "react" and use React. on slate/src/private/shared/types.ts
If you could point me in code where we set this state, currentSourceFile, maybe I would be able to fix it there

CROSS-FILE TUPLE: pos=59935, nodeFile=.../node_modules/@types/react/index.d.ts, currentFile=.../slate/src/private/shared/types.ts, currentLen=1905
SLICE BOUNDS DANGER: pos=59935 > currentLen=1905
  - Node file: .../node_modules/@types/react/index.d.ts
  - Current file: .../slate/src/private/shared/types.ts  
  - Position exceeds current file length by 58030 characters
  - This would cause: runtime error: slice bounds out of range [:59935] with length 1905

[jakebailey]

I understand that they are different, but they would be different in the old TS codebase too. It's odd that a 1:1 port is not working here.

Calling GetSourceFileOfNode on the input node is very suspect because the input may be transformed and not have a parent at all.

[hamidrezahanafi]

I understand that they are different, but they would be different in the old TS codebase too. It's odd that a 1:1 port is not working here.

Calling GetSourceFileOfNode on the input node is very suspect because the input may be transformed and not have a parent at all.

I tried asking AI to look for this case, failing and passing and it has trouble finding where the react types are being inlined when there is no explicit React import, if you have any ideas on where it happen, I can take further look
I wish I could create a repro

[relsunkaev]

I have a test in this pr that reproduces the bug

[jakebailey]

Turned into a compiler test, that might work, but we usually do not add this sort of unit test since it's not clear that the unit tests is actually setting up things how the actual compiler does.

[hamidrezahanafi]

@jakebailey GPT-5 found this a fix that actually works, please let me know if it makes sense, also I am not able to write a compiler test that fails

Queue late-painted statements only from the active file:
In internal/transformers/declarations/tracker.go, we now append to lateMarkedStatements only if ast.GetSourceFileOfNode(ref) == s.state.currentSourceFile. This mirrors TS’s per-file queue during declaration emit.

TS sets currentSourceFile at file entry for both single-file and bundle flows and assumes the late-painted queue is for the same file; it then uses currentSourceFile for position math during declaration emit:
File entry and per-file traversal: declarations.ts L510–538, bundle path L466–499.
Tuple single-line check uses currentSourceFile: declarations.ts L1260.

[weswigham]

I can understand how this would fix the problem - this kind of check is pervasive in the node builder - but since strada doesn't have this check here (and doesn't seem to need to?) we'd really want a test case demonstrating the crash, so we can see where, exactly, the divergence in behavior is. I'd much prefer to fix the difference in behavior at the root than at some symptom.

[hamidrezahanafi]

I can understand how this would fix the problem - this kind of check is pervasive in the node builder - but since strada doesn't have this check here (and doesn't seem to need to?) we'd really want a test case demonstrating the crash, so we can see where, exactly, the divergence in behavior is. I'd much prefer to fix the difference in behavior at the root than at some symptom.

Tried many test cases on compiler tests and couldn't do the panic
On our codebase, whenever we remove import React from "react", TS 5.9 works fine but TSGo panics, but it's not happening on new repo so size also matters maybe

[weswigham]

Sure, but what I'm getting at is that the issue isn't that these paths don't check for containing file consistency, but rather that the built/marked nodes shouldn't be getting their .original set/value returned when the original containing file is inconsistent with the destination location for the node in the first place, and that's where the bug truly lies - and tracking where that is down without a test case is going to be challenging, as will preventing a regression, so a test case is probably the first order of business.

TL;DR: This is the wrong fix - it's masking a symptom, not fixing the cause.

[hamidrezahanafi]

Sure, but what I'm getting at is that the issue isn't that these paths don't check for containing file consistency, but rather that the built/marked nodes shouldn't be getting their .original set/value returned when the original containing file is inconsistent with the destination location for the node in the first place, and that's where the bug truly lies - and tracking where that is down without a test case is going to be challenging, as will preventing a regression, so a test case is probably the first order of business.

TL;DR: This is the wrong fix - it's masking a symptom, not fixing the cause.

Finally could create a reproduction
https://github.com/hamidrezahanafi/nx-empty-baseurl/tree/hrh.triggertspanic

do npm install and npm run tsc-test-app

[jakebailey]

I followed those steps and it did not panic.

[hamidrezahanafi]

hrh.triggertspanic

Are you on this branch on that repo? hrh.triggertspanic
I can reliably reproduce it with npm run tsc-test-app

[jakebailey]

Yep:

[hamidrezahanafi]

Yep:

It needed the tsconfig.tsbuildinfo that I had to trigger the panic, pushed it to the branch, it should panic now 🤞

[jakebailey]

Yes, I now see the crash. #1408 also reproduces this with probably less stuff in the repo, so it might be a better candidate for mimimization. Though neither of these are minimal.

[jakebailey]

Surprisingly, this issue does seem to require a tsbuildinfo be present to work; same happens in #1408's.

[jakebailey]

Hm, no, it doesn't.

[jakebailey]

Here is a minimal repro:

// @strict: true
// @module: preserve
// @declaration: true

// @filename: node_modules/knex/index.d.ts

// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward
// A bunch of random text to move the positions forward

type ShouldJustBeAny = [any][0];

declare namespace knex {
  export { Knex };
}

declare namespace Knex {
  interface Interface {
    method(): ShouldJustBeAny;
  }
}

export = knex;

// @filename: index.ts

import "knex";
declare module "knex" {
  namespace Knex {
    function newFunc(): Knex.Interface;
  }
}

[jakebailey]

Debugging, what I'm seeing is that in transformAndReplaceLatePaintedStatements we walk nodes in the node_modules source file (the wrong file), but I think that was already said.

[jakebailey]

Yeah, so the code that we have now is the same as Strada and behaves the same. We really do attempt to ask for position information out of the wrong file. This only succeeds because GetLineAndCharacterOfPosition allows it in Strada. But Corsa's implementation is different and doesn't.

I think the fix is either to make GetLineAndCharacterOfPosition behave more like Strada (here be UTF-8 dragons), or actually do get the original node for the input, then get its position info from its own source file if one exists.

[hamidrezahanafi]

Updated based on your suggestions, let me know if the actual fix is something else

[hamidrezahanafi]

@microsoft-github-policy-service agree

[jakebailey]

I would suspect that you need to do MostOriginal on input (that's what I meant by original, our specific terminology), just in case we ever pass in something that has been transformed. But I suspect that is never the case, so maybe not.

[Copilot]

Pull Request Overview

This PR fixes a panic runtime error that occurs when processing AST nodes of type KindTupleTypes during declaration emit. The root issue was that the declaration transformer was using the wrong source file context when calculating position information for nodes that originated from dependency files.

• Uses the node's original source file instead of the current source file for position calculations
• Adds proper source file resolution using MostOriginal() and GetSourceFileOfNode()
• Includes a comprehensive test case that reproduces the cross-file node processing scenario

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

File	Description
internal/transformers/declarations/transform.go	Fixed source file context resolution for TupleType nodes by using original node's source file for position calculations
testdata/tests/cases/compiler/declarationEmitAugmentationUsesCorrectSourceFile.ts	Added test case with module augmentation scenario to reproduce the bug
testdata/baselines/reference/compiler/declarationEmitAugmentationUsesCorrectSourceFile.*	Generated baseline files showing expected compiler output for the test case