diff --git a/src/rules/trailingCommaRule.ts b/src/rules/trailingCommaRule.ts index 8e49e334418..f9b8b5ca22d 100644 --- a/src/rules/trailingCommaRule.ts +++ b/src/rules/trailingCommaRule.ts @@ -15,15 +15,19 @@ * limitations under the License. */ -import { getChildOfKind, isSameLine } from "tsutils"; +import { getChildOfKind, isReassignmentTarget, isSameLine } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; type OptionValue = "always" | "never" | "ignore"; type OptionName = "arrays" | "exports" | "functions" | "imports" | "objects" | "typeLiterals"; -type CustomOptionValue = Partial>; -type Options = Record<"multiline" | "singleline", CustomOptionValue>; +type CustomOptionValue = Record; +interface Options { + multiline: CustomOptionValue; + singleline: CustomOptionValue; + specCompliant: boolean; +} const defaultOptions: CustomOptionValue = fillOptions("ignore" as "ignore"); @@ -38,13 +42,13 @@ function fillOptions(value: T): Record { }; } -type OptionsJson = Record<"multiline" | "singleline", Partial | OptionValue>; +type OptionsJson = Partial | OptionValue> & {esSpecCompliant: boolean}>; function normalizeOptions(options: OptionsJson): Options { - return { multiline: normalize(options.multiline), singleline: normalize(options.singleline) }; + return { multiline: normalize(options.multiline), singleline: normalize(options.singleline), specCompliant: !!options.esSpecCompliant}; - function normalize(value: OptionsJson["multiline"]): CustomOptionValue { - return typeof value === "string" ? fillOptions(value) : { ...defaultOptions, ...value }; - } +} +function normalize(value: OptionsJson["multiline"]): CustomOptionValue { + return typeof value === "string" ? fillOptions(value) : { ...defaultOptions, ...value }; } /* tslint:disable:object-literal-sort-keys */ @@ -87,12 +91,18 @@ export class Rule extends Lint.Rules.AbstractRule { An array is considered "multiline" if its closing bracket is on a line after the last array element. The same general logic is followed for object literals, function typings, named import statements - and function parameters.`, + and function parameters. + + To align this rule with the ECMAScript specification that is implemented in modern JavaScript VMs, + there is a third option \`esSpecCompliant\`. Set this option to \`true\` to disallow trailing comma on + object and array rest and rest parameters. + `, options: { type: "object", properties: { multiline: metadataOptionShape, singleline: metadataOptionShape, + esSpecCompliant: {type: "boolean"}, }, additionalProperties: false, }, @@ -107,6 +117,7 @@ export class Rule extends Lint.Rules.AbstractRule { functions: "never", typeLiterals: "ignore", }, + esSpecCompliant: true, }, ], ], @@ -116,10 +127,11 @@ export class Rule extends Lint.Rules.AbstractRule { /* tslint:enable:object-literal-sort-keys */ public static FAILURE_STRING_NEVER = "Unnecessary trailing comma"; + public static FAILURE_STRING_FORBIDDEN = "Forbidden trailing comma"; public static FAILURE_STRING_ALWAYS = "Missing trailing comma"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const options = normalizeOptions(this.ruleArguments[0] as Options); + const options = normalizeOptions(this.ruleArguments[0] as OptionsJson); return this.applyWithWalker(new TrailingCommaWalker(sourceFile, this.ruleName, options)); } @@ -133,23 +145,25 @@ class TrailingCommaWalker extends Lint.AbstractWalker { const cb = (node: ts.Node): void => { switch (node.kind) { case ts.SyntaxKind.ArrayLiteralExpression: + this.checkList((node as ts.ArrayLiteralExpression).elements, node.end, "arrays", isArrayRest); + break; case ts.SyntaxKind.ArrayBindingPattern: - this.checkList((node as ts.ArrayLiteralExpression | ts.ArrayBindingPattern).elements, node.end, "arrays"); + this.checkList((node as ts.BindingPattern).elements, node.end, "arrays", isDestructuringRest); break; case ts.SyntaxKind.ObjectBindingPattern: - this.checkList((node as ts.ObjectBindingPattern).elements, node.end, "objects"); + this.checkList((node as ts.BindingPattern).elements, node.end, "objects", isDestructuringRest); break; case ts.SyntaxKind.NamedImports: - this.checkList((node as ts.NamedImports).elements, node.end, "imports"); + this.checkList((node as ts.NamedImports).elements, node.end, "imports", noRest); break; case ts.SyntaxKind.NamedExports: - this.checkList((node as ts.NamedExports).elements, node.end, "exports"); + this.checkList((node as ts.NamedExports).elements, node.end, "exports", noRest); break; case ts.SyntaxKind.ObjectLiteralExpression: - this.checkList((node as ts.ObjectLiteralExpression).properties, node.end, "objects"); + this.checkList((node as ts.ObjectLiteralExpression).properties, node.end, "objects", isObjectRest); break; case ts.SyntaxKind.EnumDeclaration: - this.checkList((node as ts.EnumDeclaration).members, node.end, "objects"); + this.checkList((node as ts.EnumDeclaration).members, node.end, "objects", noRest); break; case ts.SyntaxKind.NewExpression: if ((node as ts.NewExpression).arguments === undefined) { @@ -157,7 +171,7 @@ class TrailingCommaWalker extends Lint.AbstractWalker { } // falls through case ts.SyntaxKind.CallExpression: - this.checkList((node as ts.CallExpression | ts.NewExpression).arguments!, node.end, "functions"); + this.checkList((node as ts.CallExpression | ts.NewExpression).arguments!, node.end, "functions", noRest); break; case ts.SyntaxKind.ArrowFunction: case ts.SyntaxKind.Constructor: @@ -170,11 +184,12 @@ class TrailingCommaWalker extends Lint.AbstractWalker { case ts.SyntaxKind.ConstructorType: case ts.SyntaxKind.FunctionType: case ts.SyntaxKind.CallSignature: - this.checkListWithEndToken( - node, + this.checkList( (node as ts.SignatureDeclaration).parameters, - ts.SyntaxKind.CloseParenToken, - "functions"); + getChildOfKind(node, ts.SyntaxKind.CloseParenToken, this.sourceFile)!.end, + "functions", + isRestParameter, + ); break; case ts.SyntaxKind.TypeLiteral: this.checkTypeLiteral(node as ts.TypeLiteralNode); @@ -200,29 +215,33 @@ class TrailingCommaWalker extends Lint.AbstractWalker { } // The trailing comma is part of the last member and therefore not present as hasTrailingComma on the NodeArray const hasTrailingComma = sourceText[members.end - 1] === ","; - return this.checkComma(hasTrailingComma, members, node.end, "typeLiterals"); + return this.checkComma(hasTrailingComma, members, node.end, "typeLiterals", noRest); } - private checkListWithEndToken(node: ts.Node, list: ts.NodeArray, closeTokenKind: ts.SyntaxKind, optionKey: OptionName) { + private checkList(list: ts.NodeArray, closeElementPos: number, optionKey: OptionName, isRest: (n: T) => boolean) { if (list.length === 0) { return; } - const token = getChildOfKind(node, closeTokenKind, this.sourceFile); - if (token !== undefined) { - return this.checkComma(list.hasTrailingComma, list, token.end, optionKey); - } + return this.checkComma(list.hasTrailingComma, list, closeElementPos, optionKey, isRest); } - private checkList(list: ts.NodeArray, closeElementPos: number, optionKey: OptionName) { - if (list.length === 0) { + /* Expects `list.length !== 0` */ + private checkComma( + hasTrailingComma: boolean | undefined, + list: ts.NodeArray, + closeTokenPos: number, + optionKey: OptionName, + isRest: (node: T) => boolean, + ) { + const last = list[list.length - 1]; + if (this.options.specCompliant && isRest(last)) { + if (hasTrailingComma) { + this.addFailureAt(list.end - 1, 1, Rule.FAILURE_STRING_FORBIDDEN, Lint.Replacement.deleteText(list.end - 1, 1)); + } return; } - return this.checkComma(list.hasTrailingComma, list, closeElementPos, optionKey); - } - /* Expects `list.length !== 0` */ - private checkComma(hasTrailingComma: boolean | undefined, list: ts.NodeArray, closeTokenPos: number, optionKey: OptionName) { - const options = isSameLine(this.sourceFile, list[list.length - 1].end, closeTokenPos) + const options = isSameLine(this.sourceFile, last.end, closeTokenPos) ? this.options.singleline : this.options.multiline; const option = options[optionKey]; @@ -234,3 +253,23 @@ class TrailingCommaWalker extends Lint.AbstractWalker { } } } + +function isRestParameter(node: ts.ParameterDeclaration) { + return node.dotDotDotToken !== undefined; +} + +function isDestructuringRest(node: ts.ArrayBindingElement) { + return node.kind === ts.SyntaxKind.BindingElement && node.dotDotDotToken !== undefined; +} + +function isObjectRest(node: ts.ObjectLiteralElementLike) { + return node.kind === ts.SyntaxKind.SpreadAssignment && isReassignmentTarget(node.expression); +} + +function isArrayRest(node: ts.Expression) { + return node.kind === ts.SyntaxKind.SpreadElement && isReassignmentTarget(node); +} + +function noRest() { + return false; +} diff --git a/test/rules/trailing-comma/multiline-always/test.ts.fix b/test/rules/trailing-comma/multiline-always/test.ts.fix index 38a6cf4013a..83867505978 100644 --- a/test/rules/trailing-comma/multiline-always/test.ts.fix +++ b/test/rules/trailing-comma/multiline-always/test.ts.fix @@ -547,3 +547,51 @@ export { // don't crash new Foo + +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} + +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} diff --git a/test/rules/trailing-comma/multiline-always/test.ts.lint b/test/rules/trailing-comma/multiline-always/test.ts.lint index 77d4f47fee5..54ae5b9be4e 100644 --- a/test/rules/trailing-comma/multiline-always/test.ts.lint +++ b/test/rules/trailing-comma/multiline-always/test.ts.lint @@ -576,3 +576,56 @@ export { // don't crash new Foo + +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} + +let { + ...x + ~nil [Missing trailing comma] +} = {}; +let [ + ...y + ~nil [Missing trailing comma] +] = []; + +[ + ...y + ~nil [Missing trailing comma] +] = []; +({ + ...y + ~nil [Missing trailing comma] +} = {}) +function foo( + ...z + ~nil [Missing trailing comma] +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} diff --git a/test/rules/trailing-comma/multiline-never/test.ts.fix b/test/rules/trailing-comma/multiline-never/test.ts.fix index bd92433c252..9847bac3e8d 100644 --- a/test/rules/trailing-comma/multiline-never/test.ts.fix +++ b/test/rules/trailing-comma/multiline-never/test.ts.fix @@ -496,3 +496,50 @@ const enum Foo { Baz = 2 } +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} diff --git a/test/rules/trailing-comma/multiline-never/test.ts.lint b/test/rules/trailing-comma/multiline-never/test.ts.lint index 910d09ebe9d..f3e0937fa2f 100644 --- a/test/rules/trailing-comma/multiline-never/test.ts.lint +++ b/test/rules/trailing-comma/multiline-never/test.ts.lint @@ -525,3 +525,55 @@ const enum Foo { ~ [Unnecessary trailing comma] } +let { + ...x, + ~ [Unnecessary trailing comma] +} = {}; +let [ + ...y, + ~ [Unnecessary trailing comma] +] = []; + +[ + ...y, + ~ [Unnecessary trailing comma] +] = []; +({ + ...y, + ~ [Unnecessary trailing comma] +} = {}) +function foo( + ...z, + ~ [Unnecessary trailing comma] +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} diff --git a/test/rules/trailing-comma/singleline-always/test.ts.fix b/test/rules/trailing-comma/singleline-always/test.ts.fix index 2aa1e2ec29b..4bdc196a641 100644 --- a/test/rules/trailing-comma/singleline-always/test.ts.fix +++ b/test/rules/trailing-comma/singleline-always/test.ts.fix @@ -538,3 +538,50 @@ const enum Foo { Baz = 2, } +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} diff --git a/test/rules/trailing-comma/singleline-always/test.ts.lint b/test/rules/trailing-comma/singleline-always/test.ts.lint index e2fb321bdda..949b532c9a2 100644 --- a/test/rules/trailing-comma/singleline-always/test.ts.lint +++ b/test/rules/trailing-comma/singleline-always/test.ts.lint @@ -563,3 +563,55 @@ const enum Foo { Baz = 2, } +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x,} = {}; +let [...y,] = []; +[...y,] = []; +({...y,} = {}) +function foo(...z,) {} + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; + ~nil [Missing trailing comma] +let [...y] = []; + ~nil [Missing trailing comma] +[...y] = []; + ~nil [Missing trailing comma] +({...y} = {}) + ~nil [Missing trailing comma] +function foo(...z) {} + ~nil [Missing trailing comma] diff --git a/test/rules/trailing-comma/singleline-never/test.ts.fix b/test/rules/trailing-comma/singleline-never/test.ts.fix index 5198ed5e6d3..de3ae58c3b1 100644 --- a/test/rules/trailing-comma/singleline-never/test.ts.fix +++ b/test/rules/trailing-comma/singleline-never/test.ts.fix @@ -498,3 +498,50 @@ const enum Foo { Baz = 2, } +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} diff --git a/test/rules/trailing-comma/singleline-never/test.ts.lint b/test/rules/trailing-comma/singleline-never/test.ts.lint index e1d597707fb..ba133a56149 100644 --- a/test/rules/trailing-comma/singleline-never/test.ts.lint +++ b/test/rules/trailing-comma/singleline-never/test.ts.lint @@ -521,3 +521,55 @@ const enum Foo { Baz = 2, } +let { + ...x, +} = {}; +let [ + ...y, +] = []; + +[ + ...y, +] = []; +({ + ...y, +} = {}) +function foo( + ...z, +) {} + + +let {...x,} = {}; + ~ [Unnecessary trailing comma] +let [...y,] = []; + ~ [Unnecessary trailing comma] +[...y,] = []; + ~ [Unnecessary trailing comma] +({...y,} = {}) + ~ [Unnecessary trailing comma] +function foo(...z,) {} + ~ [Unnecessary trailing comma] + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} diff --git a/test/rules/trailing-comma/spec-compliant/test.ts.fix b/test/rules/trailing-comma/spec-compliant/test.ts.fix new file mode 100644 index 00000000000..73c83825bb4 --- /dev/null +++ b/test/rules/trailing-comma/spec-compliant/test.ts.fix @@ -0,0 +1,66 @@ +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} + + +[...y] = [...y,]; +({ + ...x +} = { + ...x, +}); +({ + ...x +} = { + ...x, + y, +}) +foo(...y,); + +function combined([a, ...arr], {b, ...obj}, regular,) {} + +function combined([a, ...arr], {b, ...obj}, regular,) {} diff --git a/test/rules/trailing-comma/spec-compliant/test.ts.lint b/test/rules/trailing-comma/spec-compliant/test.ts.lint new file mode 100644 index 00000000000..a2b896d2409 --- /dev/null +++ b/test/rules/trailing-comma/spec-compliant/test.ts.lint @@ -0,0 +1,83 @@ +let { + ...x, + ~ [Forbidden trailing comma] +} = {}; +let [ + ...y, + ~ [Forbidden trailing comma] +] = []; + +[ + ...y, + ~ [Forbidden trailing comma] +] = []; +({ + ...y, + ~ [Forbidden trailing comma] +} = {}) +function foo( + ...z, + ~ [Forbidden trailing comma] +) {} + + +let {...x,} = {}; + ~ [Forbidden trailing comma] +let [...y,] = []; + ~ [Forbidden trailing comma] +[...y,] = []; + ~ [Forbidden trailing comma] +({...y,} = {}) + ~ [Forbidden trailing comma] +function foo(...z,) {} + ~ [Forbidden trailing comma] + +let { + ...x +} = {}; +let [ + ...y +] = []; + +[ + ...y +] = []; +({ + ...y +} = {}) +function foo( + ...z +) {} + + +let {...x} = {}; +let [...y] = []; +[...y] = []; +({...y} = {}) +function foo(...z) {} + + +[...y] = [...y]; + ~nil [Missing trailing comma] +({ + ...x +} = { + ...x + ~nil [Missing trailing comma] +}); +({ + ...x +} = { + ...x, + y + ~nil [Missing trailing comma] +}) +foo(...y); + ~nil [Missing trailing comma] + +function combined([a, ...arr], {b, ...obj}, regular) {} + ~nil [Missing trailing comma] + +function combined([a, ...arr,], {b, ...obj,}, regular,) {} + ~ [Forbidden trailing comma] + ~ [Forbidden trailing comma] diff --git a/test/rules/trailing-comma/spec-compliant/tslint.json b/test/rules/trailing-comma/spec-compliant/tslint.json new file mode 100644 index 00000000000..3005eb3add4 --- /dev/null +++ b/test/rules/trailing-comma/spec-compliant/tslint.json @@ -0,0 +1,12 @@ +{ + "rules": { + "trailing-comma": [ + true, + { + "multiline": "always", + "singleline": "always", + "esSpecCompliant": true + } + ] + } +}