refactor(core): undecorated-classes migration should properly construct object literal from metadata (#32319)

The `undecorated-classes-with-di` migration currently creates invalid object literals from parsed
NGC metadata files if there are object literal properties with keys that contain special characters.

e.g. consider a decorated base class with a host binding using `[class.X]`. Currently the migration
parses and converts the metadata to TypeScript code but incorrectly uses `[class.X]` unquoted as
identifier.

PR Close #32319
This commit is contained in:
Paul Gschwendtner 2019-08-26 18:26:04 +02:00 committed by Miško Hevery
parent d0f3539e6e
commit 543631f2b3
2 changed files with 33 additions and 6 deletions

View File

@ -9,6 +9,9 @@
import {StaticSymbol} from '@angular/compiler'; import {StaticSymbol} from '@angular/compiler';
import * as ts from 'typescript'; import * as ts from 'typescript';
/** Error that will be thrown if an unexpected value needs to be converted. */
export class UnexpectedMetadataValueError extends Error {}
/** /**
* Converts a directive metadata object into a TypeScript expression. Throws * Converts a directive metadata object into a TypeScript expression. Throws
* if metadata cannot be cleanly converted. * if metadata cannot be cleanly converted.
@ -63,7 +66,7 @@ export function convertDirectiveMetadataToExpression(
metadataValue, resolveSymbolImport, createImport, convertProperty); metadataValue, resolveSymbolImport, createImport, convertProperty);
} }
literalProperties.push(ts.createPropertyAssignment(key, propertyValue)); literalProperties.push(ts.createPropertyAssignment(getPropertyName(key), propertyValue));
} }
return ts.createObjectLiteral(literalProperties, true); return ts.createObjectLiteral(literalProperties, true);
@ -72,5 +75,17 @@ export function convertDirectiveMetadataToExpression(
throw new UnexpectedMetadataValueError(); throw new UnexpectedMetadataValueError();
} }
/** Error that will be thrown if a unexpected value needs to be converted. */ /**
export class UnexpectedMetadataValueError extends Error {} * Gets a valid property name from the given text. If the text cannot be used
* as unquoted identifier, the name will be wrapped in a string literal.
*/
function getPropertyName(name: string): string|ts.StringLiteral {
// Matches the most common identifiers that do not need quotes. Constructing a
// regular expression that matches the ECMAScript specification in order to determine
// whether quotes are needed is out of scope for this migration. For those more complex
// property names, we just always use quotes (when constructing AST from metadata).
if (/^[a-zA-Z_$]+$/.test(name)) {
return name;
}
return ts.createStringLiteral(name);
}

View File

@ -1027,7 +1027,10 @@ describe('Undecorated classes with DI migration', () => {
provide: NG_VALIDATORS, provide: NG_VALIDATORS,
useExisting: BaseComponent, useExisting: BaseComponent,
multi: true multi: true
}] }],
host: {
"[class.is-enabled]": "isEnabled === true"
}
}) })
export class PassThrough extends BaseComponent {}`); export class PassThrough extends BaseComponent {}`);
expect(tree.readContent('/index.ts')).toContain(dedent ` expect(tree.readContent('/index.ts')).toContain(dedent `
@ -1040,7 +1043,10 @@ describe('Undecorated classes with DI migration', () => {
provide: NG_VALIDATORS, provide: NG_VALIDATORS,
useExisting: BaseComponent, useExisting: BaseComponent,
multi: true multi: true
}] }],
host: {
"[class.is-enabled]": "isEnabled === true"
}
}) })
export class MyComp extends PassThrough {}`); export class MyComp extends PassThrough {}`);
expect(tree.readContent('/index.ts')).toContain(dedent ` expect(tree.readContent('/index.ts')).toContain(dedent `
@ -1081,7 +1087,10 @@ describe('Undecorated classes with DI migration', () => {
provide: NG_VALIDATORS, provide: NG_VALIDATORS,
useExisting: BaseComponent, useExisting: BaseComponent,
multi: true multi: true
}] }],
host: {
"[class.is-enabled]": "isEnabled === true"
}
}) })
export class MyComp extends BaseComponent {}`); export class MyComp extends BaseComponent {}`);
}); });
@ -1238,6 +1247,9 @@ describe('Undecorated classes with DI migration', () => {
member: 'None' member: 'None'
}, },
providers: [{__symbolic: 'reference', name: 'testValidators'}], providers: [{__symbolic: 'reference', name: 'testValidators'}],
host: {
'[class.is-enabled]': 'isEnabled === true',
}
}] }]
}], }],
members: {} members: {}