feat: #707 - Add useTrailingCommas manipulation setting option.

dsherret · Oct 5, 2019 · 938c05c · 938c05c
1 parent 2043ccb
commit 938c05c
Show file tree

Hide file tree

Showing 20 changed files with 114 additions and 29 deletions.
diff --git a/docs/manipulation/settings.md b/docs/manipulation/settings.md
@@ -21,7 +21,10 @@ const project = new Project({
         // Whether to change shorthand property assignments to property assignments
         // and add aliases to import & export specifiers (see more information in
         // the renaming section of the documentation).
-        usePrefixAndSuffixTextForRename: false
+        usePrefixAndSuffixTextForRename: false,
+        // Whether to use trailing commas in multi-line scenarios where trailing
+        // commas would be used.
+        useTrailingCommas: false
     }
 });
 ```

diff --git a/lib/ts-morph.d.ts b/lib/ts-morph.d.ts
@@ -4402,6 +4402,10 @@ export declare class Node<NodeType extends ts.Node = ts.Node> {
      * Gets the child nodes passed to the delegate of `node.forEachChild(child => {})` as an array.
      */
     forEachChildAsArray(): Node<ts.Node>[];
+    /**
+     * Gets the descendant nodes passed to the delegate of `node.forEachDescendant(descendant => {})` as an array.
+     */
+    forEachDescendantAsArray(): Node<ts.Node>[];
     /**
      * Gets the node's descendants.
      */
@@ -11072,6 +11076,10 @@ export interface ManipulationSettings extends SupportedFormatCodeSettingsOnly {
      * This setting is only available when using TypeScript 3.4+.
      */
     usePrefixAndSuffixTextForRename: boolean;
+    /**
+     * Whether to use trailing commas when inserting or removing nodes.
+     */
+    useTrailingCommas: boolean;
 }
 
 /**
@@ -11133,6 +11141,10 @@ export declare class ManipulationSettingsContainer extends SettingsContainer<Man
      * Gets whether to use prefix and suffix text when renaming.
      */
     getUsePrefixAndSuffixTextForRename(): boolean;
+    /**
+     * Gets whether trailing commas should be used.
+     */
+    getUseTrailingCommas(): boolean;
     /**
      * Sets one or all of the settings.
      * @param settings - Settings to set.

diff --git a/src/compiler/ast/base/ArgumentedNode.ts b/src/compiler/ast/base/ArgumentedNode.ts
@@ -88,7 +88,8 @@ export function ArgumentedNode<T extends Constructor<ArgumentedNodeExtensionType
                 parent: this.getFirstChildByKindOrThrow(SyntaxKind.OpenParenToken).getNextSiblingIfKindOrThrow(SyntaxKind.SyntaxList),
                 currentNodes: originalArgs,
                 insertIndex: index,
-                newText: writer.toString()
+                newText: writer.toString(),
+                useTrailingCommas: false
             });
 
             return getNodesToReturn(originalArgs, this.getArguments(), index, false);

diff --git a/src/compiler/ast/base/ExtendsClauseableNode.ts b/src/compiler/ast/base/ExtendsClauseableNode.ts
@@ -91,7 +91,8 @@ export function ExtendsClauseableNode<T extends Constructor<ExtendsClauseableNod
                     parent: extendsClause.getFirstChildByKindOrThrow(SyntaxKind.SyntaxList),
                     currentNodes: originalExtends,
                     insertIndex: index,
-                    newText: writer.toString()
+                    newText: writer.toString(),
+                    useTrailingCommas: false
                 });
             }
             else {

diff --git a/src/compiler/ast/base/ImplementsClauseableNode.ts b/src/compiler/ast/base/ImplementsClauseableNode.ts
@@ -92,7 +92,8 @@ export function ImplementsClauseableNode<T extends Constructor<ImplementsClausea
                     parent: implementsClause.getFirstChildByKindOrThrow(SyntaxKind.SyntaxList),
                     currentNodes: originalImplements,
                     insertIndex: index,
-                    newText: writer.toString()
+                    newText: writer.toString(),
+                    useTrailingCommas: false
                 });
             }
             else {

diff --git a/src/compiler/ast/base/ParameteredNode.ts b/src/compiler/ast/base/ParameteredNode.ts
@@ -104,7 +104,8 @@ export function ParameteredNode<T extends Constructor<ParameteredNodeExtensionTy
                 parent: syntaxList,
                 currentNodes: parameters,
                 insertIndex: index,
-                newText: writer.toString()
+                newText: writer.toString(),
+                useTrailingCommas: false
             });
 
             return getNodesToReturn(parameters, this.getParameters(), index, false);

diff --git a/src/compiler/ast/base/TypeArgumentedNode.ts b/src/compiler/ast/base/TypeArgumentedNode.ts
@@ -94,7 +94,8 @@ export function TypeArgumentedNode<T extends Constructor<TypeArgumentedNodeExten
                     parent: this.getFirstChildByKindOrThrow(SyntaxKind.LessThanToken).getNextSiblingIfKindOrThrow(SyntaxKind.SyntaxList),
                     currentNodes: typeArguments,
                     insertIndex: index,
-                    newText: argumentTexts.join(", ")
+                    newText: argumentTexts.join(", "),
+                    useTrailingCommas: false
                 });
             }
 

diff --git a/src/compiler/ast/base/TypeParameteredNode.ts b/src/compiler/ast/base/TypeParameteredNode.ts
@@ -114,7 +114,8 @@ export function TypeParameteredNode<T extends Constructor<TypeParameteredNodeExt
                     parent: this.getFirstChildByKindOrThrow(SyntaxKind.LessThanToken).getNextSiblingIfKindOrThrow(SyntaxKind.SyntaxList),
                     currentNodes: typeParameters,
                     insertIndex: index,
-                    newText: writer.toString()
+                    newText: writer.toString(),
+                    useTrailingCommas: false
                 });
             }
 

diff --git a/src/compiler/ast/enum/EnumDeclaration.ts b/src/compiler/ast/enum/EnumDeclaration.ts
@@ -114,7 +114,8 @@ export class EnumDeclaration extends EnumDeclarationBase<ts.EnumDeclaration> {
             currentNodes: members,
             insertIndex: index,
             newText: writer.toString(),
-            useNewLines: true
+            useNewLines: true,
+            useTrailingCommas: this._context.manipulationSettings.getUseTrailingCommas()
         });
 
         // get the members

diff --git a/src/compiler/ast/expression/array/ArrayLiteralExpression.ts b/src/compiler/ast/expression/array/ArrayLiteralExpression.ts
@@ -69,7 +69,8 @@ export class ArrayLiteralExpression extends PrimaryExpression<ts.ArrayLiteralExp
                 currentNodes: elements,
                 insertIndex: index,
                 newText: writer.toString(),
-                useNewLines
+                useNewLines,
+                useTrailingCommas: useNewLines && node._context.manipulationSettings.getUseTrailingCommas()
             });
 
             const newElements = node.getElements();

diff --git a/src/compiler/ast/expression/object/ObjectLiteralExpression.ts b/src/compiler/ast/expression/object/ObjectLiteralExpression.ts
@@ -136,7 +136,8 @@ export class ObjectLiteralExpression extends ObjectLiteralExpressionBase<ts.Obje
             currentNodes: properties,
             insertIndex: index,
             newText: writer.toString(),
-            useNewLines: true
+            useNewLines: true,
+            useTrailingCommas: this._context.manipulationSettings.getUseTrailingCommas()
         });
 
         // get the properties
@@ -391,7 +392,8 @@ export class ObjectLiteralExpression extends ObjectLiteralExpressionBase<ts.Obje
             currentNodes: oldProperties,
             insertIndex: index,
             newText: writer.toString(),
-            useNewLines: true
+            useNewLines: true,
+            useTrailingCommas: this._context.manipulationSettings.getUseTrailingCommas()
         });
 
         return getNodesToReturn(oldProperties, this.getPropertiesWithComments(), index, false);

diff --git a/src/compiler/ast/module/ExportDeclaration.ts b/src/compiler/ast/module/ExportDeclaration.ts
@@ -199,7 +199,8 @@ export class ExportDeclaration extends ExportDeclarationBase<ts.ExportDeclaratio
                 currentNodes: originalNamedExports,
                 insertIndex: index,
                 newText: writer.toString(),
-                surroundWithSpaces: this._context.getFormatCodeSettings().insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces
+                surroundWithSpaces: this._context.getFormatCodeSettings().insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces,
+                useTrailingCommas: false
             });
         }
 

diff --git a/src/compiler/ast/module/ImportDeclaration.ts b/src/compiler/ast/module/ImportDeclaration.ts
@@ -330,7 +330,8 @@ export class ImportDeclaration extends ImportDeclarationBase<ts.ImportDeclaratio
                 currentNodes: originalNamedImports,
                 insertIndex: index,
                 newText: writer.toString(),
-                surroundWithSpaces: this._context.getFormatCodeSettings().insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces
+                surroundWithSpaces: this._context.getFormatCodeSettings().insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces,
+                useTrailingCommas: false
             });
         }
 

diff --git a/src/compiler/ast/variable/VariableDeclarationList.ts b/src/compiler/ast/variable/VariableDeclarationList.ts
@@ -112,7 +112,8 @@ export class VariableDeclarationList extends VariableDeclarationListBase<ts.Vari
             parent: this.getFirstChildByKindOrThrow(SyntaxKind.SyntaxList),
             currentNodes: this.getDeclarations(),
             insertIndex: index,
-            newText: writer.toString()
+            newText: writer.toString(),
+            useTrailingCommas: false
         });
 
         return getNodesToReturn(originalChildrenCount, this.getDeclarations(), index, false);

diff --git a/src/manipulation/manipulations/insertion.ts b/src/manipulation/manipulations/insertion.ts
@@ -68,6 +68,7 @@ export interface InsertIntoCommaSeparatedNodesOptions {
     insertIndex: number;
     newText: string;
     parent: Node;
+    useTrailingCommas: boolean;
     useNewLines?: boolean;
     surroundWithSpaces?: boolean;
 }
@@ -89,7 +90,7 @@ export function insertIntoCommaSeparatedNodes(opts: InsertIntoCommaSeparatedNode
     if (previousNode != null) {
         prependCommaAndSeparator();
 
-        if (nextNonCommentNode != null)
+        if (nextNonCommentNode != null || opts.useTrailingCommas)
             appendCommaAndSeparator();
         else if (opts.useNewLines || opts.surroundWithSpaces)
             appendSeparator();
@@ -109,10 +110,10 @@ export function insertIntoCommaSeparatedNodes(opts: InsertIntoCommaSeparatedNode
         if (opts.useNewLines || opts.surroundWithSpaces)
             prependSeparator();
 
-        if (nextNonCommentNode == null)
-            appendSeparator();
-        else
+        if (nextNonCommentNode != null || opts.useTrailingCommas)
             appendCommaAndSeparator();
+        else
+            appendSeparator();
 
         const insertPos = isContained ? parent.getPos() : parent.getStart(true);
         insertIntoParentTextRange({
@@ -125,7 +126,11 @@ export function insertIntoCommaSeparatedNodes(opts: InsertIntoCommaSeparatedNode
     else {
         if (opts.useNewLines || opts.surroundWithSpaces) {
             prependSeparator();
-            appendSeparator();
+
+            if (opts.useTrailingCommas)
+                appendCommaAndSeparator();
+            else
+                appendSeparator();
         }
         else {
             appendIndentation();

diff --git a/src/options/ManipulationSettingsContainer.ts b/src/options/ManipulationSettingsContainer.ts
@@ -33,6 +33,8 @@ export interface ManipulationSettings extends SupportedFormatCodeSettingsOnly {
      * This setting is only available when using TypeScript 3.4+.
      */
     usePrefixAndSuffixTextForRename: boolean;
+    /** Whether to use trailing commas when inserting or removing nodes. */
+    useTrailingCommas: boolean;
 }
 
 /**
@@ -68,7 +70,8 @@ export class ManipulationSettingsContainer extends SettingsContainer<Manipulatio
             newLineKind: NewLineKind.LineFeed,
             quoteKind: QuoteKind.Double,
             insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces: true,
-            usePrefixAndSuffixTextForRename: false
+            usePrefixAndSuffixTextForRename: false,
+            useTrailingCommas: false
         });
     }
 
@@ -146,6 +149,13 @@ export class ManipulationSettingsContainer extends SettingsContainer<Manipulatio
         return this._settings.usePrefixAndSuffixTextForRename;
     }
 
+    /**
+     * Gets whether trailing commas should be used.
+     */
+    getUseTrailingCommas() {
+        return this._settings.useTrailingCommas;
+    }
+
     /**
      * Sets one or all of the settings.
      * @param settings - Settings to set.

diff --git a/src/tests/compiler/ast/enum/enumDeclarationTests.ts b/src/tests/compiler/ast/enum/enumDeclarationTests.ts
@@ -135,8 +135,10 @@ describe(nameof(EnumDeclaration), () => {
         });
 
         type WriterStructureType = (OptionalKind<EnumMemberStructure> | WriterFunction | string)[] | string | WriterFunction;
-        function doWriterTest(startCode: string, index: number, structures: WriterStructureType, expectedCode: string) {
-            const { firstChild, sourceFile } = getInfoFromText<EnumDeclaration>(startCode);
+        function doWriterTest(startCode: string, index: number, structures: WriterStructureType, expectedCode: string, options?: { useTrailingCommas?: boolean; }) {
+            const { firstChild, sourceFile, project } = getInfoFromText<EnumDeclaration>(startCode);
+            if (options && options.useTrailingCommas)
+                project.manipulationSettings.set({ useTrailingCommas: true });
             const result = firstChild.insertMembers(index, structures);
             assert<IsExact<typeof result, (EnumMember | CommentEnumMember)[]>>(true);
 
@@ -167,6 +169,16 @@ describe(nameof(EnumDeclaration), () => {
                 "enum MyEnum {\n    // testing\n}\n"
             );
         });
+
+        it("should support trailing commas", () => {
+            doWriterTest(
+                "enum MyEnum {\n}\n",
+                0,
+                "member",
+                "enum MyEnum {\n    member,\n}\n",
+                { useTrailingCommas: true }
+            );
+        });
     });
 
     describe(nameof<EnumDeclaration>(d => d.insertMember), () => {

diff --git a/src/tests/compiler/ast/expression/array/arrayLiteralExpressionTests.ts b/src/tests/compiler/ast/expression/array/arrayLiteralExpressionTests.ts
@@ -30,8 +30,11 @@ describe(nameof(ArrayLiteralExpression), () => {
     });
 
     describe(nameof<ArrayLiteralExpression>(e => e.insertElements), () => {
-        function doTest(text: string, index: number, elementTexts: string[], expectedText: string, options?: { useNewLines?: boolean; }) {
-            const { arrayLiteralExpression, sourceFile } = getArrayLiteralExpression(text);
+        type Options = { useNewLines?: boolean; useTrailingCommas?: boolean; };
+        function doTest(text: string, index: number, elementTexts: string[], expectedText: string, options?: Options) {
+            const { arrayLiteralExpression, sourceFile, project } = getArrayLiteralExpression(text);
+            if (options && options.useTrailingCommas)
+                project.manipulationSettings.set({ useTrailingCommas: true });
             const result = arrayLiteralExpression.insertElements(index, elementTexts, options);
             expect(result.map(r => r.getText())).to.deep.equal(elementTexts);
             expect(sourceFile.getFullText()).to.equal(expectedText);
@@ -73,6 +76,14 @@ describe(nameof(ArrayLiteralExpression), () => {
             doTest("var t = [2, 3, 4]", 0, ["1"], `var t = [\n    1,\n    2, 3, 4]`, { useNewLines: true });
         });
 
+        it("should not use trailing commas when single line", () => {
+            doTest("var t = [1, 2, 3]", 3, ["4"], `var t = [1, 2, 3, 4]`, { useNewLines: false, useTrailingCommas: true });
+        });
+
+        it("should use trailing commas when multi-line", () => {
+            doTest("var t = [\n    1,\n    2\n]", 2, ["3"], `var t = [\n    1,\n    2,\n    3,\n]`, { useNewLines: true, useTrailingCommas: true });
+        });
+
         function doWriterTest(text: string, index: number, elements: WriterFunction | ReadonlyArray<string | WriterFunction>, expectedText: string,
             options?: { useNewLines?: boolean; })
         {

diff --git a/src/tests/compiler/ast/expression/object/objectLiteralExpressionTests.ts b/src/tests/compiler/ast/expression/object/objectLiteralExpressionTests.ts
@@ -85,8 +85,10 @@ describe(nameof(ObjectLiteralExpression), () => {
 
     describe(nameof<ObjectLiteralExpression>(e => e.insertProperties), () => {
         type StructuresType = string | WriterFunction | (string | WriterFunction | ObjectLiteralExpressionPropertyStructures)[];
-        function doTest(text: string, index: number, structures: StructuresType, expectedText: string) {
-            const { sourceFile, objectLiteralExpression } = getObjectLiteralExpression(text);
+        function doTest(text: string, index: number, structures: StructuresType, expectedText: string, options?: { useTrailingCommas: boolean; }) {
+            const { sourceFile, objectLiteralExpression, project } = getObjectLiteralExpression(text);
+            if (options && options.useTrailingCommas)
+                project.manipulationSettings.set({ useTrailingCommas: true });
             const result = objectLiteralExpression.insertProperties(index, structures);
             expect(sourceFile.getFullText()).to.equal(expectedText);
             expect(result.length).to.deep.equal(structures.length);
@@ -117,6 +119,20 @@ describe(nameof(ObjectLiteralExpression), () => {
                 name: "s1"
             }, "//1", writer => writer.write("//2")], expectedText);
         });
+
+        it("should support trailing commas when inserting a single node", () => {
+            doTest("const o = {\n};", 0, [{
+                kind: StructureKind.ShorthandPropertyAssignment,
+                name: "p1"
+            }], "const o = {\n    p1,\n};", { useTrailingCommas: true });
+        });
+
+        it("should support trailing when inserting after another node", () => {
+            doTest("const o = {\n    p1,\n};", 1, [{
+                kind: StructureKind.ShorthandPropertyAssignment,
+                name: "p2"
+            }], "const o = {\n    p1,\n    p2,\n};", { useTrailingCommas: true });
+        });
     });
 
     describe(nameof<ObjectLiteralExpression>(e => e.insertProperty), () => {

diff --git a/src/tests/manipulationSettingsTests.ts b/src/tests/manipulationSettingsTests.ts
@@ -35,7 +35,8 @@ describe(nameof(ManipulationSettingsContainer), () => {
             newLineKind: NewLineKind.LineFeed,
             indentationText: IndentationText.FourSpaces,
             insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces: true,
-            usePrefixAndSuffixTextForRename: false
+            usePrefixAndSuffixTextForRename: false,
+            useTrailingCommas: false
         });
     });
 
@@ -50,7 +51,8 @@ describe(nameof(ManipulationSettingsContainer), () => {
             newLineKind: NewLineKind.LineFeed,
             indentationText: IndentationText.FourSpaces,
             insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces: true,
-            usePrefixAndSuffixTextForRename: false
+            usePrefixAndSuffixTextForRename: false,
+            useTrailingCommas: false
         });
     });
 
@@ -69,7 +71,8 @@ describe(nameof(ManipulationSettingsContainer), () => {
             newLineKind: NewLineKind.CarriageReturnLineFeed,
             indentationText: IndentationText.EightSpaces,
             insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces: false,
-            usePrefixAndSuffixTextForRename: true
+            usePrefixAndSuffixTextForRename: true,
+            useTrailingCommas: false
         });
     });