diff --git a/src/rules/objectLiteralSortKeysRule.ts b/src/rules/objectLiteralSortKeysRule.ts index a2fee6afee6..da6664f9925 100644 --- a/src/rules/objectLiteralSortKeysRule.ts +++ b/src/rules/objectLiteralSortKeysRule.ts @@ -15,74 +15,183 @@ * limitations under the License. */ -import { isObjectLiteralExpression, isSameLine } from "tsutils"; +import { isInterfaceDeclaration, isObjectLiteralExpression, isSameLine, isTypeAliasDeclaration, isTypeLiteralNode } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; const OPTION_IGNORE_CASE = "ignore-case"; +const OPTION_MATCH_DECLARATION_ORDER = "match-declaration-order"; interface Options { ignoreCase: boolean; + matchDeclarationOrder: boolean; } -export class Rule extends Lint.Rules.AbstractRule { +export class Rule extends Lint.Rules.OptionallyTypedRule { /* 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", + description: "Checks ordering of keys in object literals.", rationale: "Useful in preventing merge conflicts", - optionsDescription: `You may optionally pass "${OPTION_IGNORE_CASE}" to compare keys case insensitive.`, + optionsDescription: Lint.Utils.dedent` + By default, this rule checks that keys are in alphabetical order. + The following may optionally be passed: + + * "${OPTION_IGNORE_CASE}" will to compare keys in a case insensitive way. + * "${OPTION_MATCH_DECLARATION_ORDER} will prefer to use the key ordering of the contextual type of the object literal, as in: + + interface I { foo: number; bar: number; } + const obj: I = { foo: 1, bar: 2 }; + + If a contextual type is not found, alphabetical ordering will be used instead. + `, options: { type: "string", - enum: [OPTION_IGNORE_CASE], + enum: [OPTION_IGNORE_CASE, OPTION_MATCH_DECLARATION_ORDER], }, optionExamples: [ true, - [true, OPTION_IGNORE_CASE], + [true, OPTION_IGNORE_CASE, OPTION_MATCH_DECLARATION_ORDER], ], type: "maintainability", typescriptOnly: false, }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_STRING_FACTORY(name: string) { + public static FAILURE_STRING_ALPHABETICAL(name: string): string { return `The key '${name}' is not sorted alphabetically`; } + public static FAILURE_STRING_USE_DECLARATION_ORDER(propName: string, typeName: string | undefined): string { + const type = typeName === undefined ? "its type declaration" : `'${typeName}'`; + return `The key '${propName}' is not in the same order as it is in ${type}.`; + } public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, walk, { - ignoreCase: this.ruleArguments.indexOf(OPTION_IGNORE_CASE) !== -1, - }); + const options = parseOptions(this.ruleArguments); + if (options.matchDeclarationOrder) { + throw new Error(`${this.ruleName} needs type info to use "${OPTION_MATCH_DECLARATION_ORDER}".`); + } + return this.applyWithFunction(sourceFile, walk, options); + } + + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithFunction( + sourceFile, (ctx) => walk(ctx, program.getTypeChecker()), parseOptions(this.ruleArguments)); } } -function walk(ctx: Lint.WalkContext) { - return ts.forEachChild(ctx.sourceFile, function cb(node): void { - 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) { - 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; +function parseOptions(ruleArguments: any[]): Options { + return { + ignoreCase: has(OPTION_IGNORE_CASE), + matchDeclarationOrder: has(OPTION_MATCH_DECLARATION_ORDER), + }; + + function has(name: string) { + return ruleArguments.indexOf(name) !== -1; + } +} + +function walk(ctx: Lint.WalkContext, checker?: ts.TypeChecker): void { + const { sourceFile, options: { ignoreCase, matchDeclarationOrder } } = ctx; + ts.forEachChild(sourceFile, function cb(node): void { + if (isObjectLiteralExpression(node) && node.properties.length > 1) { + check(node); + } + ts.forEachChild(node, cb); + }); + + function check(node: ts.ObjectLiteralExpression): void { + if (matchDeclarationOrder) { + const type = getContextualType(node, checker!); + // If type has an index signature, we can't check ordering. + // If type has call/construct signatures, it can't be satisfied by an object literal anyway. + if (type !== undefined + && type.members.every((m) => m.kind === ts.SyntaxKind.PropertySignature || m.kind === ts.SyntaxKind.MethodSignature)) { + checkMatchesDeclarationOrder(node, type, type.members as ReadonlyArray); + return; + } + } + checkAlphabetical(node); + } + + function checkAlphabetical(node: ts.ObjectLiteralExpression): void { + if (isSameLine(ctx.sourceFile, node.properties.pos, node.end)) { + return; + } + + let lastKey: string | undefined; + 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_ALPHABETICAL(property.name.text)); + return; // only show warning on first out-of-order property } + lastKey = key; + } + } + } + } + + function checkMatchesDeclarationOrder( + { properties }: ts.ObjectLiteralExpression, + type: TypeLike, + members: ReadonlyArray<{ name: ts.PropertyName }>): void { + + let memberIndex = 0; + outer: for (const prop of properties) { + if (prop.kind === ts.SyntaxKind.SpreadAssignment) { + memberIndex = 0; + continue; + } + + if (prop.name.kind === ts.SyntaxKind.ComputedPropertyName) { continue; } + + const propName = prop.name.text; + + for (; memberIndex !== members.length; memberIndex++) { + const { name: memberName } = members[memberIndex]; + if (memberName.kind !== ts.SyntaxKind.ComputedPropertyName && propName === memberName.text) { + continue outer; // tslint:disable-line no-unsafe-any (fixed in tslint 5.4) } } + + // This We didn't find the member we were looking for past the previous member, + // so it must have come before it and is therefore out of order. + ctx.addFailureAtNode(prop.name, Rule.FAILURE_STRING_USE_DECLARATION_ORDER(propName, typeName(type))); + // Don't bother with multiple errors. + break; } - return ts.forEachChild(node, cb); - }); + } +} + +function typeName(t: TypeLike): string | undefined { + const parent = t.parent!; + return t.kind === ts.SyntaxKind.InterfaceDeclaration ? t.name.text : isTypeAliasDeclaration(parent) ? parent.name.text : undefined; +} + +type TypeLike = ts.InterfaceDeclaration | ts.TypeLiteralNode; + +function getContextualType(node: ts.Expression, checker: ts.TypeChecker): TypeLike | undefined { + const c = checker.getContextualType(node); + if (c === undefined || c.symbol === undefined) { + return undefined; + } + + const { declarations } = c.symbol; + if (declarations === undefined || declarations.length !== 1) { + return undefined; + } + + const decl = declarations[0]; + return isInterfaceDeclaration(decl) || isTypeLiteralNode(decl) ? decl : undefined; } diff --git a/test/rules/object-literal-sort-keys/match-declaration-order/test.ts.lint b/test/rules/object-literal-sort-keys/match-declaration-order/test.ts.lint new file mode 100644 index 00000000000..3d2079221d1 --- /dev/null +++ b/test/rules/object-literal-sort-keys/match-declaration-order/test.ts.lint @@ -0,0 +1,72 @@ +interface I { + b; + a; +} + +declare function f(i: I): void; + +const a = 0, b = 0; + +f({ b, a }); + +f({ a, b }); + ~ [0 % ('b', 'I')] + +// Resets ordering after spread operator. + +f({ a, ...x, b }); + +f({ a, ...x, a, b }); + ~ [0 % ('b', 'I')] + + +// Methods and getters/setters work like any other key. + +f({ b() {}, a() {} }); + +f({ a() {}, b() {} }); + ~ [0 % ('b', 'I')] + +f({ + get b() {}, + a, + set b(v) {}, + ~ [0 % ('b', 'I')] +}); + +f({ + get b() {}, + set b() {}, + a, +}); + +// Ignores computed properties. Does not ignore string / number keys. + +interface J { + "foo"; + 2; + [Symol.iterator]; +} +declare function j(j: J): void; +j({ [Symbol.iterator]: 1, "foo": 1, 2: 1 }); +j({ [Symbol.iterator]: 1, 2: 1, "foo": 1 }); + ~~~~~ [0 % ('foo', 'J')] + +// Works with anonymous type too. +type T = { b, a }; +const o: T = { a, b }; + ~ [0 % ('b', 'T')] + +const o: { b, a } = { a, b }; + ~ [1 % ('b')] + +// Goes with alphabetical ordering if it can't find a type. + +const o = { + b, + a, + ~ [The key 'a' is not sorted alphabetically] +}; + +[0]: The key '%s' is not in the same order as it is in '%s'. +[1]: The key '%s' is not in the same order as it is in its type declaration. diff --git a/test/rules/object-literal-sort-keys/match-declaration-order/tsconfig.json b/test/rules/object-literal-sort-keys/match-declaration-order/tsconfig.json new file mode 100644 index 00000000000..9e26dfeeb6e --- /dev/null +++ b/test/rules/object-literal-sort-keys/match-declaration-order/tsconfig.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/test/rules/object-literal-sort-keys/match-declaration-order/tslint.json b/test/rules/object-literal-sort-keys/match-declaration-order/tslint.json new file mode 100644 index 00000000000..c4d505791fe --- /dev/null +++ b/test/rules/object-literal-sort-keys/match-declaration-order/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "object-literal-sort-keys": [true, "ignore-case", "match-declaration-order"] + } +}