member-ordering: Add 'alphabetize' option (palantir#2101)

src/rules/memberOrderingRule.ts

@@ -19,6 +19,7 @@ import * as ts from "typescript";
 import * as Lint from "../index";
 
 const OPTION_ORDER = "order";
+const OPTION_ALPHABETIZE = "alphabetize";
 
 enum MemberKind {
     publicStaticField,
@@ -118,7 +119,9 @@ const optionsDescription = Lint.Utils.dedent`
                 "public-static-method",
                 "protected-static-method"
             ]
-        }`;
+        }
+
+    The '${OPTION_ALPHABETIZE}' option will enforce that members within the same category should be alphabetically sorted by name.`;
 
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
@@ -178,18 +181,26 @@ export class Rule extends Lint.Rules.AbstractRule {
         type: "typescript",
         typescriptOnly: true,
     };
+
+    public static FAILURE_STRING_ALPHABETIZE(prevName: string, curName: string) {
+        return `${show(curName)} should come alphabetically before ${show(prevName)}`;
+        function show(s: string) {
+            return s === "" ? "Computed property" : `'${s}'`;
+        }
+    }
+
     /* tslint:enable:object-literal-sort-keys */
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         return this.applyWithWalker(new MemberOrderingWalker(sourceFile, this.getOptions()));
     }
 }
 
 export class MemberOrderingWalker extends Lint.RuleWalker {
-    private readonly order: MemberCategory[];
+    private readonly opts: Options;
 
     constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
         super(sourceFile, options);
-        this.order = getOrder(this.getOptions());
+        this.opts = parseOptions(this.getOptions());
     }
 
     public visitClassDeclaration(node: ts.ClassDeclaration) {
@@ -214,6 +225,7 @@ export class MemberOrderingWalker extends Lint.RuleWalker {
 
     private visitMembers(members: Member[]) {
         let prevRank = -1;
+        let prevName: string | undefined;
         for (const member of members) {
             const rank = this.memberRank(member);
             if (rank === -1) {
@@ -232,12 +244,41 @@ export class MemberOrderingWalker extends Lint.RuleWalker {
                     `Instead, this should come ${locationHint}.`;
                 this.addFailureAtNode(member, errorLine1);
             } else {
+                if (this.opts.alphabetize && member.name) {
+                    if (rank !== prevRank) {
+                        // No alphabetical ordering between different ranks
+                        prevName = undefined;
+                    }
+
+                    const curName = nameString(member.name);
+                    if (prevName !== undefined && caseInsensitiveLess(curName, prevName)) {
+                        this.addFailureAtNode(member.name,
+                            Rule.FAILURE_STRING_ALPHABETIZE(this.findLowerName(members, rank, curName), curName));
+                    } else {
+                        prevName = curName;
+                    }
+                }
+
                 // keep track of last good node
                 prevRank = rank;
             }
         }
     }
 
+    /** Finds the lowest name higher than 'targetName'. */
+    private findLowerName(members: Member[], targetRank: Rank, targetName: string): string {
+        for (const member of members) {
+            if (!member.name || this.memberRank(member) !== targetRank) {
+                continue;
+            }
+            const name = nameString(member.name);
+            if (caseInsensitiveLess(targetName, name)) {
+                return name;
+            }
+        }
+        throw new Error("Expected to find a name");
+    }
+
     /** Finds the highest existing rank lower than `targetRank`. */
     private findLowerRank(members: Member[], targetRank: Rank): Rank | -1 {
         let max: Rank | -1 = -1;
@@ -255,14 +296,18 @@ export class MemberOrderingWalker extends Lint.RuleWalker {
         if (optionName === undefined) {
             return -1;
         }
-        return this.order.findIndex((category) => category.has(optionName));
+        return this.opts.order.findIndex((category) => category.has(optionName));
     }
 
     private rankName(rank: Rank): string {
-        return this.order[rank].name;
+        return this.opts.order[rank].name;
     }
 }
 
+function caseInsensitiveLess(a: string, b: string) {
+    return a.toLowerCase() < b.toLowerCase();
+}
+
 function memberKindForConstructor(access: Access): MemberKind {
     return (MemberKind as any)[access + "Constructor"];
 }
@@ -329,23 +374,30 @@ type Rank = number;
 
 type Access = "public" | "protected" | "private";
 
-function getOrder(options: any[]): MemberCategory[] {
-    return getOrderJson(options).map((cat) => typeof cat === "string"
+interface Options {
+    order: MemberCategory[];
+    alphabetize: boolean;
+}
+
+function parseOptions(options: any[]): Options {
+    const { order: orderJson, alphabetize } = getOptionsJson(options);
+    const order = orderJson.map((cat) => typeof cat === "string"
         ? new MemberCategory(cat.replace(/-/g, " "), new Set(memberKindFromName(cat)))
         : new MemberCategory(cat.name, new Set(flatMap(cat.kinds, memberKindFromName))));
+    return { order, alphabetize };
 }
-function getOrderJson(allOptions: any[]): MemberCategoryJson[] {
+function getOptionsJson(allOptions: any[]): { order: MemberCategoryJson[], alphabetize: boolean } {
     if (allOptions == null || allOptions.length === 0 || allOptions[0] == null) {
         throw new Error("Got empty options");
     }
 
     const firstOption = allOptions[0];
     if (typeof firstOption !== "object") {
         // Undocumented direct string option. Deprecate eventually.
-        return convertFromOldStyleOptions(allOptions); // presume it to be string[]
+        return { order: convertFromOldStyleOptions(allOptions), alphabetize: false }; // presume allOptions to be string[]
     }
 
-    return categoryFromOption(firstOption[OPTION_ORDER]);
+    return { order: categoryFromOption(firstOption[OPTION_ORDER]), alphabetize: !!firstOption[OPTION_ALPHABETIZE] };
 }
 function categoryFromOption(orderOption: {}): MemberCategoryJson[] {
     if (Array.isArray(orderOption)) {
@@ -416,6 +468,17 @@ function isFunctionLiteral(node: ts.Node | undefined) {
     }
 }
 
+function nameString(name: ts.PropertyName): string {
+    switch (name.kind) {
+        case ts.SyntaxKind.Identifier:
+        case ts.SyntaxKind.StringLiteral:
+        case ts.SyntaxKind.NumericLiteral:
+            return (name as ts.Identifier | ts.LiteralExpression).text;
+        default:
+            return "";
+    }
+}
+
 function mapDefined<T, U>(inputs: T[], getOutput: (input: T) => U | undefined): U[] {
     const out = [];
     for (const input of inputs) {

test/rules/member-ordering/alphabetize/test.ts.lint

@@ -0,0 +1,25 @@
+class X {
+    // Different categories have alphabetization together.
+    bar: number;
+    foo: string;
+
+    [Symbol.iterator]() {}
+    // No ordering among computed properties
+    [Symbol.alpherator]() {}
+
+    0() {}
+    2() {}
+    1() {}
+    ~ ['1' should come alphabetically before '2']
+
+    foo() {}
+    "goo"() {}
+    Hoo() {}
+    ioo() {}
+    bar() {}
+    ~~~ ['bar' should come alphabetically before 'foo']
+
+    // Computed properties must go at the beginning.
+    [Symbol.zeterator]() {}
+    ~~~~~~~~~~~~~~~~~~ [Computed property should come alphabetically before '0']
+}

test/rules/member-ordering/alphabetize/tslint.json

@@ -0,0 +1,8 @@
+{
+    "rules": {
+        "member-ordering": [true, {
+            "order": "fields-first",
+            "alphabetize": true
+        }]
+    }
+}