Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 13.7k
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
f6e323c909268844568f7c1b1d4052c2ff105431a65810eab5891f3290dade93b445692cbb9f3ac4ba37File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,6 +6,7 @@ import { | ||
| addRange, | ||
| addRelatedInfo, | ||
| addSyntheticLeadingComment, | ||
| addSyntheticTrailingComment, | ||
| AliasDeclarationNode, | ||
| AllAccessorDeclarations, | ||
| AmbientModuleDeclaration, | ||
| @@ -7054,6 +7055,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | ||
| function createTypeNodesFromResolvedType(resolvedType: ResolvedType): TypeElement[] | undefined { | ||
| if (checkTruncationLength(context)) { | ||
| if (context.flags & NodeBuilderFlags.NoTruncation) { | ||
| return [addSyntheticTrailingComment(factory.createNotEmittedTypeElement(), SyntaxKind.MultiLineCommentTrivia, "elided")]; | ||
| } | ||
| return [factory.createPropertySignature(/*modifiers*/ undefined, "...", /*questionToken*/ undefined, /*type*/ undefined)]; | ||
| } | ||
| const typeElements: TypeElement[] = []; | ||
| @@ -7085,7 +7089,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | ||
| } | ||
| } | ||
| if (checkTruncationLength(context) && (i + 2 < properties.length - 1)) { | ||
| typeElements.push(factory.createPropertySignature(/*modifiers*/ undefined, `... ${properties.length - i} more ...`, /*questionToken*/ undefined, /*type*/ undefined)); | ||
| if (context.flags & NodeBuilderFlags.NoTruncation) { | ||
| const typeElement = typeElements.pop()!; | ||
| typeElements.push(addSyntheticTrailingComment(typeElement, SyntaxKind.MultiLineCommentTrivia, `... ${properties.length - i} more elided ...`)); | ||
| } | ||
| else { | ||
| typeElements.push(factory.createPropertySignature(/*modifiers*/ undefined, `... ${properties.length - i} more ...`, /*questionToken*/ undefined, /*type*/ undefined)); | ||
| } | ||
| addPropertyToElementList(properties[properties.length - 1], context, typeElements); | ||
| break; | ||
| } | ||
| @@ -7100,7 +7110,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | ||
| if (!(context.flags & NodeBuilderFlags.NoTruncation)) { | ||
| return factory.createTypeReferenceNode(factory.createIdentifier("..."), /*typeArguments*/ undefined); | ||
| } | ||
| return factory.createKeywordTypeNode(SyntaxKind.AnyKeyword); | ||
| return addSyntheticLeadingComment(factory.createKeywordTypeNode(SyntaxKind.AnyKeyword), SyntaxKind.MultiLineCommentTrivia, "elided"); | ||
| } | ||
| function shouldUsePlaceholderForProperty(propertySymbol: Symbol, context: NodeBuilderContext) { | ||
| @@ -7260,12 +7270,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | ||
| if (some(types)) { | ||
| if (checkTruncationLength(context)) { | ||
| if (!isBareList) { | ||
| return [factory.createTypeReferenceNode("...", /*typeArguments*/ undefined)]; | ||
| return [ | ||
| context.flags & NodeBuilderFlags.NoTruncation | ||
| ? addSyntheticLeadingComment(factory.createKeywordTypeNode(SyntaxKind.AnyKeyword), SyntaxKind.MultiLineCommentTrivia, "elided") | ||
| : factory.createTypeReferenceNode("...", /*typeArguments*/ undefined), | ||
| ]; | ||
| } | ||
| else if (types.length > 2) { | ||
| return [ | ||
| typeToTypeNodeHelper(types[0], context), | ||
| factory.createTypeReferenceNode(`... ${types.length - 2} more ...`, /*typeArguments*/ undefined), | ||
| context.flags & NodeBuilderFlags.NoTruncation | ||
| ? addSyntheticLeadingComment(factory.createKeywordTypeNode(SyntaxKind.AnyKeyword), SyntaxKind.MultiLineCommentTrivia, `... ${types.length - 2} more elided ...`) | ||
| : factory.createTypeReferenceNode(`... ${types.length - 2} more ...`, /*typeArguments*/ undefined), | ||
| typeToTypeNodeHelper(types[types.length - 1], context), | ||
| ]; | ||
| } | ||
| @@ -7278,7 +7294,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | ||
| for (const type of types) { | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Am I confused or is this truncating things more when the flag is enabled? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not more per se, but it is replacing the previous values with an We kinda have 3 states: when Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A
| ||
| i++; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this is discarding all the parts we already actually got to before hitting the truncation limit - shouldn't this all be something more like result.push(context.flags&NodeBuilderFlags.NoTruncation ? addSyntheticLeadingComment(factory.createKeywordTypeNode(SyntaxKind.AnyKeyword),SyntaxKind.MultiLineCommentTrivia,`... ${types.length-i} more elided ...`) : factory.createTypeReferenceNode(`... ${types.length-i} more ...`,/*typeArguments*/undefined));merged into the line below? This way we still get something like out? (Part of the reason elision is a bit special here is because it endeavors to retain the last element, since elision only really makes sense in the middle.) for the whole union, even if there's enough allotted space for a lot of the union.
| ||
| if (checkTruncationLength(context) && (i + 2 < types.length - 1)) { | ||
| result.push(factory.createTypeReferenceNode(`... ${types.length - i} more ...`, /*typeArguments*/ undefined)); | ||
| result.push( | ||
| context.flags & NodeBuilderFlags.NoTruncation | ||
| ? addSyntheticLeadingComment(factory.createKeywordTypeNode(SyntaxKind.AnyKeyword), SyntaxKind.MultiLineCommentTrivia, `... ${types.length - i} more elided ...`) | ||
| : factory.createTypeReferenceNode(`... ${types.length - i} more ...`, /*typeArguments*/ undefined), | ||
| ); | ||
| const typeNode = typeToTypeNodeHelper(types[types.length - 1], context); | ||
| if (typeNode) { | ||
| result.push(typeNode); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -39,6 +39,6 @@ var C = /** @class */ (function () { | ||
| declare class C { | ||
| static D: { | ||
| new (): {}; | ||
| D: any; | ||
| D: /*elided*/ any; | ||
| }; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.typesaccess this code.Check line 18, for the last property
zz. It changes from"zz": { ...; }tozz: { };. This is a "type" tests, during emit the "elided" comment gets added:zz: { /*elided*/ };There was a problem hiding this comment.
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
elidedin type tests?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
*.typesfiles we will write"zz": { ...; }but in emit we will writezz: { /*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?There was a problem hiding this comment.
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
*.typesfiles and during emit we write"zz": { ...; }.After this change,
*.typesfiles and emit write"zz": { }, and.d.tsfiles"zz": { /*elided*/ }respectively.See the test
codeFixClassImplementInterfaceNoTruncationProperties.tsfor the emit example.hugeDeclarationOutputGetsTruncatedWithError.typesshows the*.typeschange.