Skip to content
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

[nightly] Syntactically invalid declaration files emitted under extremely specific conditions #58807

Closed
MichaelMitchell-at opened this issue Jun 8, 2024 · 1 comment Β· Fixed by #58810
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Fix Available A PR has been opened for this issue

Comments

@MichaelMitchell-at
Copy link

MichaelMitchell-at commented Jun 8, 2024

πŸ”Ž Search Terms

declaration syntax invalid comment docstring

πŸ•— Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/dev/bug-workbench/?target=7&composite=true&ts=5.6.0-dev.20240608#code/PTAEAEGMHsFsAdoGcCWAXApgLlGgTgK4YBQIES+KkaO+RpY4AJhpADYCGeHaK0AdrUIky4JAAtoAdwCisdA3KTZ8tJiYAxFG2ygOAOib60SYovAAzbRn4dYugyeIoE0PGlABvUACNoADwwmACEA0ABfUAs8OFAAIn1gP0CQgLiAbjMMf0R3UBh+ClAAfVAAXl8AoND-TMUAQQ8UfhZ-UAAOAGZSACoe4lAe0CkeAZ7gM1ErHVt7HGTqgMNjU2zcjxZ2Lgx8gSKF1P8cF3WACgSktIBKfRqAHk8B0GeD49d3c8TkuJv7-gJYD4MHgAHyZcJg55Q6Ew2HQsgAPQAKuIUEhQGiMS1sh1OljcKj0dMSOZibNdMllk41m4NqxOHgdlskOj7kiQV4ns8AG4cNhEHBIzLQgoUQjUNynXn83RIq7g4g0vKbBk7CwEfjUPj8Sr+O7sqV8gWgOU4Nlg4hAA

πŸ’» Code

// @composite: true
// @strict: true
// @declaration: true
// @showEmit
// @showEmittedFile: a.d.ts

// @filename: a.ts
import { boxedBox } from "./boxedBox";

export const _ = boxedBox;

// At index 83
/**
 * wat
 */

// @filename: boxedBox.d.ts
export declare const boxedBox: import("./box").Box<{
    boxed: import("./box").Box<number>;
}>;                        // ^This is index 83 in this file

// @filename: box.d.ts
export declare class Box<T> {
    value: T;
    constructor(value: T);
}
export declare function box<T>(value: T): Box<T>;

πŸ™ Actual behavior

a.d.ts is emitted as

export declare const _: import("./box").Box<{
    boxed: import("./box").Box /**
     * wat
     */<number>;
}>;
/**
 * wat
 */

πŸ™‚ Expected behavior

a.d.ts is emitted as

export declare const _: import("./box").Box<{
    boxed: import("./box").Box<number>;
}>;
/**
 * wat
 */

Additional information about the issue

Given the extremely specific conditions under which this bug arises, somehow we hit the lottery by encountering it. Basically what I observed is that there is a point where this code is reached:

function emitCommentsAfterNode(node: Node, savedContainerPos: number, savedContainerEnd: number, savedDeclarationListContainerEnd: number) {
const emitFlags = getEmitFlags(node);
const commentRange = getCommentRange(node);
// Emit trailing comments
if (emitFlags & EmitFlags.NoNestedComments) {
commentsDisabled = false;
}
emitTrailingCommentsOfNode(node, emitFlags, commentRange.pos, commentRange.end, savedContainerPos, savedContainerEnd, savedDeclarationListContainerEnd);
const typeNode = getTypeNode(node);
if (typeNode) {
emitTrailingCommentsOfNode(node, emitFlags, typeNode.pos, typeNode.end, savedContainerPos, savedContainerEnd, savedDeclarationListContainerEnd);
}
}

in the following state:

  • node refers to Box, the one that's part of import("./box").Box that comes just before the syntax error.
  • getSourceFileOfNode(node) evaluates to the b.d.ts which is different from currentSourceFile, which is a.ts
  • We use node.pos and node.end to figure out which comment range to get the comment from. But these indexes are relative to b.d.ts, not a.ts! So we attempt to look for a comment in a.ts. Most of the time no comment would be found at these positions in a.ts, but by sheer luck, there happens to be a comment in a.ts starting right at node.end!

Imagine our surprise when my teammate's compilation started failing after touching a completely different part of a different file. They were just unlucky enough to have shifted a token into the exact right spot to surface the bug.

@fatcerberus
Copy link

fatcerberus commented Jun 9, 2024

This sounds similar to #58698 which was supposed to have been fixed by #58706

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants