From 4061b66ea5ae951770981dbb42c4b56b69575579 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 16 Apr 2017 23:00:44 +0200 Subject: [PATCH 1/3] object-literal-sort-keys: rewrite, handle spread and shorthand [new-rule-option] `object-literal-sort-keys` adds `ignore-case` Fixes: #1924 [enhancement] `object-literal-sort-keys`: check shorthand properties Fixes: #2205 [bugfix] `object-literal-sort-keys`: handle object spread correctly Fixes: #2554 --- src/rules/objectLiteralSortKeysRule.ts | 96 +++++++++---------- src/rules/quotemarkRule.ts | 2 +- .../{ => default}/test.ts.lint | 55 ++++++++--- .../{ => default}/tslint.json | 0 .../ignore-case/test.ts.lint | 12 +++ .../ignore-case/tslint.json | 5 + 6 files changed, 107 insertions(+), 63 deletions(-) rename test/rules/object-literal-sort-keys/{ => default}/test.ts.lint (67%) rename test/rules/object-literal-sort-keys/{ => default}/tslint.json (100%) create mode 100644 test/rules/object-literal-sort-keys/ignore-case/test.ts.lint create mode 100644 test/rules/object-literal-sort-keys/ignore-case/tslint.json diff --git a/src/rules/objectLiteralSortKeysRule.ts b/src/rules/objectLiteralSortKeysRule.ts index db6b2f92c48..81dbb697cdf 100644 --- a/src/rules/objectLiteralSortKeysRule.ts +++ b/src/rules/objectLiteralSortKeysRule.ts @@ -15,19 +15,32 @@ * limitations under the License. */ +import { isObjectLiteralExpression } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +const OPTION_IGNORE_CASE = "ignore-case"; + +interface Options { + ignoreCase: boolean; +} + export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { ruleName: "object-literal-sort-keys", description: "Requires keys in object literals to be sorted alphabetically", rationale: "Useful in preventing merge conflicts", - optionsDescription: "Not configurable.", - options: null, - optionExamples: [true], + optionsDescription: `You may optionally pass "${OPTION_IGNORE_CASE}" to compare keys case insensitive.`, + options: { + type: "string", + enum: [OPTION_IGNORE_CASE], + }, + optionExamples: [ + true, + [true, OPTION_IGNORE_CASE], + ], type: "maintainability", typescriptOnly: false, }; @@ -38,59 +51,42 @@ export class Rule extends Lint.Rules.AbstractRule { } public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ObjectLiteralSortKeysWalker(sourceFile, this.getOptions())); + return this.applyWithFunction(sourceFile, walk, { + ignoreCase: this.ruleArguments.indexOf(OPTION_IGNORE_CASE) !== -1, + }); } } -class ObjectLiteralSortKeysWalker extends Lint.RuleWalker { - // stacks are used to maintain state while recursing through nested object literals - private lastSortedKeyStack: string[] = []; - private multilineFlagStack: boolean[] = []; - private sortedStateStack: boolean[] = []; - - public visitObjectLiteralExpression(node: ts.ObjectLiteralExpression) { - // char code 0; every string should be >= to this - this.lastSortedKeyStack.push(""); - // sorted state is always initially true - this.sortedStateStack.push(true); - this.multilineFlagStack.push(this.isMultilineListNode(node)); - super.visitObjectLiteralExpression(node); - this.multilineFlagStack.pop(); - this.lastSortedKeyStack.pop(); - this.sortedStateStack.pop(); - } - - public visitPropertyAssignment(node: ts.PropertyAssignment) { - const sortedState = this.sortedStateStack[this.sortedStateStack.length - 1]; - const isMultiline = this.multilineFlagStack[this.multilineFlagStack.length - 1]; - - // skip remainder of object literal scan if a previous key was found - // in an unsorted position. This ensures only one error is thrown at - // a time and keeps error output clean. Skip also single line objects. - if (sortedState && isMultiline) { - const lastSortedKey = this.lastSortedKeyStack[this.lastSortedKeyStack.length - 1]; - const keyNode = node.name; - if (isIdentifierOrStringLiteral(keyNode)) { - const key = keyNode.text; - if (key < lastSortedKey) { - const failureString = Rule.FAILURE_STRING_FACTORY(key); - this.addFailureAtNode(keyNode, failureString); - this.sortedStateStack[this.sortedStateStack.length - 1] = false; - } else { - this.lastSortedKeyStack[this.lastSortedKeyStack.length - 1] = key; +function walk(ctx: Lint.WalkContext) { + return ts.forEachChild(ctx.sourceFile, function cb(node): void { + if (isObjectLiteralExpression(node) && node.properties.length > 1 && isMultiline(node, ctx.sourceFile)) { + let lastKey: string | undefined; + const {options: {ignoreCase}} = ctx; + outer: for (const property of node.properties) { + switch (property.kind) { + case ts.SyntaxKind.SpreadAssignment: + lastKey = undefined; // reset at spread + break; + case ts.SyntaxKind.ShorthandPropertyAssignment: + case ts.SyntaxKind.PropertyAssignment: + if (property.name.kind === ts.SyntaxKind.Identifier || + property.name.kind === ts.SyntaxKind.StringLiteral) { + const key = ignoreCase ? property.name.text.toLowerCase() : property.name.text; + // comparison with undefined is expected + if (lastKey! > key) { + ctx.addFailureAtNode(property.name, Rule.FAILURE_STRING_FACTORY(property.name.text)); + break outer; // only show warning on first out-of-order property + } + lastKey = key; + } } } } - super.visitPropertyAssignment(node); - } - - private isMultilineListNode(node: ts.ObjectLiteralExpression) { - const startLineOfNode = this.getLineAndCharacterOfPosition(node.getStart()).line; - const endLineOfNode = this.getLineAndCharacterOfPosition(node.getEnd()).line; - return endLineOfNode !== startLineOfNode; - } + return ts.forEachChild(node, cb); + }); } -function isIdentifierOrStringLiteral(node: ts.Node): node is (ts.Identifier | ts.StringLiteral) { - return node.kind === ts.SyntaxKind.Identifier || node.kind === ts.SyntaxKind.StringLiteral; +function isMultiline(node: ts.ObjectLiteralExpression, sourceFile: ts.SourceFile): boolean { + return ts.getLineAndCharacterOfPosition(sourceFile, node.properties[0].pos - 1).line + !== ts.getLineAndCharacterOfPosition(sourceFile, node.end).line; } diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts index a6893978ef5..617cf53fb3a 100644 --- a/src/rules/quotemarkRule.ts +++ b/src/rules/quotemarkRule.ts @@ -77,11 +77,11 @@ export class Rule extends Lint.Rules.AbstractRule { const args = this.ruleArguments; const quoteMark = args[0] === OPTION_SINGLE ? "'" : '"'; return this.applyWithFunction(sourceFile, walk, { - quoteMark, avoidEscape: args.indexOf(OPTION_AVOID_ESCAPE) !== -1, jsxQuoteMark: args.indexOf(OPTION_JSX_SINGLE) !== -1 ? "'" : args.indexOf(OPTION_JSX_DOUBLE) ? '"' : quoteMark, + quoteMark, }); } } diff --git a/test/rules/object-literal-sort-keys/test.ts.lint b/test/rules/object-literal-sort-keys/default/test.ts.lint similarity index 67% rename from test/rules/object-literal-sort-keys/test.ts.lint rename to test/rules/object-literal-sort-keys/default/test.ts.lint index 5668b48773e..3f92252d945 100644 --- a/test/rules/object-literal-sort-keys/test.ts.lint +++ b/test/rules/object-literal-sort-keys/default/test.ts.lint @@ -6,7 +6,7 @@ var passA = { var failA = { b: 1, a: 2 - ~ [The key 'a' is not sorted alphabetically] + ~ [err % ('a')] }; var passB = { @@ -19,7 +19,7 @@ var passB = { var failB = { c: 3, a: 1, - ~ [The key 'a' is not sorted alphabetically] + ~ [err % ('a')] b: 2, d: 4 }; @@ -37,7 +37,7 @@ var failC = { b: { bb: 2, aa: 1 - ~~ [The key 'aa' is not sorted alphabetically] + ~~ [err % ('aa')] } }; @@ -57,7 +57,7 @@ var failD = { bb: 2 }, b: 3 - ~ [The key 'b' is not sorted alphabetically] + ~ [err % ('b')] }; var passE = {}; @@ -70,7 +70,7 @@ var passF = { var failF = { sdfa: {}, asdf: [1, 2, 3] - ~~~~ [The key 'asdf' is not sorted alphabetically] + ~~~~ [err % ('asdf')] }; var passG = { @@ -81,7 +81,7 @@ var passG = { var failG = { sdafn: function () {}, asdfn: function () {} - ~~~~~ [The key 'asdfn' is not sorted alphabetically] + ~~~~~ [err % ('asdfn')] }; var passH = { @@ -93,7 +93,7 @@ var passH = { var failH = { 'b': 2, a: 1, - ~ [The key 'a' is not sorted alphabetically] + ~ [err % ('a')] c: 3 } @@ -106,7 +106,7 @@ var passI = { var failI = { À: 2, 'Z': 1, - ~~~ [The key 'Z' is not sorted alphabetically] + ~~~ [err % ('Z')] è: 3, } @@ -122,7 +122,7 @@ var failJ = { 2: 4, A: 3, '11': 2 - ~~~~ [The key '11' is not sorted alphabetically] + ~~~~ [err % ('11')] } var passK = { @@ -138,10 +138,10 @@ var failK = { 'b': { e: 5, 'd': 4 - ~~~ [The key 'd' is not sorted alphabetically] + ~~~ [err % ('d')] }, a: 1, - ~ [The key 'a' is not sorted alphabetically] + ~ [err % ('a')] c: 3 } @@ -150,7 +150,7 @@ var passL = {z: 1, y: '1', x: [1, 2]}; var failL = {x: 1, y: { b: 1, a: 2 - ~ [The key 'a' is not sorted alphabetically] + ~ [err % ('a')] }, z: [1, 2]}; var passM = { @@ -161,3 +161,34 @@ var passM = { }, z: {z: 1, y: '1', x: [1, 2]} }; + +const spread = { + c, + ...x, + b: b, + a, + ~ [err % ('a')] +}; + +const numbers = { + 1: true, + 2: true, + 100: true, + 1e4: true, + 2e-1: true, +} + +const a = { + array: [], + objList: [{}, {}], + object: {}, +} + +const b = { + array: [], + object: {}, + objList: [{}, {}], + ~~~~~~~ [err % ('objList')] +} + +[err]: The key '%s' is not sorted alphabetically \ No newline at end of file diff --git a/test/rules/object-literal-sort-keys/tslint.json b/test/rules/object-literal-sort-keys/default/tslint.json similarity index 100% rename from test/rules/object-literal-sort-keys/tslint.json rename to test/rules/object-literal-sort-keys/default/tslint.json diff --git a/test/rules/object-literal-sort-keys/ignore-case/test.ts.lint b/test/rules/object-literal-sort-keys/ignore-case/test.ts.lint new file mode 100644 index 00000000000..b706a1a6145 --- /dev/null +++ b/test/rules/object-literal-sort-keys/ignore-case/test.ts.lint @@ -0,0 +1,12 @@ +const a = { + array: [], + objList: [{}, {}], + object: {}, + ~~~~~~ [The key 'object' is not sorted alphabetically] +} + +const b = { + array: [], + object: {}, + objList: [{}, {}], +} diff --git a/test/rules/object-literal-sort-keys/ignore-case/tslint.json b/test/rules/object-literal-sort-keys/ignore-case/tslint.json new file mode 100644 index 00000000000..0e35ccdd33c --- /dev/null +++ b/test/rules/object-literal-sort-keys/ignore-case/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "object-literal-sort-keys": [true, "ignore-case"] + } +} From 3c43848ba59ae12a338b11b77a665ec6dcd5321f Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Tue, 18 Apr 2017 20:53:33 +0200 Subject: [PATCH 2/3] Use isSameLine helper --- package.json | 4 ++-- src/rules/objectLiteralSortKeysRule.ts | 10 +++------- yarn.lock | 6 +++--- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 6ef99768e97..ceb55cd6d4b 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "optimist": "~0.6.0", "resolve": "^1.3.2", "semver": "^5.3.0", - "tsutils": "^1.6.0" + "tsutils": "^1.7.0" }, "peerDependencies": { "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev" @@ -71,7 +71,7 @@ "npm-run-all": "^3.1.0", "nyc": "^10.2.0", "rimraf": "^2.5.4", - "tslint": "latest", + "tslint": "^5.1.0", "tslint-test-config-non-relative": "file:test/external/tslint-test-config-non-relative", "typescript": "^2.3.0" }, diff --git a/src/rules/objectLiteralSortKeysRule.ts b/src/rules/objectLiteralSortKeysRule.ts index 81dbb697cdf..a9260494ca6 100644 --- a/src/rules/objectLiteralSortKeysRule.ts +++ b/src/rules/objectLiteralSortKeysRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { isObjectLiteralExpression } from "tsutils"; +import { isObjectLiteralExpression, isSameLine } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -59,7 +59,8 @@ export class Rule extends Lint.Rules.AbstractRule { function walk(ctx: Lint.WalkContext) { return ts.forEachChild(ctx.sourceFile, function cb(node): void { - if (isObjectLiteralExpression(node) && node.properties.length > 1 && isMultiline(node, ctx.sourceFile)) { + if (isObjectLiteralExpression(node) && node.properties.length > 1 && + !isSameLine(ctx.sourceFile, node.properties.pos, node.end)) { let lastKey: string | undefined; const {options: {ignoreCase}} = ctx; outer: for (const property of node.properties) { @@ -85,8 +86,3 @@ function walk(ctx: Lint.WalkContext) { return ts.forEachChild(node, cb); }); } - -function isMultiline(node: ts.ObjectLiteralExpression, sourceFile: ts.SourceFile): boolean { - return ts.getLineAndCharacterOfPosition(sourceFile, node.properties[0].pos - 1).line - !== ts.getLineAndCharacterOfPosition(sourceFile, node.end).line; -} diff --git a/yarn.lock b/yarn.lock index 66546498424..d661088106d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1450,9 +1450,9 @@ tslint@latest: semver "^5.3.0" tsutils "^1.4.0" -tsutils@^1.4.0, tsutils@^1.6.0: - version "1.6.0" - resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-1.6.0.tgz#1fd7fac2a61369ed99cd3997f0fbb437128850f2" +tsutils@^1.4.0, tsutils@^1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-1.7.0.tgz#2e63ccc2d6912bb095f7e363ff4100721dc86f50" type-detect@0.1.1: version "0.1.1" From 9a4409d94340f8e77c8703934fd2d5767bba8186 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Tue, 18 Apr 2017 21:16:19 +0200 Subject: [PATCH 3/3] fix package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ceb55cd6d4b..91ff692453d 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "npm-run-all": "^3.1.0", "nyc": "^10.2.0", "rimraf": "^2.5.4", - "tslint": "^5.1.0", + "tslint": "latest", "tslint-test-config-non-relative": "file:test/external/tslint-test-config-non-relative", "typescript": "^2.3.0" },