Refactor object-literal-key-quotes rule (#1874)

palantir · Dec 18, 2016 · 552ad84 · 552ad84
1 parent 4677686
commit 552ad84
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 62 deletions.
diff --git a/src/rules/objectLiteralKeyQuotesRule.ts b/src/rules/objectLiteralKeyQuotesRule.ts
@@ -76,85 +76,81 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 }
 
-// This is simplistic. See https://mothereff.in/js-properties for the gorey details.
-const IDENTIFIER_NAME_REGEX = /^(?:[\$A-Z_a-z])+$/;
-const NUMBER_REGEX = /^[0-9]+$/;
 type QuotesMode = "always" | "as-needed" | "consistent" | "consistent-as-needed";
 
-interface IObjectLiteralState {
-    // potential failures for properties that have quotes but don't need them
-    quotesNotNeededProperties: Lint.RuleFailure[];
-    // potential failures for properties that don't have quotes
-    unquotedProperties: Lint.RuleFailure[];
-    // whether or not any of the properties require quotes
-    hasQuotesNeededProperty: boolean;
-}
-
 class ObjectLiteralKeyQuotesWalker extends Lint.RuleWalker {
     private mode: QuotesMode;
-    private currentState: IObjectLiteralState;
 
     constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
         super(sourceFile, options);
-
         this.mode = this.getOptions()[0] || "always";
     }
 
-    public visitPropertyAssignment(node: ts.PropertyAssignment) {
-        const name = node.name;
-        if (name.kind !== ts.SyntaxKind.StringLiteral &&
-            name.kind !== ts.SyntaxKind.ComputedPropertyName) {
-
-            const errorText = Rule.UNQUOTED_PROPERTY(name.getText());
-            this.currentState.unquotedProperties.push(this.createFailure(name.getStart(), name.getWidth(), errorText));
-        }
-        if (name.kind === ts.SyntaxKind.StringLiteral) {
-            // Check if the quoting is necessary.
-            const stringNode = name as ts.StringLiteral;
-            const property = stringNode.text;
-
-            const isIdentifier = IDENTIFIER_NAME_REGEX.test(property);
-            const isNumber = NUMBER_REGEX.test(property);
-            if (isIdentifier || (isNumber && Number(property).toString() === property)) {
-                const errorText = Rule.UNNEEDED_QUOTES(property);
-                const failure = this.createFailure(stringNode.getStart(), stringNode.getWidth(), errorText);
-                this.currentState.quotesNotNeededProperties.push(failure);
-            } else {
-                this.currentState.hasQuotesNeededProperty = true;
-            }
-        }
-
-        super.visitPropertyAssignment(node);
-    }
-
     public visitObjectLiteralExpression(node: ts.ObjectLiteralExpression) {
-        let state: IObjectLiteralState = {
-            hasQuotesNeededProperty: false,
-            quotesNotNeededProperties: [],
-            unquotedProperties: [],
-        };
-        // a nested object literal should store its parent state to restore when finished
-        let previousState = this.currentState;
-        this.currentState = state;
+        const { properties } = node;
+        switch (this.mode) {
+            case "always":
+                this.allMustHaveQuotes(properties);
+                break;
+            case "as-needed":
+                this.noneMayHaveQuotes(properties);
+                break;
+            case "consistent":
+                if (quotesAreInconsistent(properties)) {
+                    this.addFailureAt(node.getStart(), 1, Rule.INCONSISTENT_PROPERTY);
+                }
+                break;
+            case "consistent-as-needed":
+                if (properties.some(({ name }) => name.kind === ts.SyntaxKind.StringLiteral && propertyNeedsQuotes(name.text))) {
+                    this.allMustHaveQuotes(properties);
+                } else {
+                    this.noneMayHaveQuotes(properties, true);
+                }
+                break;
+            default:
+                break;
+        }
 
         super.visitObjectLiteralExpression(node);
+    }
 
-        if (this.mode === "always" || (this.mode === "consistent-as-needed" && state.hasQuotesNeededProperty)) {
-            for (const failure of state.unquotedProperties) {
-                this.addFailure(failure);
+    private allMustHaveQuotes(properties: ts.ObjectLiteralElementLike[]) {
+        for (const { name } of properties) {
+            if (name.kind !== ts.SyntaxKind.StringLiteral && name.kind !== ts.SyntaxKind.ComputedPropertyName) {
+                this.addFailureAtNode(name, Rule.UNQUOTED_PROPERTY(name.getText()));
             }
-        } else if (this.mode === "as-needed" || (this.mode === "consistent-as-needed" && !state.hasQuotesNeededProperty)) {
-            for (const failure of state.quotesNotNeededProperties) {
-                this.addFailure(failure);
-            }
-        } else if (this.mode === "consistent") {
-            const hasQuotedProperties = state.hasQuotesNeededProperty || state.quotesNotNeededProperties.length > 0;
-            const hasUnquotedProperties = state.unquotedProperties.length > 0;
-            if (hasQuotedProperties && hasUnquotedProperties) {
-                this.addFailureAt(node.getStart(), 1, Rule.INCONSISTENT_PROPERTY);
+        }
+    }
+
+    private noneMayHaveQuotes(properties: ts.ObjectLiteralElementLike[], noneNeedQuotes?: boolean) {
+        for (const { name } of properties) {
+            if (name.kind === ts.SyntaxKind.StringLiteral && (noneNeedQuotes || !propertyNeedsQuotes(name.text))) {
+                this.addFailureAtNode(name, Rule.UNNEEDED_QUOTES(name.text));
             }
         }
+    }
+}
 
-        this.currentState = previousState;
+function quotesAreInconsistent(properties: ts.ObjectLiteralElementLike[]): boolean {
+    let propertiesAreQuoted: boolean | undefined; // inferred on first (non-computed) property
+    for (const { name: { kind } } of properties) {
+        if (kind === ts.SyntaxKind.ComputedPropertyName) {
+            continue;
+        }
+        const thisOneIsQuoted = kind === ts.SyntaxKind.StringLiteral;
+        if (propertiesAreQuoted === undefined) {
+            propertiesAreQuoted = thisOneIsQuoted;
+        } else if (propertiesAreQuoted !== thisOneIsQuoted) {
+            return true;
+        }
     }
+    return false;
+}
+
+function propertyNeedsQuotes(property: string): boolean {
+    return !(IDENTIFIER_NAME_REGEX.test(property) || NUMBER_REGEX.test(property) && Number(property).toString() === property);
 }
+
+// This is simplistic. See https://mothereff.in/js-properties for the gorey details.
+const IDENTIFIER_NAME_REGEX = /^(?:[\$A-Z_a-z])+$/;
+const NUMBER_REGEX = /^[0-9]+$/;
diff --git a/test/rules/object-literal-key-quotes/always/test.js.lint b/test/rules/object-literal-key-quotes/always/test.js.lint
@@ -16,6 +16,7 @@ const o = {
   '010': 'but this one does.',
   '.123': 'as does this one',
   fn() { return },
+  ~~               [Unquoted property 'fn' found.]
   true: 0, // failure
   ~~~~  [Unquoted property 'true' found.]
   "0x0": 0,

diff --git a/test/rules/object-literal-key-quotes/always/test.ts.lint b/test/rules/object-literal-key-quotes/always/test.ts.lint
@@ -16,6 +16,7 @@ const o = {
   '010': 'but this one does.',
   '.123': 'as does this one',
   fn() { return },
+  ~~               [Unquoted property 'fn' found.]
   true: 0, // failure
   ~~~~  [Unquoted property 'true' found.]
   "0x0": 0,