Use shorthand property assignment in convert parameters to object (#30468)

* create shorthand property assignment in argument object when possible

* add shorthand property assignment test

* don't offer refactor on jsdoc comment

* add jsdoc test

* improve jsdoc test

* use crlf

Author: gabritto

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<Expression>): 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;
         });

tests/cases/fourslash/refactorConvertParamsToDestructuredObject_functionJSDoc.ts

@@ -0,0 +1,18 @@
+/// <reference path='fourslash.ts' />
+
+/////**
+//// * 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");

tests/cases/fourslash/refactorConvertParamsToDestructuredObject_shorthandProperty.ts

@@ -0,0 +1,25 @@
+/// <reference path='fourslash.ts' />
+
+// @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 */ });`
+});

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 });
     }
 }`
 });