From bcf17bc91c464e936f0e8e6ba49bb6d3a7353035 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Fri, 25 Jan 2019 12:12:57 +0000 Subject: [PATCH] refactor(compiler-cli): return TS nodes from TypeTranslatorVisitor (#28342) The TypeTranslatorVisitor visitor returned strings because before it wasn't possible to transform declaration files directly through the TypeScript custom transformer API. Now that's possible though, so it should return nodes instead. PR Close #28342 --- .../src/ngcc/src/rendering/renderer.ts | 4 +- .../src/ngtsc/transform/src/declaration.ts | 3 +- .../src/ngtsc/translator/src/translator.ts | 106 +++++++++--------- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 10 +- 4 files changed, 63 insertions(+), 60 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts index 1b702c0ade..e310b6172b 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts @@ -169,6 +169,7 @@ export abstract class Renderer { renderDtsFile(dtsFile: ts.SourceFile, renderInfo: DtsRenderInfo): FileInfo[] { const input = this.extractSourceMap(dtsFile); const outputText = new MagicString(input.source); + const printer = ts.createPrinter(); const importManager = new ImportManager( this.getImportRewriter(this.bundle.dts !.r3SymbolsFile, false), IMPORT_PREFIX); @@ -176,7 +177,8 @@ export abstract class Renderer { const endOfClass = dtsClass.dtsDeclaration.getEnd(); dtsClass.compilation.forEach(declaration => { const type = translateType(declaration.type, importManager); - const newStatement = ` static ${declaration.name}: ${type};\n`; + const typeStr = printer.printNode(ts.EmitHint.Unspecified, type, dtsFile); + const newStatement = ` static ${declaration.name}: ${typeStr};\n`; outputText.appendRight(endOfClass - 1, newStatement); }); }); diff --git a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts index 59a2bdbebb..0873dea3e2 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts @@ -57,8 +57,7 @@ export class DtsFileTransformer { const decls = this.ivyFields.get(node.name.text) !; const newMembers = decls.map(decl => { const modifiers = [ts.createModifier(ts.SyntaxKind.StaticKeyword)]; - const type = translateType(decl.type, this.imports); - const typeRef = ts.createTypeReferenceNode(ts.createIdentifier(type), undefined); + const typeRef = translateType(decl.type, this.imports); return ts.createProperty(undefined, modifiers, decl.name, undefined, typeRef, undefined); }); diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index a3fbc1ac6c..d8124c61b7 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -86,7 +86,7 @@ export function translateStatement(statement: Statement, imports: ImportManager) return statement.visitStatement(new ExpressionTranslatorVisitor(imports), new Context(true)); } -export function translateType(type: Type, imports: ImportManager): string { +export function translateType(type: Type, imports: ImportManager): ts.TypeNode { return type.visitType(new TypeTranslatorVisitor(imports), new Context(false)); } @@ -294,44 +294,44 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { constructor(private imports: ImportManager) {} - visitBuiltinType(type: BuiltinType, context: Context): string { + visitBuiltinType(type: BuiltinType, context: Context): ts.KeywordTypeNode { switch (type.name) { case BuiltinTypeName.Bool: - return 'boolean'; + return ts.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword); case BuiltinTypeName.Dynamic: - return 'any'; + return ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword); case BuiltinTypeName.Int: case BuiltinTypeName.Number: - return 'number'; + return ts.createKeywordTypeNode(ts.SyntaxKind.NumberKeyword); case BuiltinTypeName.String: - return 'string'; + return ts.createKeywordTypeNode(ts.SyntaxKind.StringKeyword); case BuiltinTypeName.None: - return 'never'; + return ts.createKeywordTypeNode(ts.SyntaxKind.NeverKeyword); default: throw new Error(`Unsupported builtin type: ${BuiltinTypeName[type.name]}`); } } - visitExpressionType(type: ExpressionType, context: Context): string { - const exprStr = type.value.visitExpression(this, context); - if (type.typeParams !== null) { - const typeSegments = type.typeParams.map(param => param.visitType(this, context)); - return `${exprStr}<${typeSegments.join(', ')}>`; - } else { - return exprStr; - } + visitExpressionType(type: ExpressionType, context: Context): ts.TypeReferenceType { + const expr: ts.Identifier|ts.QualifiedName = type.value.visitExpression(this, context); + const typeArgs = type.typeParams !== null ? + type.typeParams.map(param => param.visitType(this, context)) : + undefined; + + return ts.createTypeReferenceNode(expr, typeArgs); } - visitArrayType(type: ArrayType, context: Context): string { - return `Array<${type.visitType(this, context)}>`; + visitArrayType(type: ArrayType, context: Context): ts.ArrayTypeNode { + return ts.createArrayTypeNode(type.visitType(this, context)); } - visitMapType(type: MapType, context: Context): string { - if (type.valueType !== null) { - return `{[key: string]: ${type.valueType.visitType(this, context)}}`; - } else { - return '{[key: string]: any}'; - } + visitMapType(type: MapType, context: Context): ts.TypeLiteralNode { + const parameter = ts.createParameter( + undefined, undefined, undefined, 'key', undefined, + ts.createKeywordTypeNode(ts.SyntaxKind.StringKeyword)); + const typeArgs = type.valueType !== null ? type.valueType.visitType(this, context) : undefined; + const indexSignature = ts.createIndexSignature(undefined, undefined, [parameter], typeArgs); + return ts.createTypeLiteralNode([indexSignature]); } visitReadVarExpr(ast: ReadVarExpr, context: Context): string { @@ -365,28 +365,26 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { throw new Error('Method not implemented.'); } - visitLiteralExpr(ast: LiteralExpr, context: Context): string { - if (typeof ast.value === 'string') { - const escaped = ast.value.replace(/\'/g, '\\\''); - return `'${escaped}'`; - } else { - return `${ast.value}`; - } + visitLiteralExpr(ast: LiteralExpr, context: Context): ts.LiteralExpression { + return ts.createLiteral(ast.value as string); } - visitExternalExpr(ast: ExternalExpr, context: Context): string { + visitExternalExpr(ast: ExternalExpr, context: Context): ts.TypeNode { if (ast.value.moduleName === null || ast.value.name === null) { throw new Error(`Import unknown module or symbol`); } const {moduleImport, symbol} = this.imports.generateNamedImport(ast.value.moduleName, ast.value.name); - const base = moduleImport ? `${moduleImport}.${symbol}` : symbol; - if (ast.typeParams !== null) { - const generics = ast.typeParams.map(type => type.visitType(this, context)).join(', '); - return `${base}<${generics}>`; - } else { - return base; - } + const symbolIdentifier = ts.createIdentifier(symbol); + + const typeName = moduleImport ? + ts.createPropertyAccess(ts.createIdentifier(moduleImport), symbolIdentifier) : + symbolIdentifier; + + const typeArguments = + ast.typeParams ? ast.typeParams.map(type => type.visitType(this, context)) : undefined; + + return ts.createExpressionWithTypeArguments(typeArguments, typeName); } visitConditionalExpr(ast: ConditionalExpr, context: Context) { @@ -417,37 +415,43 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { throw new Error('Method not implemented.'); } - visitLiteralArrayExpr(ast: LiteralArrayExpr, context: Context): string { + visitLiteralArrayExpr(ast: LiteralArrayExpr, context: Context): ts.TupleTypeNode { const values = ast.entries.map(expr => expr.visitExpression(this, context)); - return `[${values.join(', ')}]`; + return ts.createTupleTypeNode(values); } - visitLiteralMapExpr(ast: LiteralMapExpr, context: Context) { + visitLiteralMapExpr(ast: LiteralMapExpr, context: Context): ts.ObjectLiteralExpression { const entries = ast.entries.map(entry => { const {key, quoted} = entry; const value = entry.value.visitExpression(this, context); - if (quoted) { - return `'${key}': ${value}`; - } else { - return `${key}: ${value}`; - } + return ts.createPropertyAssignment(quoted ? `'${key}'` : key, value); }); - return `{${entries.join(', ')}}`; + return ts.createObjectLiteral(entries); } visitCommaExpr(ast: CommaExpr, context: Context) { throw new Error('Method not implemented.'); } - visitWrappedNodeExpr(ast: WrappedNodeExpr, context: Context) { + visitWrappedNodeExpr(ast: WrappedNodeExpr, context: Context): ts.Identifier { const node: ts.Node = ast.node; if (ts.isIdentifier(node)) { - return node.text; + return node; } else { throw new Error( `Unsupported WrappedNodeExpr in TypeTranslatorVisitor: ${ts.SyntaxKind[node.kind]}`); } } - visitTypeofExpr(ast: TypeofExpr, context: Context): string { - return `typeof ${ast.expr.visitExpression(this, context)}`; + visitTypeofExpr(ast: TypeofExpr, context: Context): ts.TypeQueryNode { + let expr = translateExpression(ast.expr, this.imports); + return ts.createTypeQueryNode(expr as ts.Identifier); } } + +function entityNameToExpr(entity: ts.EntityName): ts.Expression { + if (ts.isIdentifier(entity)) { + return entity; + } + const {left, right} = entity; + const leftExpr = ts.isIdentifier(left) ? left : entityNameToExpr(left); + return ts.createPropertyAccess(leftExpr, right); +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index da5cfa7a5d..e7a7f570df 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -181,7 +181,7 @@ describe('ngtsc behavioral tests', () => { const dtsContents = env.getContents('test.d.ts'); expect(dtsContents) .toContain( - 'static ngComponentDef: i0.ɵComponentDefWithMeta'); + 'static ngComponentDef: i0.ɵComponentDefWithMeta'); }); it('should compile Components without errors', () => { @@ -292,7 +292,7 @@ describe('ngtsc behavioral tests', () => { const dtsContents = env.getContents('test.d.ts'); expect(dtsContents) .toContain( - 'static ngComponentDef: i0.ɵComponentDefWithMeta'); + 'static ngComponentDef: i0.ɵComponentDefWithMeta'); expect(dtsContents) .toContain( 'static ngModuleDef: i0.ɵNgModuleDefWithMeta'); @@ -502,8 +502,7 @@ describe('ngtsc behavioral tests', () => { .toContain( 'TestPipe.ngPipeDef = i0.ɵdefinePipe({ name: "test-pipe", type: TestPipe, ' + 'factory: function TestPipe_Factory(t) { return new (t || TestPipe)(); }, pure: false })'); - expect(dtsContents) - .toContain('static ngPipeDef: i0.ɵPipeDefWithMeta;'); + expect(dtsContents).toContain('static ngPipeDef: i0.ɵPipeDefWithMeta;'); }); it('should compile pure Pipes without errors', () => { @@ -526,8 +525,7 @@ describe('ngtsc behavioral tests', () => { .toContain( 'TestPipe.ngPipeDef = i0.ɵdefinePipe({ name: "test-pipe", type: TestPipe, ' + 'factory: function TestPipe_Factory(t) { return new (t || TestPipe)(); }, pure: true })'); - expect(dtsContents) - .toContain('static ngPipeDef: i0.ɵPipeDefWithMeta;'); + expect(dtsContents).toContain('static ngPipeDef: i0.ɵPipeDefWithMeta;'); }); it('should compile Pipes with dependencies', () => {