Turn quoted access to declared properties into dotted access.

TypeScript allows accessing properties with an element access expression, as
long as the argument is a string literal:

    const x = { y: number; }
    x['y']; // legal

This change turns all those accesses into dotted accesses, to make sure they are
consistently renamed by Closure Compiler.

    const x = { y: number; }
    x.y; // changed to use dotted access

Author: mprobst

src/tsickle.ts

@@ -762,13 +762,39 @@ class Annotator extends ClosureRewriter {
           return true;
         }
         break;
+      case ts.SyntaxKind.ElementAccessExpression:
+        // Convert quoted accesses to properties that have a symbol to dotted accesses, to get
+        // consistent Closure renaming. This can happen because TS allows quoted access with literal
+        // strings to properties.
+        const eae = node as ts.ElementAccessExpression;
+        if (!eae.argumentExpression ||
+            eae.argumentExpression.kind !== ts.SyntaxKind.StringLiteral) {
+          return false;
+        }
+        const quotedPropSym = this.typeChecker.getSymbolAtLocation(eae.argumentExpression);
+        // If it has a symbol, it's actually a regular declared property.
+        if (!quotedPropSym) return false;
+        const propName = (eae.argumentExpression as ts.StringLiteral).text;
+        this.writeNode(eae.expression);
+        this.emit(`.${propName}`);
+        return true;
       case ts.SyntaxKind.PropertyAccessExpression:
         // Convert dotted accesses to types that have an index type declared to quoted accesses, to
         // avoid Closure renaming one access but not the other.
         // This can happen because TS allows dotted access to string index types.
         const pae = node as ts.PropertyAccessExpression;
         const t = this.typeChecker.getTypeAtLocation(pae.expression);
         if (!t.getStringIndexType()) return false;
+        // Types can have string index signatures and declared properties (of the matching type).
+        // These properties have a symbol, as opposed to pure string index types.
+        const propSym = this.typeChecker.getSymbolAtLocation(pae.name);
+        // The decision to return below is a judgement call. Presumably, in most situations, dotted
+        // access to a property is correct, and should not be turned into quoted access even if
+        // there is a string index on the type. However it is possible to construct programs where
+        // this is incorrect, e.g. where user code assigns into a property through the index access
+        // in another location.
+        if (propSym) return false;
+
         this.debugWarn(
             pae,
             this.typeChecker.typeToString(t) +

test_files/enum/enum.js

@@ -16,7 +16,7 @@ EnumTest1[EnumTest1.PI] = "PI";
 // number.  Verify that the resulting TypeScript still allows you to
 // index into the enum with all the various ways allowed of enums.
 let /** @type {number} */ enumTestValue = EnumTest1.XYZ;
-let /** @type {number} */ enumTestValue2 = EnumTest1['XYZ'];
+let /** @type {number} */ enumTestValue2 = EnumTest1.XYZ;
 let /** @type {string} */ enumNumIndex = EnumTest1[((null))];
 let /** @type {number} */ enumStrIndex = EnumTest1[((null))];
 /**
@@ -25,7 +25,8 @@ let /** @type {number} */ enumStrIndex = EnumTest1[((null))];
  */
 function enumTestFunction(val) { }
 enumTestFunction(enumTestValue);
-let /** @type {number} */ enumTestLookup = EnumTest1["XYZ"];
+let /** @type {number} */ enumTestLookup = EnumTest1.XYZ;
+let /** @type {?} */ enumTestLookup2 = EnumTest1["xyz".toUpperCase()];
 exports.EnumTest2 = {};
 /** @type {number} */
 exports.EnumTest2.XYZ = 0;

test_files/enum/enum.ts

@@ -14,6 +14,7 @@ function enumTestFunction(val: EnumTest1) {}
 enumTestFunction(enumTestValue);
 
 let enumTestLookup = EnumTest1["XYZ"];
+let enumTestLookup2 = EnumTest1["xyz".toUpperCase()];
 
 // This additional exported enum is here to exercise the fix for issue #51.
 export enum EnumTest2 {XYZ, PI = 3.14159}

test_files/enum/enum.tsickle.ts

@@ -21,7 +21,7 @@ EnumTest1[EnumTest1.PI] = "PI";
 // number.  Verify that the resulting TypeScript still allows you to
 // index into the enum with all the various ways allowed of enums.
 let /** @type {number} */ enumTestValue: EnumTest1 = EnumTest1.XYZ;
-let /** @type {number} */ enumTestValue2: EnumTest1 = EnumTest1['XYZ'];
+let /** @type {number} */ enumTestValue2: EnumTest1 = EnumTest1.XYZ;
 let /** @type {string} */ enumNumIndex: string = EnumTest1[ /** @type {number} */(( /** @type {?} */((null as any)) as number))];
 let /** @type {number} */ enumStrIndex: number = EnumTest1[ /** @type {string} */(( /** @type {?} */((null as any)) as string))];
 /**
@@ -31,7 +31,8 @@ let /** @type {number} */ enumStrIndex: number = EnumTest1[ /** @type {string} *
 function enumTestFunction(val: EnumTest1) {}
 enumTestFunction(enumTestValue);
 
-let /** @type {number} */ enumTestLookup = EnumTest1["XYZ"];
+let /** @type {number} */ enumTestLookup = EnumTest1.XYZ;
+let /** @type {?} */ enumTestLookup2 = EnumTest1["xyz".toUpperCase()];
 export type EnumTest2 = number;
 export let EnumTest2: any = {};
 /** @type {number} */

test_files/quote_props/quote.js

@@ -1,4 +1,5 @@
-goog.module('test_files.quote_props.quote');var module = module || {id: 'test_files/quote_props/quote.js'};/**
+goog.module('test_files.quote_props.quote');var module = module || {id: 'test_files/quote_props/quote.js'};
+/**
  * @record
  */
 function Quoted() { }
@@ -13,10 +14,8 @@ quoted['hello'] = 1;
 function QuotedMixed() { }
 /** @type {number} */
 QuotedMixed.prototype.foo;
-// TODO(martinprobst): should 'foo: 1' below be quoted?
 let /** @type {!QuotedMixed} */ quotedMixed = { foo: 1 };
-console.log(quotedMixed['foo']);
-// TODO(martinprobst): should this access to a declared property be quoted?
-quotedMixed['foo'] = 1;
-// TODO(martinprobst): should this access to a declared property be un-quoted?
-quotedMixed['foo'] = 1;
+console.log(quotedMixed.foo);
+quotedMixed.foo = 1;
+// Should be converted to non-quoted access.
+quotedMixed.foo = 1;

test_files/quote_props/quote.ts

@@ -1,3 +1,6 @@
+// silence warnings about redeclaring vars.
+export {};
+
 interface Quoted {
   [k: string]: number;
 }
@@ -13,11 +16,9 @@ interface QuotedMixed extends Quoted {
   // access this field in a mixed fashion.
   foo: number;
 }
-// TODO(martinprobst): should 'foo: 1' below be quoted?
 let quotedMixed: QuotedMixed = {foo: 1};
 console.log(quotedMixed.foo);
 
-// TODO(martinprobst): should this access to a declared property be quoted?
 quotedMixed.foo = 1;
-// TODO(martinprobst): should this access to a declared property be un-quoted?
+// Should be converted to non-quoted access.
 quotedMixed['foo'] = 1;

test_files/quote_props/quote.tsickle.ts

@@ -1,16 +1,17 @@
-Warning at test_files/quote_props/quote.ts:6:13: Quoted has a string index type but is accessed using dotted access. Quoting the access.
-Warning at test_files/quote_props/quote.ts:7:1: Quoted has a string index type but is accessed using dotted access. Quoting the access.
-Warning at test_files/quote_props/quote.ts:18:13: QuotedMixed has a string index type but is accessed using dotted access. Quoting the access.
-Warning at test_files/quote_props/quote.ts:21:1: QuotedMixed has a string index type but is accessed using dotted access. Quoting the access.
+Warning at test_files/quote_props/quote.ts:9:13: Quoted has a string index type but is accessed using dotted access. Quoting the access.
+Warning at test_files/quote_props/quote.ts:10:1: Quoted has a string index type but is accessed using dotted access. Quoting the access.
 ====
-
+// silence warnings about redeclaring vars.
+export {};
 /**
  * @record
  */
 function Quoted() {}
 /* TODO: handle strange member:
 [k: string]: number;
 */
+
+
 interface Quoted {
   [k: string]: number;
 }
@@ -34,11 +35,9 @@ interface QuotedMixed extends Quoted {
   // access this field in a mixed fashion.
   foo: number;
 }
-// TODO(martinprobst): should 'foo: 1' below be quoted?
 let /** @type {!QuotedMixed} */ quotedMixed: QuotedMixed = {foo: 1};
-console.log(quotedMixed['foo']);
+console.log(quotedMixed.foo);
 
-// TODO(martinprobst): should this access to a declared property be quoted?
-quotedMixed['foo'] = 1;
-// TODO(martinprobst): should this access to a declared property be un-quoted?
-quotedMixed['foo'] = 1;
+quotedMixed.foo = 1;
+// Should be converted to non-quoted access.
+quotedMixed.foo = 1;