diff --git a/src/services/refactors/convertParamsToDestructuredObject.ts b/src/services/refactors/convertParamsToDestructuredObject.ts index 91836f29821..86aac3a5a0c 100644 --- a/src/services/refactors/convertParamsToDestructuredObject.ts +++ b/src/services/refactors/convertParamsToDestructuredObject.ts @@ -233,6 +233,10 @@ namespace ts.refactor.convertParamsToDestructuredObject { function getFunctionDeclarationAtPosition(file: SourceFile, startPosition: number, checker: TypeChecker): ValidFunctionDeclaration | undefined { const node = getTouchingToken(file, startPosition); const functionDeclaration = getContainingFunction(node); + + // don't offer refactor on top-level JSDoc + if (isTopLevelJSDoc(node)) return undefined; + if (functionDeclaration && isValidFunctionDeclaration(functionDeclaration, checker) && rangeContainsRange(functionDeclaration, node) @@ -241,6 +245,15 @@ namespace ts.refactor.convertParamsToDestructuredObject { return undefined; } + function isTopLevelJSDoc(node: Node): boolean { + const containingJSDoc = findAncestor(node, isJSDocNode); + if (containingJSDoc) { + const containingNonJSDoc = findAncestor(containingJSDoc, n => !isJSDocNode(n)); + return !!containingNonJSDoc && isFunctionLikeDeclaration(containingNonJSDoc); + } + return false; + } + function isValidFunctionDeclaration( functionDeclaration: SignatureDeclaration, checker: TypeChecker): functionDeclaration is ValidFunctionDeclaration { @@ -308,13 +321,23 @@ namespace ts.refactor.convertParamsToDestructuredObject { return parameters; } + function createPropertyOrShorthandAssignment(name: string, initializer: Expression): PropertyAssignment | ShorthandPropertyAssignment { + if (isIdentifier(initializer) && getTextOfIdentifierOrLiteral(initializer) === name) { + return createShorthandPropertyAssignment(name); + } + return createPropertyAssignment(name, initializer); + } + function createNewArgument(functionDeclaration: ValidFunctionDeclaration, functionArguments: NodeArray): ObjectLiteralExpression { const parameters = getRefactorableParameters(functionDeclaration.parameters); const hasRestParameter = isRestParameter(last(parameters)); const nonRestArguments = hasRestParameter ? functionArguments.slice(0, parameters.length - 1) : functionArguments; const properties = map(nonRestArguments, (arg, i) => { - const property = createPropertyAssignment(getParameterName(parameters[i]), arg); - suppressLeadingAndTrailingTrivia(property.initializer); + const parameterName = getParameterName(parameters[i]); + const property = createPropertyOrShorthandAssignment(parameterName, arg); + + suppressLeadingAndTrailingTrivia(property.name); + if (isPropertyAssignment(property)) suppressLeadingAndTrailingTrivia(property.initializer); copyComments(arg, property); return property; }); diff --git a/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_functionJSDoc.ts b/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_functionJSDoc.ts new file mode 100644 index 00000000000..fb87c205025 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_functionJSDoc.ts @@ -0,0 +1,18 @@ +/// + +/////** +//// * Return the boolean state of attribute from an element +//// * /*a*/@param/*b*/ el The source of the attributes. +//// * @param atty Name of the attribute or a string of candidate attribute names. +//// * @param def Default boolean value when attribute is undefined. +//// */ +////export function /*c*/getBoolFromAttribute/*d*/( +//// /*e*//** inline JSDoc *//*f*/ attr: string | string[], +//// def: boolean = false): boolean { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Convert parameters to destructured object"); +goTo.select("c", "d"); +verify.refactorAvailable("Convert parameters to destructured object"); +goTo.select("e", "f"); +verify.refactorAvailable("Convert parameters to destructured object"); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_shorthandProperty.ts b/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_shorthandProperty.ts new file mode 100644 index 00000000000..86ea98582ab --- /dev/null +++ b/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_shorthandProperty.ts @@ -0,0 +1,25 @@ +/// + +// @Filename: f.ts +////function /*a*/f/*b*/(a: number, b: number, ...rest: string[]) { } +////const a = 4; +////const b = 5; +////f(a, b); +////const rest = ["a", "b", "c"]; +////f(a, b, ...rest); +////f(/** a */ a /** aa */, /** b */ b /** bb */); + + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert parameters to destructured object", + actionName: "Convert parameters to destructured object", + actionDescription: "Convert parameters to destructured object", + newContent: `function f({ a, b, rest = [] }: { a: number; b: number; rest?: string[]; }) { } +const a = 4; +const b = 5; +f({ a, b }); +const rest = ["a", "b", "c"]; +f({ a, b, rest: [...rest] }); +f({ /** a */ a /** aa */, /** b */ b /** bb */ });` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_superCall.ts b/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_superCall.ts index 56227a9078a..2bdee836472 100644 --- a/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_superCall.ts +++ b/tests/cases/fourslash/refactorConvertParamsToDestructuredObject_superCall.ts @@ -19,7 +19,7 @@ edit.applyRefactor({ } class B extends A { constructor(a: string, b: string, c: string) { - super({ a: a, b: b }); + super({ a, b }); } }` }); \ No newline at end of file