From abad6673e6ee23e10e194f12aedade89948a561e Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 26 Aug 2016 15:54:34 -0700 Subject: [PATCH] fix(ngc): don't quote properties in literal maps (#11110) Closure compiler treats quoted properties specially, and doesn't rename them. Fixes #11050 --- .../compiler/src/output/abstract_emitter.ts | 11 ++++++---- .../test/output/abstract_emitter_spec.ts | 21 +++++++++++-------- .../compiler/test/output/js_emitter_spec.ts | 3 +-- .../compiler/test/output/ts_emitter_spec.ts | 3 +-- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/modules/@angular/compiler/src/output/abstract_emitter.ts b/modules/@angular/compiler/src/output/abstract_emitter.ts index 359867cf2a..54dc6b1981 100644 --- a/modules/@angular/compiler/src/output/abstract_emitter.ts +++ b/modules/@angular/compiler/src/output/abstract_emitter.ts @@ -11,6 +11,7 @@ import {StringWrapper, isBlank, isPresent, isString} from '../facade/lang'; import * as o from './output_ast'; var _SINGLE_QUOTE_ESCAPE_STRING_RE = /'|\\|\n|\r|\$/g; +var _LEGAL_IDENTIFIER_RE = /^[$A-Z_][0-9A-Z_$]*$/i; export var CATCH_ERROR_VAR = o.variable('error'); export var CATCH_STACK_VAR = o.variable('stack'); @@ -253,7 +254,7 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex any { var value = ast.value; if (isString(value)) { - ctx.print(escapeSingleQuoteString(value, this._escapeDollarInStrings)); + ctx.print(escapeIdentifier(value, this._escapeDollarInStrings)); } else if (isBlank(value)) { ctx.print(absentValue); } else { @@ -368,7 +369,7 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex ctx.print(`{`, useNewLine); ctx.incIndent(); this.visitAllObjects((entry: any /** TODO #9100 */) => { - ctx.print(`${escapeSingleQuoteString(entry[0], this._escapeDollarInStrings)}: `); + ctx.print(`${escapeIdentifier(entry[0], this._escapeDollarInStrings, false)}: `); entry[1].visitExpression(this, ctx); }, ast.entries, ctx, ',', useNewLine); ctx.decIndent(); @@ -403,7 +404,8 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex } } -export function escapeSingleQuoteString(input: string, escapeDollar: boolean): any { +export function escapeIdentifier( + input: string, escapeDollar: boolean, alwaysQuote: boolean = true): any { if (isBlank(input)) { return null; } @@ -419,7 +421,8 @@ export function escapeSingleQuoteString(input: string, escapeDollar: boolean): a return `\\${match[0]}`; } }); - return `'${body}'`; + let requiresQuotes = alwaysQuote || !_LEGAL_IDENTIFIER_RE.test(body); + return requiresQuotes ? `'${body}'` : body; } function _createIndent(count: number): string { diff --git a/modules/@angular/compiler/test/output/abstract_emitter_spec.ts b/modules/@angular/compiler/test/output/abstract_emitter_spec.ts index d64edde3c5..d5b885e4b1 100644 --- a/modules/@angular/compiler/test/output/abstract_emitter_spec.ts +++ b/modules/@angular/compiler/test/output/abstract_emitter_spec.ts @@ -6,27 +6,30 @@ * found in the LICENSE file at https://angular.io/license */ -import {escapeSingleQuoteString} from '@angular/compiler/src/output/abstract_emitter'; +import {escapeIdentifier} from '@angular/compiler/src/output/abstract_emitter'; import {beforeEach, ddescribe, describe, expect, iit, inject, it, xit} from '@angular/core/testing/testing_internal'; export function main() { describe('AbstractEmitter', () => { - describe('escapeSingleQuoteString', () => { + describe('escapeIdentifier', () => { it('should escape single quotes', - () => { expect(escapeSingleQuoteString(`'`, false)).toEqual(`'\\''`); }); + () => { expect(escapeIdentifier(`'`, false)).toEqual(`'\\''`); }); it('should escape backslash', - () => { expect(escapeSingleQuoteString('\\', false)).toEqual(`'\\\\'`); }); + () => { expect(escapeIdentifier('\\', false)).toEqual(`'\\\\'`); }); it('should escape newlines', - () => { expect(escapeSingleQuoteString('\n', false)).toEqual(`'\\n'`); }); + () => { expect(escapeIdentifier('\n', false)).toEqual(`'\\n'`); }); it('should escape carriage returns', - () => { expect(escapeSingleQuoteString('\r', false)).toEqual(`'\\r'`); }); + () => { expect(escapeIdentifier('\r', false)).toEqual(`'\\r'`); }); - it('should escape $', () => { expect(escapeSingleQuoteString('$', true)).toEqual(`'\\$'`); }); - it('should not escape $', - () => { expect(escapeSingleQuoteString('$', false)).toEqual(`'$'`); }); + it('should escape $', () => { expect(escapeIdentifier('$', true)).toEqual(`'\\$'`); }); + it('should not escape $', () => { expect(escapeIdentifier('$', false)).toEqual(`'$'`); }); + it('should add quotes for non-identifiers', + () => { expect(escapeIdentifier('==', false, false)).toEqual(`'=='`); }); + it('does not escape class (but it probably should)', + () => { expect(escapeIdentifier('class', false, false)).toEqual('class'); }); }); }); diff --git a/modules/@angular/compiler/test/output/js_emitter_spec.ts b/modules/@angular/compiler/test/output/js_emitter_spec.ts index 4a9fbb15ef..77f0a28b70 100644 --- a/modules/@angular/compiler/test/output/js_emitter_spec.ts +++ b/modules/@angular/compiler/test/output/js_emitter_spec.ts @@ -104,8 +104,7 @@ export function main() { expect(emitStmt(o.literal(true).toStmt())).toEqual('true;'); expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`); expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`); - expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())) - .toEqual(`{'someKey': 1};`); + expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey: 1};`); }); it('should support external identifiers', () => { diff --git a/modules/@angular/compiler/test/output/ts_emitter_spec.ts b/modules/@angular/compiler/test/output/ts_emitter_spec.ts index 1799af86d8..562c8dd107 100644 --- a/modules/@angular/compiler/test/output/ts_emitter_spec.ts +++ b/modules/@angular/compiler/test/output/ts_emitter_spec.ts @@ -106,8 +106,7 @@ export function main() { expect(emitStmt(o.literal(true).toStmt())).toEqual('true;'); expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`); expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`); - expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())) - .toEqual(`{'someKey': 1};`); + expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey: 1};`); }); it('should support external identifiers', () => {