From dd0519abadde5eebf418e4bca3b03a56818fd52f Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Mon, 12 Dec 2016 10:49:17 -0800 Subject: [PATCH] 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 | 1 + .../compiler/src/aot/static_reflector.ts | 8 ++- .../compiler/src/output/abstract_emitter.ts | 4 +- .../compiler/src/output/output_ast.ts | 12 ++-- .../compiler/src/output/output_interpreter.ts | 3 +- .../compiler/src/output/value_util.ts | 12 +++- .../test/output/ts_emitter_node_only_spec.ts | 72 +++++++++++++++++++ tools/@angular/tsc-wrapped/src/collector.ts | 15 +++- tools/@angular/tsc-wrapped/src/evaluator.ts | 13 +++- .../tsc-wrapped/test/collector.spec.ts | 17 +++-- 10 files changed, 136 insertions(+), 21 deletions(-) create mode 100644 modules/@angular/compiler/test/output/ts_emitter_node_only_spec.ts diff --git a/karma-js.conf.js b/karma-js.conf.js index 33acd2bfa7..f8de95f154 100644 --- a/karma-js.conf.js +++ b/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/**', diff --git a/modules/@angular/compiler/src/aot/static_reflector.ts b/modules/@angular/compiler/src/aot/static_reflector.ts index 262b85ac77..ed22fb3d4a 100644 --- a/modules/@angular/compiler/src/aot/static_reflector.ts +++ b/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; diff --git a/modules/@angular/compiler/src/output/abstract_emitter.ts b/modules/@angular/compiler/src/output/abstract_emitter.ts index 99b205313c..7ddd257bf7 100644 --- a/modules/@angular/compiler/src/output/abstract_emitter.ts +++ b/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)}: `); - entry[1].visitExpression(this, ctx); + ctx.print(`${escapeIdentifier(entry.key, this._escapeDollarInStrings, entry.quoted)}: `); + entry.value.visitExpression(this, ctx); }, ast.entries, ctx, ',', useNewLine); ctx.decIndent(); ctx.print(`}`, useNewLine); diff --git a/modules/@angular/compiler/src/output/output_ast.ts b/modules/@angular/compiler/src/output/output_ast.ts index 1171705877..867d19258e 100644 --- a/modules/@angular/compiler/src/output/output_ast.ts +++ b/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) => (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 { diff --git a/modules/@angular/compiler/src/output/output_interpreter.ts b/modules/@angular/compiler/src/output/output_interpreter.ts index 9efafdc3c7..06356cbe4e 100644 --- a/modules/@angular/compiler/src/output/output_interpreter.ts +++ b/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)[entry[0]] = - (entry[1]).visitExpression(this, ctx)); + (entry) => (result as any)[entry.key] = entry.value.visitExpression(this, ctx)); return result; } diff --git a/modules/@angular/compiler/src/output/value_util.ts b/modules/@angular/compiler/src/output/value_util.ts index f8ed97befd..6862eb1d0f 100644 --- a/modules/@angular/compiler/src/output/value_util.ts +++ b/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][] = []; - Object.keys(map).forEach(key => { entries.push([key, visitValue(map[key], this, null)]); }); - return o.literalMap(entries, type); + const entries: o.LiteralMapEntry[] = []; + const quotedSet = new Set(map && map[QUOTED_KEYS]); + 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); } diff --git a/modules/@angular/compiler/test/output/ts_emitter_node_only_spec.ts b/modules/@angular/compiler/test/output/ts_emitter_node_only_spec.ts new file mode 100644 index 0000000000..22c6c3d206 --- /dev/null +++ b/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 ''; } +} \ No newline at end of file diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 62e4deaaa0..b9be3bca3e 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/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(); - 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[]; diff --git a/tools/@angular/tsc-wrapped/src/evaluator.ts b/tools/@angular/tsc-wrapped/src/evaluator.ts index ff266fff44..b668da190f 100644 --- a/tools/@angular/tsc-wrapped/src/evaluator.ts +++ b/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) {} + constructor( + private symbols: Symbols, private nodeMap: Map, + 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 = 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[] = []; diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts index 1fdc9b980a..c3aba82e9f 100644 --- a/tools/@angular/tsc-wrapped/test/collector.spec.ts +++ b/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': 13, 'name': 'Bombasto'}, {'id': 14, 'name': 'Celeritas'}, - {'id': 15, 'name': 'Magneta'}, {'id': 16, 'name': 'RubberMan'}, - {'id': 17, 'name': 'Dynama'}, {'id': 18, 'name': 'Dr IQ'}, {'id': 19, 'name': 'Magma'}, - {'id': 20, 'name': 'Tornado'} + {'id': 11, 'name': 'Mr. Nice', '$quoted$': ['id', 'name']}, + {'id': 12, 'name': 'Narco', '$quoted$': ['id', 'name']}, + {'id': 13, 'name': 'Bombasto', '$quoted$': ['id', 'name']}, + {'id': 14, 'name': 'Celeritas', '$quoted$': ['id', 'name']}, + {'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']} ] } });