diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index b1bb2418f6ad9..b2325d2e3e6f0 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4980,7 +4980,7 @@ namespace ts { } function emitLeadingSynthesizedComment(comment: SynthesizedComment) { - if (comment.kind === SyntaxKind.SingleLineCommentTrivia) { + if (comment.hasLeadingNewline || comment.kind === SyntaxKind.SingleLineCommentTrivia) { writer.writeLine(); } writeSynthesizedComment(comment); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 1d0a125c3832b..c10e348bba53d 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2600,6 +2600,7 @@ namespace ts { text: string; pos: -1; end: -1; + hasLeadingNewline?: boolean; } // represents a top level: { type } expression in a JSDoc comment. diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 5539a8e30cb2d..23011643811f3 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -574,7 +574,7 @@ namespace ts.formatting { return childKind !== SyntaxKind.JsxClosingFragment; case SyntaxKind.IntersectionType: case SyntaxKind.UnionType: - if (childKind === SyntaxKind.TypeLiteral) { + if (childKind === SyntaxKind.TypeLiteral || childKind === SyntaxKind.TupleType) { return false; } // falls through diff --git a/src/services/refactors/convertOverloadListToSingleSignature.ts b/src/services/refactors/convertOverloadListToSingleSignature.ts index cad439a7adb68..3c747fb373678 100644 --- a/src/services/refactors/convertOverloadListToSingleSignature.ts +++ b/src/services/refactors/convertOverloadListToSingleSignature.ts @@ -25,6 +25,8 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { const signatureDecls = getConvertableOverloadListAtPosition(file, startPosition, program); if (!signatureDecls) return undefined; + const checker = program.getTypeChecker(); + const lastDeclaration = signatureDecls[signatureDecls.length - 1]; let updated = lastDeclaration; switch (lastDeclaration.kind) { @@ -39,6 +41,21 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { ); break; } + case SyntaxKind.MethodDeclaration: { + updated = updateMethod( + lastDeclaration, + lastDeclaration.decorators, + lastDeclaration.modifiers, + lastDeclaration.asteriskToken, + lastDeclaration.name, + lastDeclaration.questionToken, + lastDeclaration.typeParameters, + getNewParametersForCombinedSignature(signatureDecls), + lastDeclaration.type, + lastDeclaration.body + ); + break; + } case SyntaxKind.CallSignature: { updated = updateCallSignature( lastDeclaration, @@ -48,6 +65,16 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { ); break; } + case SyntaxKind.Constructor: { + updated = updateConstructor( + lastDeclaration, + lastDeclaration.decorators, + lastDeclaration.modifiers, + getNewParametersForCombinedSignature(signatureDecls), + lastDeclaration.body + ); + break; + } case SyntaxKind.ConstructSignature: { updated = updateConstructSignature( lastDeclaration, @@ -84,7 +111,12 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return { renameFilename: undefined, renameLocation: undefined, edits }; - function getNewParametersForCombinedSignature(signatureDeclarations: (MethodSignature | CallSignatureDeclaration | ConstructSignatureDeclaration | FunctionDeclaration)[]): NodeArray { + function getNewParametersForCombinedSignature(signatureDeclarations: (MethodSignature | MethodDeclaration | CallSignatureDeclaration | ConstructorDeclaration | ConstructSignatureDeclaration | FunctionDeclaration)[]): NodeArray { + const lastSig = signatureDeclarations[signatureDeclarations.length - 1]; + if (isFunctionLikeDeclaration(lastSig) && lastSig.body) { + // Trim away implementation signature arguments (they should already be compatible with overloads, but are likely less precise to guarantee compatability with the overloads) + signatureDeclarations = signatureDeclarations.slice(0, signatureDeclarations.length - 1); + } return createNodeArray([ createParameter( /*decorators*/ undefined, @@ -97,24 +129,56 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { ]); } - function convertSignatureParametersToTuple(decl: MethodSignature | CallSignatureDeclaration | ConstructSignatureDeclaration | FunctionDeclaration): TupleTypeNode { - return setEmitFlags(createTupleTypeNode(map(decl.parameters, convertParameterToNamedTupleMember)), EmitFlags.SingleLine); + function convertSignatureParametersToTuple(decl: MethodSignature | MethodDeclaration | CallSignatureDeclaration | ConstructorDeclaration | ConstructSignatureDeclaration | FunctionDeclaration): TupleTypeNode { + const members = map(decl.parameters, convertParameterToNamedTupleMember); + return setEmitFlags(createTupleTypeNode(members), some(members, m => !!length(getSyntheticLeadingComments(m))) ? EmitFlags.None : EmitFlags.SingleLine); } function convertParameterToNamedTupleMember(p: ParameterDeclaration): NamedTupleMember { Debug.assert(isIdentifier(p.name)); // This is checked during refactoring applicability checking - return setTextRange(createNamedTupleMember( + const result = setTextRange(createNamedTupleMember( p.dotDotDotToken, p.name, p.questionToken, p.type || createKeywordTypeNode(SyntaxKind.AnyKeyword) ), p); + const parameterDocComment = p.symbol && p.symbol.getDocumentationComment(checker); + if (parameterDocComment) { + const newComment = displayPartsToString(parameterDocComment); + if (newComment.length) { + setSyntheticLeadingComments(result, [{ + text: `* +${newComment.split("\n").map(c => ` * ${c}`).join("\n")} + `, + kind: SyntaxKind.MultiLineCommentTrivia, + pos: -1, + end: -1, + hasTrailingNewLine: true, + hasLeadingNewline: true, + }]); + } + } + return result; } + + } + + function isConvertableSignatureDeclaration(d: Node): d is MethodSignature | MethodDeclaration | CallSignatureDeclaration | ConstructorDeclaration | ConstructSignatureDeclaration | FunctionDeclaration { + switch (d.kind) { + case SyntaxKind.MethodSignature: + case SyntaxKind.MethodDeclaration: + case SyntaxKind.CallSignature: + case SyntaxKind.Constructor: + case SyntaxKind.ConstructSignature: + case SyntaxKind.FunctionDeclaration: + return true; + } + return false; } function getConvertableOverloadListAtPosition(file: SourceFile, startPosition: number, program: Program) { const node = getTokenAtPosition(file, startPosition); - const containingDecl = findAncestor(node, n => isFunctionLikeDeclaration(n)); + const containingDecl = findAncestor(node, isConvertableSignatureDeclaration); if (!containingDecl) { return; } @@ -130,14 +194,14 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { if (!every(decls, d => getSourceFileOfNode(d) === file)) { return; } - const kindOne = decls[0].kind; - if (kindOne !== SyntaxKind.MethodSignature && kindOne !== SyntaxKind.CallSignature && kindOne !== SyntaxKind.ConstructSignature && kindOne !== SyntaxKind.FunctionDeclaration) { + if (!isConvertableSignatureDeclaration(decls[0])) { return; } + const kindOne = decls[0].kind; if (!every(decls, d => d.kind === kindOne)) { return; } - const signatureDecls = decls as (MethodSignature | CallSignatureDeclaration | ConstructSignatureDeclaration | FunctionDeclaration)[]; + const signatureDecls = decls as (MethodSignature | MethodDeclaration | CallSignatureDeclaration | ConstructorDeclaration | ConstructSignatureDeclaration | FunctionDeclaration)[]; if (some(signatureDecls, d => !!d.typeParameters || some(d.parameters, p => !!p.decorators || !!p.modifiers || !isIdentifier(p.name)))) { return; } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index cbce704198c15..cfc358f210f87 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -1586,6 +1586,7 @@ declare namespace ts { text: string; pos: -1; end: -1; + hasLeadingNewline?: boolean; } export interface JSDocTypeExpression extends TypeNode { kind: SyntaxKind.JSDocTypeExpression; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 717e2a56a7b9c..f6c447bedb7a9 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -1586,6 +1586,7 @@ declare namespace ts { text: string; pos: -1; end: -1; + hasLeadingNewline?: boolean; } export interface JSDocTypeExpression extends TypeNode { kind: SyntaxKind.JSDocTypeExpression; diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature2.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature2.ts index eb7e81884f08a..8259dfb79b89a 100644 --- a/tests/cases/fourslash/refactorOverloadListToSingleSignature2.ts +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature2.ts @@ -20,30 +20,30 @@ edit.applyRefactor({ refactorName: "Convert overload list to single signature", actionName: "Convert overload list to single signature", actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, -// Aspirational: -// newContent: `declare function foo(...args: [] | [ -// /** -// * a string param doc -// */ -// a: string -//] | [ -// /** -// * a number param doc -// */ -// a: number, -// /** -// * b number param doc -// */ -// b: number -//] | [ -// /** -// * rest param doc -// */ -// ...rest: symbol[] -//]): void;`, -// Actual: - newContent: `/** +// we don't delete the param comment on the signature we update because deleting *part* of a comment is... hard +// and we definitely don't want to delete the whole comment. This is probably a good argument for why jsdoc should +// really be uniformly handled as AST nodes, and transformed as such :( +newContent: `/** * @param rest rest param doc */ -declare function foo(...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]): void;` +declare function foo(...args: [] | [ + /** + * a string param doc + */ + a: string +] | [ + /** + * a number param doc + */ + a: number, + /** + * b number param doc + */ + b: number +] | [ + /** + * rest param doc + */ + ...rest: symbol[] +]): void;`, }); diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature3.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature3.ts new file mode 100644 index 0000000000000..1de08f5852e15 --- /dev/null +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature3.ts @@ -0,0 +1,19 @@ +/// + +/////*a*/function foo(): void; +////function foo(a: string): void; +////function foo(a: number, b: number): void; +////function foo(...rest: symbol[]): void;/*b*/ +////function foo(...args: any[]): void { +//// // body +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert overload list to single signature", + actionName: "Convert overload list to single signature", + actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, + newContent: `function foo(...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]): void { + // body +}`, +}); diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature4.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature4.ts new file mode 100644 index 0000000000000..d42130fc0f6b6 --- /dev/null +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature4.ts @@ -0,0 +1,23 @@ +/// + +////class A { +//// /*a*/foo(): void; +//// foo(a: string): void; +//// foo(a: number, b: number): void; +//// foo(...rest: symbol[]): void;/*b*/ +//// foo(...args: any[]): void { +//// // body +//// } +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert overload list to single signature", + actionName: "Convert overload list to single signature", + actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, + newContent: `class A { + foo(...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]): void { + // body + } +}`, +}); diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature5.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature5.ts new file mode 100644 index 0000000000000..d8aabb038fd34 --- /dev/null +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature5.ts @@ -0,0 +1,23 @@ +/// + +////class A { +//// /*a*/constructor(); +//// constructor(a: string); +//// constructor(a: number, b: number); +//// constructor(...rest: symbol[]);/*b*/ +//// constructor(...args: any[]) { +//// // body +//// } +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert overload list to single signature", + actionName: "Convert overload list to single signature", + actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, + newContent: `class A { + constructor(...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]) { + // body + } +}`, +}); diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature6.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature6.ts new file mode 100644 index 0000000000000..0c3c0143f26ca --- /dev/null +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature6.ts @@ -0,0 +1,18 @@ +/// + +////interface A { +//// /*a*/(): void; +//// (a: string): void; +//// (a: number, b: number): void; +//// (...rest: symbol[]): void;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert overload list to single signature", + actionName: "Convert overload list to single signature", + actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, + newContent: `interface A { + (...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]): void; +}`, +}); diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature7.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature7.ts new file mode 100644 index 0000000000000..90a638be021fe --- /dev/null +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature7.ts @@ -0,0 +1,18 @@ +/// + +////interface A { +//// /*a*/new (): void; +//// new (a: string): void; +//// new (a: number, b: number): void; +//// new (...rest: symbol[]): void;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert overload list to single signature", + actionName: "Convert overload list to single signature", + actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, + newContent: `interface A { + new(...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]): void; +}`, +}); diff --git a/tests/cases/fourslash/refactorOverloadListToSingleSignature8.ts b/tests/cases/fourslash/refactorOverloadListToSingleSignature8.ts new file mode 100644 index 0000000000000..a540e8544a5e6 --- /dev/null +++ b/tests/cases/fourslash/refactorOverloadListToSingleSignature8.ts @@ -0,0 +1,18 @@ +/// + +////interface A { +//// /*a*/foo(): void; +//// foo(a: string): void; +//// foo(a: number, b: number): void; +//// foo(...rest: symbol[]): void;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert overload list to single signature", + actionName: "Convert overload list to single signature", + actionDescription: ts.Diagnostics.Convert_overload_list_to_single_signature.message, + newContent: `interface A { + foo(...args: [] | [a: string] | [a: number, b: number] | [...rest: symbol[]]): void; +}`, +});