fix(compiler): emit quoted object literal keys if the source is quoted

feat(tsc-wrapped): recored when to quote a object literal key

Collecting quoted literals is off by default as it introduces
a breaking change in the .metadata.json file. A follow-up commit
will address this.

Fixes #13249
Closes #13356

karma-js.conf.js

@@ -48,6 +48,7 @@ module.exports = function(config) {
 
     exclude: [
       'dist/all/@angular/**/e2e_test/**',
+      'dist/all/@angular/**/*node_only_spec.js',
       'dist/all/@angular/benchpress/**',
       'dist/all/@angular/compiler-cli/**',
       'dist/all/@angular/compiler/test/aot/**',

modules/@angular/compiler/src/aot/static_reflector.ts

@@ -20,6 +20,8 @@ const ANGULAR_IMPORT_LOCATIONS = {
   provider: '@angular/core/src/di/provider'
 };
 
+const HIDDEN_KEY = /^\$.*\$$/;
+
 /**
  * The host of the StaticReflector disconnects the implementation from TypeScript / other language
  * services and from underlying file systems.
@@ -806,7 +808,11 @@ function mapStringMap(input: {[key: string]: any}, transform: (value: any, key:
   Object.keys(input).forEach((key) => {
     const value = transform(input[key], key);
     if (!shouldIgnore(value)) {
-      result[key] = value;
+      if (HIDDEN_KEY.test(key)) {
+        Object.defineProperty(result, key, {enumerable: false, configurable: true, value: value});
+      } else {
+        result[key] = value;
+      }
     }
   });
   return result;

modules/@angular/compiler/src/output/abstract_emitter.ts

@@ -367,8 +367,8 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex
     ctx.print(`{`, useNewLine);
     ctx.incIndent();
     this.visitAllObjects(entry => {
-      ctx.print(`${escapeIdentifier(entry[0], this._escapeDollarInStrings, false)}: `);
+      ctx.print(`${escapeIdentifier(entry.key, this._escapeDollarInStrings, entry.quoted)}: `);
-      entry[1].visitExpression(this, ctx);
+      entry.value.visitExpression(this, ctx);
     }, ast.entries, ctx, ',', useNewLine);
     ctx.decIndent();
     ctx.print(`}`, useNewLine);

modules/@angular/compiler/src/output/output_ast.ts

@@ -413,10 +413,13 @@ export class LiteralArrayExpr extends Expression {
   }
 }
 
+export class LiteralMapEntry {
+  constructor(public key: string, public value: Expression, public quoted: boolean = false) {}
+}
 
 export class LiteralMapExpr extends Expression {
   public valueType: Type = null;
-  constructor(public entries: [string, Expression][], type: MapType = null) {
+  constructor(public entries: LiteralMapEntry[], type: MapType = null) {
     super(type);
     if (isPresent(type)) {
       this.valueType = type.valueType;
@@ -677,7 +680,8 @@ export class ExpressionTransformer implements StatementVisitor, ExpressionVisito
 
   visitLiteralMapExpr(ast: LiteralMapExpr, context: any): any {
     const entries = ast.entries.map(
-        (entry): [string, Expression] => [entry[0], entry[1].visitExpression(this, context), ]);
+        (entry): LiteralMapEntry => new LiteralMapEntry(
+            entry.key, entry.value.visitExpression(this, context), entry.quoted));
     return new LiteralMapExpr(entries);
   }
   visitAllExpressions(exprs: Expression[], context: any): Expression[] {
@@ -791,7 +795,7 @@ export class RecursiveExpressionVisitor implements StatementVisitor, ExpressionV
     return ast;
   }
   visitLiteralMapExpr(ast: LiteralMapExpr, context: any): any {
-    ast.entries.forEach((entry) => (<Expression>entry[1]).visitExpression(this, context));
+    ast.entries.forEach((entry) => entry.value.visitExpression(this, context));
     return ast;
   }
   visitAllExpressions(exprs: Expression[], context: any): void {
@@ -891,7 +895,7 @@ export function literalArr(values: Expression[], type: Type = null): LiteralArra
 }
 
 export function literalMap(values: [string, Expression][], type: MapType = null): LiteralMapExpr {
-  return new LiteralMapExpr(values, type);
+  return new LiteralMapExpr(values.map(entry => new LiteralMapEntry(entry[0], entry[1])), type);
 }
 
 export function not(expr: Expression): NotExpr {

modules/@angular/compiler/src/output/output_interpreter.ts

@@ -301,8 +301,7 @@ class StatementInterpreter implements o.StatementVisitor, o.ExpressionVisitor {
   visitLiteralMapExpr(ast: o.LiteralMapExpr, ctx: _ExecutionContext): any {
     const result = {};
     ast.entries.forEach(
-        (entry) => (result as any)[<string>entry[0]] =
+        (entry) => (result as any)[entry.key] = entry.value.visitExpression(this, ctx));
-            (<o.Expression>entry[1]).visitExpression(this, ctx));
     return result;
   }
 

modules/@angular/compiler/src/output/value_util.ts

@@ -12,6 +12,8 @@ import {ValueTransformer, visitValue} from '../util';
 
 import * as o from './output_ast';
 
+export const QUOTED_KEYS = '$quoted$';
+
 export function convertValueToOutputAst(value: any, type: o.Type = null): o.Expression {
   return visitValue(value, new _ValueOutputAstTransformer(), type);
 }
@@ -22,9 +24,13 @@ class _ValueOutputAstTransformer implements ValueTransformer {
   }
 
   visitStringMap(map: {[key: string]: any}, type: o.MapType): o.Expression {
-    const entries: [string, o.Expression][] = [];
+    const entries: o.LiteralMapEntry[] = [];
-    Object.keys(map).forEach(key => { entries.push([key, visitValue(map[key], this, null)]); });
+    const quotedSet = new Set<string>(map && map[QUOTED_KEYS]);
-    return o.literalMap(entries, type);
+    Object.keys(map).forEach(key => {
+      entries.push(
+          new o.LiteralMapEntry(key, visitValue(map[key], this, null), quotedSet.has(key)));
+    });
+    return new o.LiteralMapExpr(entries, type);
   }
 
   visitPrimitive(value: any, type: o.Type): o.Expression { return o.literal(value, type); }

modules/@angular/compiler/test/output/ts_emitter_node_only_spec.ts

@@ -0,0 +1,72 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {StaticReflector, StaticReflectorHost, StaticSymbol} from '@angular/compiler';
+import * as o from '@angular/compiler/src/output/output_ast';
+import {ImportResolver} from '@angular/compiler/src/output/path_util';
+import {TypeScriptEmitter} from '@angular/compiler/src/output/ts_emitter';
+import {convertValueToOutputAst} from '@angular/compiler/src/output/value_util';
+import {MetadataCollector, isClassMetadata, isMetadataSymbolicCallExpression} from '@angular/tsc-wrapped';
+import * as ts from 'typescript';
+
+describe('TypeScriptEmitter (node only)', () => {
+  it('should quote identifiers quoted in the source', () => {
+    const sourceText = `
+      import {Component} from '@angular/core';
+
+      @Component({
+        providers: [{ provide: 'SomeToken', useValue: {a: 1, 'b': 2, c: 3, 'd': 4}}]
+      })
+      export class MyComponent {}
+    `;
+    const source = ts.createSourceFile('test.ts', sourceText, ts.ScriptTarget.Latest);
+    const collector = new MetadataCollector({quotedNames: true});
+    const stubHost = new StubReflectorHost();
+    const reflector = new StaticReflector(stubHost);
+
+    // Get the metadata from the above source
+    const metadata = collector.getMetadata(source);
+    const componentMetadata = metadata.metadata['MyComponent'];
+
+    // Get the first argument of the decorator call which is passed to @Component
+    expect(isClassMetadata(componentMetadata)).toBeTruthy();
+    if (!isClassMetadata(componentMetadata)) return;
+    const decorators = componentMetadata.decorators;
+    const firstDecorator = decorators[0];
+    expect(isMetadataSymbolicCallExpression(firstDecorator)).toBeTruthy();
+    if (!isMetadataSymbolicCallExpression(firstDecorator)) return;
+    const firstArgument = firstDecorator.arguments[0];
+
+    // Simplify this value using the StaticReflector
+    const context = reflector.getStaticSymbol('none', 'none');
+    const argumentValue = reflector.simplify(context, firstArgument);
+
+    // Convert the value to an output AST
+    const outputAst = convertValueToOutputAst(argumentValue);
+    const statement = outputAst.toStmt();
+
+    // Convert the value to text using the typescript emitter
+    const emitter = new TypeScriptEmitter(new StubImportResolver());
+    const text = emitter.emitStatements('module', [statement], []);
+
+    // Expect the keys for 'b' and 'd' to be quoted but 'a' and 'c' not to be.
+    expect(text).toContain('\'b\': 2');
+    expect(text).toContain('\'d\': 4');
+    expect(text).not.toContain('\'a\'');
+    expect(text).not.toContain('\'c\'');
+  });
+});
+
+class StubReflectorHost implements StaticReflectorHost {
+  getMetadataFor(modulePath: string): {[key: string]: any}[] { return []; }
+  moduleNameToFileName(moduleName: string, containingFile: string): string { return ''; }
+}
+
+class StubImportResolver extends ImportResolver {
+  fileNameToModuleName(importedFilePath: string, containingFilePath: string): string { return ''; }
+}

tools/@angular/tsc-wrapped/src/collector.ts

@@ -13,11 +13,22 @@ import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, Me
 import {Symbols} from './symbols';
 
 
+/**
+ * A set of collector options to use when collecting metadata.
+ */
+export class CollectorOptions {
+  /**
+   * Collect a hidden field "$quoted$" in objects literals that record when the key was quoted in
+   * the source.
+   */
+  quotedNames?: boolean;
+}
+
 /**
  * Collect decorator metadata from a TypeScript module.
  */
 export class MetadataCollector {
-  constructor() {}
+  constructor(private options: CollectorOptions = {}) {}
 
   /**
    * Returns a JSON.stringify friendly form describing the decorators of the exported classes from
@@ -26,7 +37,7 @@ export class MetadataCollector {
   public getMetadata(sourceFile: ts.SourceFile, strict: boolean = false): ModuleMetadata {
     const locals = new Symbols(sourceFile);
     const nodeMap = new Map<MetadataValue|ClassMetadata|FunctionMetadata, ts.Node>();
-    const evaluator = new Evaluator(locals, nodeMap);
+    const evaluator = new Evaluator(locals, nodeMap, this.options);
     let metadata: {[name: string]: MetadataValue | ClassMetadata | FunctionMetadata}|undefined;
     let exports: ModuleExportMetadata[];
 

tools/@angular/tsc-wrapped/src/evaluator.ts

@@ -8,6 +8,7 @@
 
 import * as ts from 'typescript';
 
+import {CollectorOptions} from './collector';
 import {MetadataEntry, MetadataError, MetadataGlobalReferenceExpression, MetadataImportedSymbolReferenceExpression, MetadataSymbolicCallExpression, MetadataSymbolicReferenceExpression, MetadataValue, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSpreadExpression} from './schema';
 import {Symbols} from './symbols';
 
@@ -97,7 +98,9 @@ export function errorSymbol(
  * possible.
  */
 export class Evaluator {
-  constructor(private symbols: Symbols, private nodeMap: Map<MetadataEntry, ts.Node>) {}
+  constructor(
+      private symbols: Symbols, private nodeMap: Map<MetadataEntry, ts.Node>,
+      private options: CollectorOptions = {}) {}
 
   nameOf(node: ts.Node): string|MetadataError {
     if (node.kind == ts.SyntaxKind.Identifier) {
@@ -223,11 +226,16 @@ export class Evaluator {
     switch (node.kind) {
       case ts.SyntaxKind.ObjectLiteralExpression:
         let obj: {[name: string]: any} = {};
+        let quoted: string[] = [];
         ts.forEachChild(node, child => {
           switch (child.kind) {
             case ts.SyntaxKind.ShorthandPropertyAssignment:
             case ts.SyntaxKind.PropertyAssignment:
               const assignment = <ts.PropertyAssignment|ts.ShorthandPropertyAssignment>child;
+              if (assignment.name.kind == ts.SyntaxKind.StringLiteral) {
+                const name = (assignment.name as ts.StringLiteral).text;
+                quoted.push(name);
+              }
               const propertyName = this.nameOf(assignment.name);
               if (isMetadataError(propertyName)) {
                 error = propertyName;
@@ -245,6 +253,9 @@ export class Evaluator {
           }
         });
         if (error) return error;
+        if (this.options.quotedNames && quoted.length) {
+          obj['$quoted$'] = quoted;
+        }
         return obj;
       case ts.SyntaxKind.ArrayLiteralExpression:
         let arr: MetadataValue[] = [];

tools/@angular/tsc-wrapped/test/collector.spec.ts

@@ -50,7 +50,7 @@ describe('Collector', () => {
     ]);
     service = ts.createLanguageService(host, documentRegistry);
     program = service.getProgram();
-    collector = new MetadataCollector();
+    collector = new MetadataCollector({quotedNames: true});
   });
 
   it('should not have errors in test data', () => { expectValidSources(service, program); });
@@ -164,11 +164,16 @@ describe('Collector', () => {
       version: 2,
       metadata: {
         HEROES: [
-          {'id': 11, 'name': 'Mr. Nice'}, {'id': 12, 'name': 'Narco'},
+          {'id': 11, 'name': 'Mr. Nice', '$quoted$': ['id', 'name']},
-          {'id': 13, 'name': 'Bombasto'}, {'id': 14, 'name': 'Celeritas'},
+          {'id': 12, 'name': 'Narco', '$quoted$': ['id', 'name']},
-          {'id': 15, 'name': 'Magneta'}, {'id': 16, 'name': 'RubberMan'},
+          {'id': 13, 'name': 'Bombasto', '$quoted$': ['id', 'name']},
-          {'id': 17, 'name': 'Dynama'}, {'id': 18, 'name': 'Dr IQ'}, {'id': 19, 'name': 'Magma'},
+          {'id': 14, 'name': 'Celeritas', '$quoted$': ['id', 'name']},
-          {'id': 20, 'name': 'Tornado'}
+          {'id': 15, 'name': 'Magneta', '$quoted$': ['id', 'name']},
+          {'id': 16, 'name': 'RubberMan', '$quoted$': ['id', 'name']},
+          {'id': 17, 'name': 'Dynama', '$quoted$': ['id', 'name']},
+          {'id': 18, 'name': 'Dr IQ', '$quoted$': ['id', 'name']},
+          {'id': 19, 'name': 'Magma', '$quoted$': ['id', 'name']},
+          {'id': 20, 'name': 'Tornado', '$quoted$': ['id', 'name']}
         ]
       }
     });