Skip to content

Added typeToTypeNode with truncation #59332

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.

Sign up for GitHub

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

Merged
armanio123 merged 12 commits into from
Aug 30, 2024
Merged

Added typeToTypeNode with truncation #59332

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f6e323c
Elide truncated typenodes
Aug 8, 2024
9092688
Fixed duplicate elided comment
Aug 9, 2024
44568f7
Change appendIfUnique for overwriting
Aug 13, 2024
c1b1d40
Handle truncation in createTypeNodesFromResolvedType
Aug 16, 2024
52c2ff1
dprint...
Aug 16, 2024
05431a6
Missing baseline
Aug 16, 2024
5810eab
Revert typos
Aug 16, 2024
5891f32
Added NotEmittedTypeElement
Aug 19, 2024
90dade9
Changed elided comment
Aug 21, 2024
3b44569
Revert "Changed elided comment"
Aug 21, 2024
2cbb9f3
Dont remove types
Aug 22, 2024
ac4ba37
Added elided test
Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Added NotEmittedTypeElement
  • Loading branch information
Armando Aguirre Sepulveda committed Aug 19, 2024
commit 5891f32fc7e5f382d17cb5720ca5821c9ee97994
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7056,7 +7056,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
function createTypeNodesFromResolvedType(resolvedType: ResolvedType): TypeElement[] | undefined {
if (checkTruncationLength(context)) {
if (context.flags & NodeBuilderFlags.NoTruncation) {
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test that shows what this looks like?

Copy link
Copy Markdown
Contributor Author

@armanio123 armanio123 Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you mentioned, there's no test for this scenario. I removed this check, and nothing regressed. I'm not sure how to test it but if it's utterly necessary I can address that another PR.

Copy link
Copy Markdown
Member

@jakebailey jakebailey Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://app.codecov.io/gh/microsoft/TypeScript/pull/59332?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=microsoft implies that this code is covered somewhere in a test. Maybe add an assert and reverse engineer a specific test from that?

Copy link
Copy Markdown
Contributor Author

@armanio123 armanio123 Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After deeper analysis, the file hugeDeclarationOutputGetsTruncatedWithError.types access this code.

Check line 18, for the last property zz. It changes from "zz": { ...; } to zz: { };. This is a "type" tests, during emit the "elided" comment gets added: zz: { /*elided*/ };

Copy link
Copy Markdown
Member

@jakebailey jakebailey Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we add elided in type tests?

Copy link
Copy Markdown
Contributor Author

@armanio123 armanio123 Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was working on it. Added.

Copy link
Copy Markdown
Member

@jakebailey jakebailey Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, you're saying that in *.types files we will write "zz": { ...; } but in emit we will write zz: { /*elided*/ };? The thing I was interested in is whether or not we should do the latter for both, but maybe that's not what you were describing?

Copy link
Copy Markdown
Contributor Author

@armanio123 armanio123 Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean, before this change files in *.types files and during emit we write "zz": { ...; }.

After this change, *.types files and emit write "zz": { }, and .d.ts files "zz": { /*elided*/ } respectively.

See the test codeFixClassImplementInterfaceNoTruncationProperties.ts for the emit example. hugeDeclarationOutputGetsTruncatedWithError.types shows the *.types change.

return [];
return [addSyntheticTrailingComment(factory.createNotEmittedTypeElement(), SyntaxKind.MultiLineCommentTrivia, "elided")];
}
return [factory.createPropertySignature(/*modifiers*/ undefined, "...", /*questionToken*/ undefined, /*type*/ undefined)];
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1880,6 +1880,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri

// Transformation nodes
case SyntaxKind.NotEmittedStatement:
case SyntaxKind.NotEmittedTypeElement:
return;
}
if (isExpression(node)) {
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/factory/nodeFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ import {
NonNullExpression,
NoSubstitutionTemplateLiteral,
NotEmittedStatement,
NotEmittedTypeElement,
NullLiteral,
nullNodeConverters,
nullParenthesizerRules,
Expand Down Expand Up @@ -1007,6 +1008,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
createSyntheticExpression,
createSyntaxList,
createNotEmittedStatement,
createNotEmittedTypeElement,
createPartiallyEmittedExpression,
updatePartiallyEmittedExpression,
createCommaListExpression,
Expand Down Expand Up @@ -6263,6 +6265,11 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
: node;
}

// @api
function createNotEmittedTypeElement() {
return createBaseNode<NotEmittedTypeElement>(SyntaxKind.NotEmittedTypeElement);
}

function flattenCommaElements(node: Expression): Expression | readonly Expression[] {
if (nodeIsSynthesized(node) && !isParseTreeNode(node) && !node.original && !node.emitNode && !node.id) {
if (isCommaListExpression(node)) {
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ export const enum SyntaxKind {

// Transformation nodes
NotEmittedStatement,
NotEmittedTypeElement,
PartiallyEmittedExpression,
CommaListExpression,
SyntheticReferenceExpression,
Expand Down Expand Up @@ -3310,6 +3311,10 @@ export interface NotEmittedStatement extends Statement {
readonly kind: SyntaxKind.NotEmittedStatement;
}

export interface NotEmittedTypeElement extends TypeElement {
readonly kind: SyntaxKind.NotEmittedTypeElement;
}

/**
* A list of comma-separated expressions. This node is only created by transformations.
*/
Expand Down Expand Up @@ -9144,6 +9149,7 @@ export interface NodeFactory {
//

createNotEmittedStatement(original: Node): NotEmittedStatement;
createNotEmittedTypeElement(): NotEmittedTypeElement;
createPartiallyEmittedExpression(expression: Expression, original?: Node): PartiallyEmittedExpression;
updatePartiallyEmittedExpression(node: PartiallyEmittedExpression, expression: Expression): PartiallyEmittedExpression;
/** @internal */ createSyntheticReferenceExpression(expression: Expression, thisArg: Expression): SyntheticReferenceExpression;
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/utilitiesPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,8 @@ export function isTypeElement(node: Node): node is TypeElement {
|| kind === SyntaxKind.MethodSignature
|| kind === SyntaxKind.IndexSignature
|| kind === SyntaxKind.GetAccessor
|| kind === SyntaxKind.SetAccessor;
|| kind === SyntaxKind.SetAccessor
|| kind === SyntaxKind.NotEmittedTypeElement;
}

export function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement {
Expand Down
13 changes: 9 additions & 4 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3983,10 +3983,11 @@ declare namespace ts {
JSDocImportTag = 351,
SyntaxList = 352,
NotEmittedStatement = 353,
PartiallyEmittedExpression = 354,
CommaListExpression = 355,
SyntheticReferenceExpression = 356,
Count = 357,
NotEmittedTypeElement = 354,
PartiallyEmittedExpression = 355,
CommaListExpression = 356,
SyntheticReferenceExpression = 357,
Count = 358,
FirstAssignment = 64,
LastAssignment = 79,
FirstCompoundAssignment = 65,
Expand Down Expand Up @@ -5212,6 +5213,9 @@ declare namespace ts {
interface NotEmittedStatement extends Statement {
readonly kind: SyntaxKind.NotEmittedStatement;
}
interface NotEmittedTypeElement extends TypeElement {
readonly kind: SyntaxKind.NotEmittedTypeElement;
}
/**
* A list of comma-separated expressions. This node is only created by transformations.
*/
Expand Down Expand Up @@ -7792,6 +7796,7 @@ declare namespace ts {
createSourceFile(statements: readonly Statement[], endOfFileToken: EndOfFileToken, flags: NodeFlags): SourceFile;
updateSourceFile(node: SourceFile, statements: readonly Statement[], isDeclarationFile?: boolean, referencedFiles?: readonly FileReference[], typeReferences?: readonly FileReference[], hasNoDefaultLib?: boolean, libReferences?: readonly FileReference[]): SourceFile;
createNotEmittedStatement(original: Node): NotEmittedStatement;
createNotEmittedTypeElement(): NotEmittedTypeElement;
createPartiallyEmittedExpression(expression: Expression, original?: Node): PartiallyEmittedExpression;
updatePartiallyEmittedExpression(node: PartiallyEmittedExpression, expression: Expression): PartiallyEmittedExpression;
createCommaListExpression(elements: readonly Expression[]): CommaListExpression;
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/hugeDeclarationOutputGetsTruncatedWithError.types
View file
Open in desktop

Large diffs are not rendered by default.