Refactor no-string-literal and add fixer (palantir#2495)

package.json

@@ -44,7 +44,7 @@
     "optimist": "~0.6.0",
     "resolve": "^1.3.2",
     "semver": "^5.3.0",
-    "tsutils": "^1.4.0"
+    "tsutils": "^1.6.0"
   },
   "peerDependencies": {
     "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev"

src/rules/noStringLiteralRule.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { isElementAccessExpression, isStringLiteral, isValidPropertyAccess } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -30,40 +31,30 @@ export class Rule extends Lint.Rules.AbstractRule {
         optionExamples: [true],
         type: "functionality",
         typescriptOnly: false,
+        hasFix: true,
     };
     /* tslint:enable:object-literal-sort-keys */
 
     public static FAILURE_STRING = "object access via string literals is disallowed";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoStringLiteralWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoStringLiteralWalker extends Lint.RuleWalker {
-    public visitElementAccessExpression(node: ts.ElementAccessExpression) {
-        const argument = node.argumentExpression;
-        if (argument != null) {
-            const accessorText = argument.getText();
-
-            // the argument expression should be a string of length at least 2 (due to quote characters)
-            if (argument.kind === ts.SyntaxKind.StringLiteral && accessorText.length > 2) {
-                const unquotedAccessorText = accessorText.substring(1, accessorText.length - 1);
-
-                // only create a failure if the identifier is valid, in which case there's no need to use string literals
-                if (isValidIdentifier(unquotedAccessorText)) {
-                    this.addFailureAtNode(argument, Rule.FAILURE_STRING);
-                }
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (isElementAccessExpression(node)) {
+            const argument = node.argumentExpression;
+            if (argument !== undefined && isStringLiteral(argument) && isValidPropertyAccess(argument.text)) {
+                ctx.addFailureAtNode(
+                    argument,
+                    Rule.FAILURE_STRING,
+                    // expr['foo'] -> expr.foo
+                    Lint.Replacement.replaceFromTo(node.expression.end, node.end, "." + argument.text),
+                );
             }
         }
-
-        super.visitElementAccessExpression(node);
-    }
-}
-
-function isValidIdentifier(token: string) {
-    const scanner = ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, token);
-    scanner.scan();
-    // if we scanned to the end of the token, we can check if the scanned item was an identifier
-    return scanner.getTokenText() === token && scanner.isIdentifier();
+        return ts.forEachChild(node, cb);
+    });
 }

test/rules/no-string-literal/test.ts.fix

@@ -0,0 +1,26 @@
+var obj = {
+    a: 1,
+    b: 2,
+    c: 3,
+    d: 4
+};
+
+function test() {
+    var a = obj.a;
+      var b = obj.bcd;
+        var c = obj.c;
+    var d = obj[b];
+}
+
+obj.invalid_accessor;
+obj._AnotherInvalidAccessor;
+obj.a; // unicode value "a"
+
+// valid accessors
+obj["a-2"];
+obj["2a"];
+obj["?a#$!$^&%&"];
+
+// invalid accessor - no crash
+obj[]
+

test/rules/no-string-literal/test.ts.lint

@@ -18,6 +18,8 @@ obj["invalid_accessor"];
     ~~~~~~~~~~~~~~~~~~   [0]
 obj["_AnotherInvalidAccessor"];
     ~~~~~~~~~~~~~~~~~~~~~~~~~   [0]
+obj["\u0061"]; // unicode value "a"
+    ~~~~~~~~ [0]
 
 // valid accessors
 obj["a-2"];