From b5ecc1e26188a025e93135e9d1debbfe7bb4a119 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 4 Apr 2022 00:39:17 +0200 Subject: [PATCH 1/4] fix(48540): Remove comments from jsdoc union type expression --- src/compiler/emitter.ts | 2 +- src/services/refactors/extractType.ts | 6 ++++++ .../fourslash/refactorExtractType_js8.ts | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/refactorExtractType_js8.ts diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index e06b0e6b8b1dc..3204bc161e233 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4434,7 +4434,7 @@ namespace ts { // Emit this child. previousSourceFileTextKind = recordBundleFileInternalSectionStart(child); - if (shouldEmitInterveningComments) { + if (shouldEmitInterveningComments && (getEmitFlags(child) & EmitFlags.NoLeadingComments) === 0) { const commentRange = getCommentRange(child); emitTrailingCommentsOfPosition(commentRange.pos); } diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index c785160ad80d2..f2b530fda172d 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -229,6 +229,12 @@ namespace ts.refactor { function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: ExtractInfo) { const { firstStatement, selection, typeParameters } = info; + if (selection.kind === SyntaxKind.UnionType) { + for (const type of (selection as UnionTypeNode).types) { + removeAllComments(type); + } + } + const node = factory.createJSDocTypedefTag( factory.createIdentifier("typedef"), factory.createJSDocTypeExpression(selection), diff --git a/tests/cases/fourslash/refactorExtractType_js8.ts b/tests/cases/fourslash/refactorExtractType_js8.ts new file mode 100644 index 0000000000000..091fedb6c74aa --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType_js8.ts @@ -0,0 +1,19 @@ +/// + +// @allowJs: true +// @Filename: a.js +////type Bar = /*a*/[|string | /* oops */ boolean|]/*b*/; + +goTo.file('a.js') +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to typedef", + actionDescription: "Extract to typedef", + newContent: +`/** + * @typedef {string | boolean} /*RENAME*/NewType + */ + +type Bar = NewType;`, +}); From caeb56030926afb712e52d4133ff011e83798109 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 4 Apr 2022 01:48:57 +0200 Subject: [PATCH 2/4] fix(48540) - Remove comment for top level import and add extract typedef tests --- src/services/organizeImports.ts | 4 ++- src/services/refactors/extractType.ts | 25 ++++++++++++++---- ...ype_js_TypeLiteral_CommentAfterProperty.ts | 26 +++++++++++++++++++ ...pe_js_TypeLiteral_CommentBeforeProperty.ts | 26 +++++++++++++++++++ ...ExtractType_js_Union_CommentAfterMember.ts | 19 ++++++++++++++ ...tractType_js_Union_CommentBeforeMember.ts} | 0 6 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentAfterProperty.ts create mode 100644 tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentBeforeProperty.ts create mode 100644 tests/cases/fourslash/refactorExtractType_js_Union_CommentAfterMember.ts rename tests/cases/fourslash/{refactorExtractType_js8.ts => refactorExtractType_js_Union_CommentBeforeMember.ts} (100%) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index d01f2eae5f6d6..27bf87565030f 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -55,7 +55,9 @@ namespace ts.OrganizeImports { // on the first import because it is probably the header comment for the file. // Consider: we could do a more careful check that this trivia is actually a header, // but the consequences of being wrong are very minor. - suppressLeadingTrivia(oldImportDecls[0]); + + // We only want to remove leading comment of this node not its descendant + addEmitFlags(oldImportDecls[0], EmitFlags.NoLeadingComments); const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier!)!); const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier)); diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index f2b530fda172d..f6d29b39b8264 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -229,11 +229,7 @@ namespace ts.refactor { function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: ExtractInfo) { const { firstStatement, selection, typeParameters } = info; - if (selection.kind === SyntaxKind.UnionType) { - for (const type of (selection as UnionTypeNode).types) { - removeAllComments(type); - } - } + removeCommentsFromTypeNode(selection); const node = factory.createJSDocTypedefTag( factory.createIdentifier("typedef"), @@ -255,4 +251,23 @@ namespace ts.refactor { changes.insertNodeBefore(file, firstStatement, factory.createJSDocComment(/* comment */ undefined, factory.createNodeArray(concatenate(templates, [node]))), /* blankLineBetween */ true); changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); } + + function removeCommentsFromTypeNode(node: TypeNode) { + let members = factory.createNodeArray(); + + switch (node.kind) { + case SyntaxKind.UnionType: + members = (node as UnionTypeNode).types; + break; + + case SyntaxKind.TypeLiteral: + members = (node as TypeLiteralNode).members; + break; + } + + for (const member of members) { + removeAllComments(member); + } + removeAllComments(node); + } } diff --git a/tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentAfterProperty.ts b/tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentAfterProperty.ts new file mode 100644 index 0000000000000..3b24179882db0 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentAfterProperty.ts @@ -0,0 +1,26 @@ +/// + +// @allowJs: true +// @Filename: a.js +////type Foo = /*a*/[|{ +//// oops: string; +//// /** +//// * +//// */ +////}|]/*b*/; + +goTo.file('a.js') +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to typedef", + actionDescription: "Extract to typedef", + newContent: +`/** + * @typedef {{ + oops: string; +}} /*RENAME*/NewType + */ + +type Foo = NewType;`, +}); diff --git a/tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentBeforeProperty.ts b/tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentBeforeProperty.ts new file mode 100644 index 0000000000000..1283bbdf8fc50 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType_js_TypeLiteral_CommentBeforeProperty.ts @@ -0,0 +1,26 @@ +/// + +// @allowJs: true +// @Filename: a.js +////type Foo = /*a*/[|{ +//// /** +//// * +//// */ +//// oops: string; +////}|]/*b*/; + +goTo.file('a.js') +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to typedef", + actionDescription: "Extract to typedef", + newContent: +`/** + * @typedef {{ + oops: string; +}} /*RENAME*/NewType + */ + +type Foo = NewType;`, +}); diff --git a/tests/cases/fourslash/refactorExtractType_js_Union_CommentAfterMember.ts b/tests/cases/fourslash/refactorExtractType_js_Union_CommentAfterMember.ts new file mode 100644 index 0000000000000..94c805a63a032 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType_js_Union_CommentAfterMember.ts @@ -0,0 +1,19 @@ +/// + +// @allowJs: true +// @Filename: a.js +////type Bar = /*a*/[|string | boolean /* oops */ |]/*b*/; + +goTo.file('a.js') +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to typedef", + actionDescription: "Extract to typedef", + newContent: +`/** + * @typedef {string | boolean} /*RENAME*/NewType + */ + +type Bar = NewType /* oops */ ;`, +}); diff --git a/tests/cases/fourslash/refactorExtractType_js8.ts b/tests/cases/fourslash/refactorExtractType_js_Union_CommentBeforeMember.ts similarity index 100% rename from tests/cases/fourslash/refactorExtractType_js8.ts rename to tests/cases/fourslash/refactorExtractType_js_Union_CommentBeforeMember.ts From f6074fc8f2c4c6b15cc281c0461ec509554fde58 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 4 Apr 2022 21:48:09 +0200 Subject: [PATCH 3/4] fix(48540) - Remove comments from jsdoc's descendant --- src/services/refactors/extractType.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index f6d29b39b8264..92f6de8750f2a 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -252,7 +252,7 @@ namespace ts.refactor { changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); } - function removeCommentsFromTypeNode(node: TypeNode) { + function removeCommentsFromTypeNode(node: Node) { let members = factory.createNodeArray(); switch (node.kind) { @@ -263,10 +263,19 @@ namespace ts.refactor { case SyntaxKind.TypeLiteral: members = (node as TypeLiteralNode).members; break; + + case SyntaxKind.IntersectionType: + members = (node as IntersectionTypeNode).types; + break; + + case SyntaxKind.TupleType: + members = (node as TupleTypeNode).elements; + break; + } for (const member of members) { - removeAllComments(member); + removeCommentsFromTypeNode(member); } removeAllComments(node); } From 723434fc24e5735dd7fdcf0d5d01aebb3151bed9 Mon Sep 17 00:00:00 2001 From: MQuy Date: Tue, 5 Apr 2022 21:40:57 +0200 Subject: [PATCH 4/4] fix(48540) - Using no nested comments instead of traversing --- src/compiler/emitter.ts | 2 +- src/services/organizeImports.ts | 4 +--- src/services/refactors/extractType.ts | 30 +-------------------------- 3 files changed, 3 insertions(+), 33 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 3204bc161e233..e06b0e6b8b1dc 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4434,7 +4434,7 @@ namespace ts { // Emit this child. previousSourceFileTextKind = recordBundleFileInternalSectionStart(child); - if (shouldEmitInterveningComments && (getEmitFlags(child) & EmitFlags.NoLeadingComments) === 0) { + if (shouldEmitInterveningComments) { const commentRange = getCommentRange(child); emitTrailingCommentsOfPosition(commentRange.pos); } diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 27bf87565030f..d01f2eae5f6d6 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -55,9 +55,7 @@ namespace ts.OrganizeImports { // on the first import because it is probably the header comment for the file. // Consider: we could do a more careful check that this trivia is actually a header, // but the consequences of being wrong are very minor. - - // We only want to remove leading comment of this node not its descendant - addEmitFlags(oldImportDecls[0], EmitFlags.NoLeadingComments); + suppressLeadingTrivia(oldImportDecls[0]); const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier!)!); const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier)); diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 92f6de8750f2a..ac6ab73963b31 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -229,7 +229,7 @@ namespace ts.refactor { function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: ExtractInfo) { const { firstStatement, selection, typeParameters } = info; - removeCommentsFromTypeNode(selection); + setEmitFlags(selection, EmitFlags.NoComments | EmitFlags.NoNestedComments); const node = factory.createJSDocTypedefTag( factory.createIdentifier("typedef"), @@ -251,32 +251,4 @@ namespace ts.refactor { changes.insertNodeBefore(file, firstStatement, factory.createJSDocComment(/* comment */ undefined, factory.createNodeArray(concatenate(templates, [node]))), /* blankLineBetween */ true); changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); } - - function removeCommentsFromTypeNode(node: Node) { - let members = factory.createNodeArray(); - - switch (node.kind) { - case SyntaxKind.UnionType: - members = (node as UnionTypeNode).types; - break; - - case SyntaxKind.TypeLiteral: - members = (node as TypeLiteralNode).members; - break; - - case SyntaxKind.IntersectionType: - members = (node as IntersectionTypeNode).types; - break; - - case SyntaxKind.TupleType: - members = (node as TupleTypeNode).elements; - break; - - } - - for (const member of members) { - removeCommentsFromTypeNode(member); - } - removeAllComments(node); - } }