refactor(ivy): mark synthetic decorators explicitly (#33362)

In ngcc's migration system, synthetic decorators can be injected into a
compilation to ensure that certain classes are compiled with Angular
logic, where the original library code did not include the necessary
decorators. Prior to this change, synthesized decorators would have a
fake AST structure as associated node and a made-up identifier. In
theory, this may introduce issues downstream:

1) a decorator's node is used for diagnostics, so it must have position
information. Having fake AST nodes without a position is therefore a
problem. Note that this is currently not a problem in practice, as
injected synthesized decorators would not produce any diagnostics.

2) the decorator's identifier should refer to an imported symbol.
Therefore, it is required that the symbol is actually imported.
Moreover, bundle formats such as UMD and CommonJS use namespaces for
imports, so a bare `ts.Identifier` would not be suitable to use as
identifier. This was also not a problem in practice, as the identifier
is only used in the `setClassMetadata` generated code, which is omitted
for synthetically injected decorators.

To remedy these potential issues, this commit makes a decorator's
identifier optional and switches its node over from a fake AST structure
to the class' name.

PR Close #33362

packages/compiler-cli/ngcc/src/migrations/utils.ts

@@ -37,48 +37,48 @@ export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator {
     // TODO: At the moment ngtsc does not accept a directive with no selector
     ts.createPropertyAssignment('selector', ts.createStringLiteral('NGCC_DUMMY')),
   ]);
-  const decoratorType = ts.createIdentifier('Directive');
-  const decoratorNode = ts.createObjectLiteral([
-    ts.createPropertyAssignment('type', decoratorType),
-    ts.createPropertyAssignment('args', ts.createArrayLiteral([selectorArg])),
-  ]);
-
-  setParentPointers(clazz.getSourceFile(), decoratorNode);
 
   return {
     name: 'Directive',
-    identifier: decoratorType,
+    identifier: null,
     import: {name: 'Directive', from: '@angular/core'},
-    node: decoratorNode,
-    args: [selectorArg],
+    node: null,
+    synthesizedFor: clazz.name,
+    args: [reifySourceFile(selectorArg)],
   };
 }
 
 /**
  * Create an empty `Injectable` decorator that will be associated with the `clazz`.
  */
 export function createInjectableDecorator(clazz: ClassDeclaration): Decorator {
-  const decoratorType = ts.createIdentifier('Injectable');
-  const decoratorNode = ts.createObjectLiteral([
-    ts.createPropertyAssignment('type', decoratorType),
-    ts.createPropertyAssignment('args', ts.createArrayLiteral([])),
-  ]);
-
-  setParentPointers(clazz.getSourceFile(), decoratorNode);
-
   return {
     name: 'Injectable',
-    identifier: decoratorType,
+    identifier: null,
     import: {name: 'Injectable', from: '@angular/core'},
-    node: decoratorNode,
+    node: null,
+    synthesizedFor: clazz.name,
     args: [],
   };
 }
 
+const EMPTY_SF = ts.createSourceFile('(empty)', '', ts.ScriptTarget.Latest);
+
 /**
- * Ensure that a tree of AST nodes have their parents wired up.
+ * Takes a `ts.Expression` and returns the same `ts.Expression`, but with an associated
+ * `ts.SourceFile`.
+ *
+ * This transformation is necessary to use synthetic `ts.Expression`s with the `PartialEvaluator`,
+ * and many decorator arguments are interpreted in this way.
  */
-export function setParentPointers(parent: ts.Node, child: ts.Node): void {
-  child.parent = parent;
-  ts.forEachChild(child, grandchild => setParentPointers(child, grandchild));
+function reifySourceFile(expr: ts.Expression): ts.Expression {
+  const printer = ts.createPrinter();
+  const exprText = printer.printNode(ts.EmitHint.Unspecified, expr, EMPTY_SF);
+  const sf = ts.createSourceFile(
+      '(synthetic)', `const expr = ${exprText};`, ts.ScriptTarget.Latest, true, ts.ScriptKind.JS);
+  const stmt = sf.statements[0];
+  if (!ts.isVariableStatement(stmt)) {
+    throw new Error(`Expected VariableStatement, got ${ts.SyntaxKind[stmt.kind]}`);
+  }
+  return stmt.declarationList.declarations[0].initializer !;
 }

packages/compiler-cli/ngcc/src/rendering/renderer.ts

@@ -130,6 +130,9 @@ export class Renderer {
       }
 
       clazz.decorators.forEach(dec => {
+        if (dec.node === null) {
+          return;
+        }
         const decoratorArray = dec.node.parent !;
         if (!decoratorsToRemove.has(decoratorArray)) {
           decoratorsToRemove.set(decoratorArray, [dec.node]);

packages/compiler-cli/ngcc/test/host/commonjs_host_import_helper_spec.ts

@@ -86,7 +86,7 @@ exports.OtherDirective = OtherDirective;
 
         const decorator = decorators[0];
         expect(decorator.name).toEqual('Directive');
-        expect(decorator.identifier.getText()).toEqual('core.Directive');
+        expect(decorator.identifier !.getText()).toEqual('core.Directive');
         expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
         expect(decorator.args !.map(arg => arg.getText())).toEqual([
           '{ selector: \'[someDirective]\' }',

packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts

@@ -175,7 +175,7 @@ runInEachFileSystem(() => {
 
             const decorator = decorators[0];
             expect(decorator.name).toEqual('Directive');
-            expect(decorator.identifier.getText()).toEqual('Directive');
+            expect(decorator.identifier !.getText()).toEqual('Directive');
             expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
             expect(decorator.args !.map(arg => arg.getText())).toEqual([
               '{ selector: \'[someDirective]\' }',
@@ -197,7 +197,7 @@ runInEachFileSystem(() => {
 
                const decorator = decorators[0];
                expect(decorator.name).toEqual('Directive');
-               expect(decorator.identifier.getText()).toEqual('Directive');
+               expect(decorator.identifier !.getText()).toEqual('Directive');
                expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
                expect(decorator.args !.map(arg => arg.getText())).toEqual([
                  '{ selector: \'[someDirective]\' }',
@@ -219,7 +219,7 @@ runInEachFileSystem(() => {
 
             const decorator = decorators[0];
             expect(decorator.name).toEqual('Directive');
-            expect(decorator.identifier.getText()).toEqual('Directive');
+            expect(decorator.identifier !.getText()).toEqual('Directive');
             expect(decorator.import).toEqual({name: 'Directive', from: './directives'});
             expect(decorator.args !.map(arg => arg.getText())).toEqual([
               '{ selector: \'[someDirective]\' }',
@@ -433,7 +433,7 @@ runInEachFileSystem(() => {
             const classNode = getDeclaration(
                 program, _('/some_directive.js'), 'SomeDirective', isNamedVariableDeclaration);
             const classDecorators = host.getDecoratorsOfDeclaration(classNode) !;
-            const decoratorNode = classDecorators[0].node;
+            const decoratorNode = classDecorators[0].node !;
             const identifierOfDirective =
                 ts.isCallExpression(decoratorNode) && ts.isIdentifier(decoratorNode.expression) ?
                 decoratorNode.expression :

packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts

@@ -202,7 +202,7 @@ export { SomeDirective };
 
             const decorator = decorators[0];
             expect(decorator.name).toEqual('Directive');
-            expect(decorator.identifier.getText()).toEqual('Directive');
+            expect(decorator.identifier !.getText()).toEqual('Directive');
             expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
             expect(decorator.args !.map(arg => arg.getText())).toEqual([
               '{ selector: \'[someDirective]\' }',
@@ -224,7 +224,7 @@ export { SomeDirective };
 
                const decorator = decorators[0];
                expect(decorator.name).toEqual('Directive');
-               expect(decorator.identifier.getText()).toEqual('Directive');
+               expect(decorator.identifier !.getText()).toEqual('Directive');
                expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
                expect(decorator.args !.map(arg => arg.getText())).toEqual([
                  '{ selector: \'[someDirective]\' }',
@@ -245,7 +245,7 @@ export { SomeDirective };
 
             const decorator = decorators[0];
             expect(decorator.name).toEqual('Directive');
-            expect(decorator.identifier.getText()).toEqual('Directive');
+            expect(decorator.identifier !.getText()).toEqual('Directive');
             expect(decorator.import).toEqual({name: 'Directive', from: './directives'});
             expect(decorator.args !.map(arg => arg.getText())).toEqual([
               '{ selector: \'[someDirective]\' }',
@@ -470,7 +470,7 @@ export { SomeDirective };
             const classNode = getDeclaration(
                 program, _('/some_directive.js'), 'SomeDirective', isNamedVariableDeclaration);
             const classDecorators = host.getDecoratorsOfDeclaration(classNode) !;
-            const decoratorNode = classDecorators[0].node;
+            const decoratorNode = classDecorators[0].node !;
 
             const identifierOfDirective =
                 ts.isCallExpression(decoratorNode) && ts.isIdentifier(decoratorNode.expression) ?

packages/compiler-cli/ngcc/test/host/umd_host_import_helper_spec.ts

@@ -79,7 +79,7 @@ runInEachFileSystem(() => {
 
         const decorator = decorators[0];
         expect(decorator.name).toEqual('Directive');
-        expect(decorator.identifier.getText()).toEqual('core.Directive');
+        expect(decorator.identifier !.getText()).toEqual('core.Directive');
         expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
         expect(decorator.args !.map(arg => arg.getText())).toEqual([
           '{ selector: \'[someDirective]\' }',

packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts

@@ -356,7 +356,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !;
            const decorator = compiledClass.decorators ![0];
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString())
                .not.toContain(`{ type: core.Directive, args: [{ selector: '[a]' }] },`);
@@ -377,7 +377,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
            const decorator = compiledClass.decorators ![0];
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString())
                .toContain(`{ type: core.Directive, args: [{ selector: '[a]' }] },`);
@@ -398,7 +398,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !;
            const decorator = compiledClass.decorators ![0];
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT');
            expect(output.toString())
@@ -423,7 +423,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !;
            const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`);
            expect(output.toString()).toContain(`OtherA()`);
@@ -440,7 +440,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
            const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`);
            expect(output.toString()).toContain(`OtherA()`);
@@ -458,7 +458,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !;
            const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`);
            expect(output.toString()).toContain(`OtherA()`);

packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts

@@ -345,7 +345,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !;
            const decorator = compiledClass.decorators ![0];
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString())
                .not.toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`);
@@ -364,7 +364,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
            const decorator = compiledClass.decorators ![0];
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`);
            expect(output.toString()).toContain(`{ type: OtherA }`);
@@ -383,7 +383,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !;
            const decorator = compiledClass.decorators ![0];
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT');
            expect(output.toString()).toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`);
@@ -406,7 +406,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !;
            const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`);
            expect(output.toString()).toContain(`OtherA()`);
@@ -423,7 +423,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
            const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`);
            expect(output.toString()).toContain(`OtherA()`);
@@ -441,7 +441,7 @@ SOME DEFINITION TEXT
                decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !;
            const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
            const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
-           decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
+           decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
            renderer.removeDecorators(output, decoratorsToRemove);
            expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`);
            expect(output.toString()).toContain(`OtherA()`);