refactor(ivy): consistently translate types to `ts.TypeNode` (#34021)

The compiler has a translation mechanism to convert from an Angular
`Type` to a `ts.TypeNode`, as appropriate. Prior to this change, it
would translate certain Angular expressions into their value equivalent
in TypeScript, instead of the correct type equivalent. This was possible
as the `ExpressionVisitor` interface is not strictly typed, with `any`s
being used for return values.

For example, a literal object was translated into a
`ts.ObjectLiteralExpression`, containing `ts.PropertyAssignment` nodes
as its entries. This has worked without issues as their printed
representation is identical, however it was incorrect from a semantic
point of view. Instead, a `ts.TypeLiteralNode` is created with
`ts.PropertySignature` as its members, which corresponds with the type
declaration of an object literal.

PR Close #34021

packages/compiler-cli/src/ngtsc/transform/src/declaration.ts

@@ -205,6 +205,7 @@ export class IvyDeclarationDtsTransform implements DtsTransform {
     const newMembers = fields.map(decl => {
       const modifiers = [ts.createModifier(ts.SyntaxKind.StaticKeyword)];
       const typeRef = translateType(decl.type, imports);
+      emitAsSingleLine(typeRef);
       return ts.createProperty(
           /* decorators */ undefined,
           /* modifiers */ modifiers,
@@ -225,6 +226,11 @@ export class IvyDeclarationDtsTransform implements DtsTransform {
   }
 }
 
+function emitAsSingleLine(node: ts.Node) {
+  ts.setEmitFlags(node, ts.EmitFlags.SingleLine);
+  ts.forEachChild(node, emitAsSingleLine);
+}
+
 export class ReturnTypeTransform implements DtsTransform {
   private typeReplacements = new Map<ts.Declaration, Type>();
 

packages/compiler-cli/src/ngtsc/translator/src/translator.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeVisitor, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler';
+import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeVisitor, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler';
 import {LocalizedString} from '@angular/compiler/src/output/output_ast';
 import * as ts from 'typescript';
 
@@ -433,33 +433,44 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
     }
   }
 
-  visitExpressionType(type: ExpressionType, context: Context): ts.TypeReferenceType {
+  visitExpressionType(type: ExpressionType, context: Context): ts.TypeNode {
-    const expr: ts.EntityName = type.value.visitExpression(this, context);
+    const typeNode = this.translateExpression(type.value, context);
-    const typeArgs = type.typeParams !== null ?
+    if (type.typeParams === null) {
-        type.typeParams.map(param => param.visitType(this, context)) :
+      return typeNode;
-        undefined;
+    }
 
-    return ts.createTypeReferenceNode(expr, typeArgs);
+    if (!ts.isTypeReferenceNode(typeNode)) {
+      throw new Error(
+          'An ExpressionType with type arguments must translate into a TypeReferenceNode');
+    } else if (typeNode.typeArguments !== undefined) {
+      throw new Error(
+          `An ExpressionType with type arguments cannot have multiple levels of type arguments`);
+    }
+
+    const typeArgs = type.typeParams.map(param => param.visitType(this, context));
+    return ts.createTypeReferenceNode(typeNode.typeName, typeArgs);
   }
 
   visitArrayType(type: ArrayType, context: Context): ts.ArrayTypeNode {
-    return ts.createArrayTypeNode(type.visitType(this, context));
+    return ts.createArrayTypeNode(this.translateType(type, context));
   }
 
   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 typeArgs = type.valueType !== null ?
+        this.translateType(type.valueType, context) :
+        ts.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword);
     const indexSignature = ts.createIndexSignature(undefined, undefined, [parameter], typeArgs);
     return ts.createTypeLiteralNode([indexSignature]);
   }
 
-  visitReadVarExpr(ast: ReadVarExpr, context: Context): string {
+  visitReadVarExpr(ast: ReadVarExpr, context: Context): ts.TypeQueryNode {
     if (ast.name === null) {
       throw new Error(`ReadVarExpr with no variable name in type`);
     }
-    return ast.name;
+    return ts.createTypeQueryNode(ts.createIdentifier(ast.name));
   }
 
   visitWriteVarExpr(expr: WriteVarExpr, context: Context): never {
@@ -486,15 +497,15 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
     throw new Error('Method not implemented.');
   }
 
-  visitLiteralExpr(ast: LiteralExpr, context: Context): ts.LiteralExpression {
+  visitLiteralExpr(ast: LiteralExpr, context: Context): ts.LiteralTypeNode {
-    return ts.createLiteral(ast.value as string);
+    return ts.createLiteralTypeNode(ts.createLiteral(ast.value as string));
   }
 
-  visitLocalizedString(ast: LocalizedString, context: Context): ts.Expression {
+  visitLocalizedString(ast: LocalizedString, context: Context): never {
     throw new Error('Method not implemented.');
   }
 
-  visitExternalExpr(ast: ExternalExpr, context: Context): ts.Node {
+  visitExternalExpr(ast: ExternalExpr, context: Context): ts.EntityName|ts.TypeReferenceNode {
     if (ast.value.moduleName === null || ast.value.name === null) {
       throw new Error(`Import unknown module or symbol`);
     }
@@ -506,11 +517,9 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
         ts.createQualifiedName(ts.createIdentifier(moduleImport), symbolIdentifier) :
         symbolIdentifier;
 
-    if (ast.typeParams === null) {
+    const typeArguments = ast.typeParams !== null ?
-      return typeName;
+        ast.typeParams.map(type => this.translateType(type, context)) :
-    }
+        undefined;
-
-    const typeArguments = ast.typeParams.map(type => type.visitType(this, context));
     return ts.createTypeReferenceNode(typeName, typeArguments);
   }
 
@@ -543,24 +552,31 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
   }
 
   visitLiteralArrayExpr(ast: LiteralArrayExpr, context: Context): ts.TupleTypeNode {
-    const values = ast.entries.map(expr => expr.visitExpression(this, context));
+    const values = ast.entries.map(expr => this.translateExpression(expr, context));
     return ts.createTupleTypeNode(values);
   }
 
-  visitLiteralMapExpr(ast: LiteralMapExpr, context: Context): ts.ObjectLiteralExpression {
+  visitLiteralMapExpr(ast: LiteralMapExpr, context: Context): ts.TypeLiteralNode {
     const entries = ast.entries.map(entry => {
       const {key, quoted} = entry;
-      const value = entry.value.visitExpression(this, context);
+      const type = this.translateExpression(entry.value, context);
-      return ts.createPropertyAssignment(quoted ? `'${key}'` : key, value);
+      return ts.createPropertySignature(
+          /* modifiers */ undefined,
+          /* name */ quoted ? ts.createStringLiteral(key) : key,
+          /* questionToken */ undefined,
+          /* type */ type,
+          /* initializer */ undefined);
     });
-    return ts.createObjectLiteral(entries);
+    return ts.createTypeLiteralNode(entries);
   }
 
   visitCommaExpr(ast: CommaExpr, context: Context) { throw new Error('Method not implemented.'); }
 
-  visitWrappedNodeExpr(ast: WrappedNodeExpr<any>, context: Context): ts.Identifier {
+  visitWrappedNodeExpr(ast: WrappedNodeExpr<any>, context: Context): ts.TypeNode {
     const node: ts.Node = ast.node;
-    if (ts.isIdentifier(node)) {
+    if (ts.isEntityName(node)) {
+      return ts.createTypeReferenceNode(node, /* typeArguments */ undefined);
+    } else if (ts.isTypeNode(node)) {
       return node;
     } else {
       throw new Error(
@@ -573,6 +589,24 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
         ast.expr, this.imports, NOOP_DEFAULT_IMPORT_RECORDER, ts.ScriptTarget.ES2015);
     return ts.createTypeQueryNode(expr as ts.Identifier);
   }
+
+  private translateType(expr: Type, context: Context): ts.TypeNode {
+    const typeNode = expr.visitType(this, context);
+    if (!ts.isTypeNode(typeNode)) {
+      throw new Error(
+          `A Type must translate to a TypeNode, but was ${ts.SyntaxKind[typeNode.kind]}`);
+    }
+    return typeNode;
+  }
+
+  private translateExpression(expr: Expression, context: Context): ts.TypeNode {
+    const typeNode = expr.visitExpression(this, context);
+    if (!ts.isTypeNode(typeNode)) {
+      throw new Error(
+          `An Expression must translate to a TypeNode, but was ${ts.SyntaxKind[typeNode.kind]}`);
+    }
+    return typeNode;
+  }
 }
 
 /**

packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ExpressionType, ExternalExpr, ReadVarExpr, Type} from '@angular/compiler';
+import {ExpressionType, ExternalExpr, Type, WrappedNodeExpr} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {NOOP_DEFAULT_IMPORT_RECORDER, Reference, ReferenceEmitter} from '../../imports';
@@ -124,13 +124,13 @@ export class Environment {
       return this.outputHelperIdent;
     }
 
-    const eventEmitter = this.referenceExternalType(
-        '@angular/core', 'EventEmitter', [new ExpressionType(new ReadVarExpr('T'))]);
-
     const outputHelperIdent = ts.createIdentifier('_outputHelper');
     const genericTypeDecl = ts.createTypeParameterDeclaration('T');
     const genericTypeRef = ts.createTypeReferenceNode('T', /* typeParameters */ undefined);
 
+    const eventEmitter = this.referenceExternalType(
+        '@angular/core', 'EventEmitter', [new ExpressionType(new WrappedNodeExpr(genericTypeRef))]);
+
     // Declare a type that has a `subscribe` method that carries over type `T` as parameter
     // into the callback. The below code generates the following type literal:
     // `{subscribe(cb: (event: T) => any): void;}`

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -666,7 +666,7 @@ runInEachFileSystem(os => {
       const dtsContents = env.getContents('test.d.ts');
       expect(dtsContents)
           .toContain(
-              `static ɵdir: i0.ɵɵDirectiveDefWithMeta<TestBase, never, never, { 'input': "input" }, {}, never>;`);
+              `static ɵdir: i0.ɵɵDirectiveDefWithMeta<TestBase, never, never, { "input": "input"; }, {}, never>;`);
     });
 
     it('should compile NgModules without errors', () => {