Do not unquote properties that are not valid JS identifiers.

angular · May 18, 2017 · 8e26ee3 · 8e26ee3
1 parent af6eb5b
commit 8e26ee3
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 4 deletions.
diff --git a/src/tsickle.ts b/src/tsickle.ts
@@ -775,6 +775,8 @@ class Annotator extends ClosureRewriter {
         // If it has a symbol, it's actually a regular declared property.
         if (!quotedPropSym) return false;
         const propName = (eae.argumentExpression as ts.StringLiteral).text;
+        // Properties containing non-JS identifier names must not be unquoted.
+        if (!isValidClosurePropertyName(propName)) return false;
         this.writeNode(eae.expression);
         this.emit(`.${propName}`);
         return true;

diff --git a/test_files/quote_props/quote.js b/test_files/quote_props/quote.js
@@ -1,4 +1,8 @@
-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'};/**
+ * @fileoverview added by tsickle
+ * @suppress {checkTypes} checked by tsc
+ */
+
 /**
  * @record
  */
@@ -14,8 +18,10 @@ quoted['hello'] = 1;
 function QuotedMixed() { }
 /** @type {number} */
 QuotedMixed.prototype.foo;
-let /** @type {!QuotedMixed} */ quotedMixed = { foo: 1 };
+let /** @type {!QuotedMixed} */ quotedMixed = { foo: 1, 'invalid-identifier': 2 };
 console.log(quotedMixed.foo);
 quotedMixed.foo = 1;
 // Should be converted to non-quoted access.
 quotedMixed.foo = 1;
+// Must not be converted to non-quoted access, as it's not valid JS.
+quotedMixed['invalid-identifier'] = 1;
diff --git a/test_files/quote_props/quote.ts b/test_files/quote_props/quote.ts
@@ -15,10 +15,13 @@ interface QuotedMixed extends Quoted {
   // It's unclear whether it's the right thing to do, user code might
   // access this field in a mixed fashion.
   foo: number;
+  'invalid-identifier': number;
 }
-let quotedMixed: QuotedMixed = {foo: 1};
+let quotedMixed: QuotedMixed = {foo: 1, 'invalid-identifier': 2};
 console.log(quotedMixed.foo);
 
 quotedMixed.foo = 1;
 // Should be converted to non-quoted access.
 quotedMixed['foo'] = 1;
+// Must not be converted to non-quoted access, as it's not valid JS.
+quotedMixed['invalid-identifier'] = 1;
diff --git a/test_files/quote_props/quote.tsickle.ts b/test_files/quote_props/quote.tsickle.ts
@@ -1,6 +1,11 @@
 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.
 ====
+/**
+ * @fileoverview added by tsickle
+ * @suppress {checkTypes} checked by tsc
+ */
+
 // silence warnings about redeclaring vars.
 export {};
 /**
@@ -27,17 +32,23 @@ quoted['hello'] = 1;
 function QuotedMixed() {}
 /** @type {number} */
 QuotedMixed.prototype.foo;
+/* TODO: handle strange member:
+'invalid-identifier': number;
+*/
 
 
 interface QuotedMixed extends Quoted {
   // Assume that foo should be renamed, as it is explicitly declared.
   // It's unclear whether it's the right thing to do, user code might
   // access this field in a mixed fashion.
   foo: number;
+  'invalid-identifier': number;
 }
-let /** @type {!QuotedMixed} */ quotedMixed: QuotedMixed = {foo: 1};
+let /** @type {!QuotedMixed} */ quotedMixed: QuotedMixed = {foo: 1, 'invalid-identifier': 2};
 console.log(quotedMixed.foo);
 
 quotedMixed.foo = 1;
 // Should be converted to non-quoted access.
 quotedMixed.foo = 1;
+// Must not be converted to non-quoted access, as it's not valid JS.
+quotedMixed['invalid-identifier'] = 1;