diff --git a/packages/core/schematics/migrations/output-migration/BUILD.bazel b/packages/core/schematics/migrations/output-migration/BUILD.bazel new file mode 100644 index 000000000000..7573766f2a4c --- /dev/null +++ b/packages/core/schematics/migrations/output-migration/BUILD.bazel @@ -0,0 +1,42 @@ +load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") + +ts_library( + name = "migration", + srcs = glob( + ["**/*.ts"], + exclude = ["*.spec.ts"], + ), + deps = [ + "//packages/compiler", + "//packages/compiler-cli", + "//packages/compiler-cli/private", + "//packages/compiler-cli/src/ngtsc/annotations", + "//packages/compiler-cli/src/ngtsc/annotations/directive", + "//packages/compiler-cli/src/ngtsc/imports", + "//packages/compiler-cli/src/ngtsc/metadata", + "//packages/compiler-cli/src/ngtsc/reflection", + "//packages/core/schematics/utils/tsurge", + "@npm//@types/node", + "@npm//typescript", + ], +) + +ts_library( + name = "test_lib", + testonly = True, + srcs = glob( + ["**/*.spec.ts"], + ), + deps = [ + ":migration", + "//packages/compiler-cli", + "//packages/compiler-cli/src/ngtsc/file_system/testing", + "//packages/core/schematics/utils/tsurge", + ], +) + +jasmine_node_test( + name = "test", + srcs = [":test_lib"], + env = {"FORCE_COLOR": "3"}, +) diff --git a/packages/core/schematics/migrations/output-migration/output-migration.spec.ts b/packages/core/schematics/migrations/output-migration/output-migration.spec.ts new file mode 100644 index 000000000000..274535805a89 --- /dev/null +++ b/packages/core/schematics/migrations/output-migration/output-migration.spec.ts @@ -0,0 +1,217 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {initMockFileSystem} from '../../../../compiler-cli/src/ngtsc/file_system/testing'; +import {runTsurgeMigration} from '../../utils/tsurge/testing'; +import {absoluteFrom} from '@angular/compiler-cli'; +import {OutputMigration} from './output-migration'; + +describe('outputs', () => { + beforeEach(() => { + initMockFileSystem('Native'); + }); + + describe('outputs migration', () => { + describe('EventEmitter declarations without problematic access patterns', () => { + it('should migrate declaration with a primitive type hint', () => { + verifyDeclaration({ + before: '@Output() readonly someChange = new EventEmitter();', + after: 'readonly someChange = output();', + }); + }); + + it('should migrate declaration with complex type hint', () => { + verifyDeclaration({ + before: '@Output() readonly someChange = new EventEmitter();', + after: 'readonly someChange = output();', + }); + }); + + it('should migrate declaration without type hint', () => { + verifyDeclaration({ + before: '@Output() readonly someChange = new EventEmitter();', + after: 'readonly someChange = output();', + }); + }); + + it('should take alias into account', () => { + verifyDeclaration({ + before: `@Output({alias: 'otherChange'}) readonly someChange = new EventEmitter();`, + after: `readonly someChange = output({ alias: 'otherChange' });`, + }); + }); + + it('should support alias as statically analyzable reference', () => { + verify({ + before: ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + const aliasParam = { alias: 'otherChange' } as const; + + @Directive() + export class TestDir { + @Output(aliasParam) someChange = new EventEmitter(); + } + `, + after: ` + import { Directive, output } from '@angular/core'; + + const aliasParam = { alias: 'otherChange' } as const; + + @Directive() + export class TestDir { + readonly someChange = output(aliasParam); + } + `, + }); + }); + + it('should add readonly modifier', () => { + verifyDeclaration({ + before: '@Output() someChange = new EventEmitter();', + after: 'readonly someChange = output();', + }); + }); + + it('should respect visibility modifiers', () => { + verifyDeclaration({ + before: `@Output() protected someChange = new EventEmitter();`, + after: `protected readonly someChange = output();`, + }); + }); + + it('should migrate multiple outputs', () => { + // TODO: whitespace are messing up test verification + verifyDeclaration({ + before: `@Output() someChange1 = new EventEmitter(); + @Output() someChange2 = new EventEmitter();`, + after: `readonly someChange1 = output(); + readonly someChange2 = output();`, + }); + }); + + it('should migrate only EventEmitter outputs when multiple outputs exist', () => { + // TODO: whitespace are messing up test verification + verifyDeclaration({ + before: `@Output() someChange1 = new EventEmitter(); + @Output() someChange2 = new Subject();`, + after: `readonly someChange1 = output(); + @Output() someChange2 = new Subject();`, + }); + }); + }); + + describe('declarations _with_ problematic access patterns', () => { + it('should _not_ migrate outputs that are used with .pipe', () => { + verifyNoChange(` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive() + export class TestDir { + @Output() someChange = new EventEmitter(); + + someMethod() { + this.someChange.pipe(); + } + } + `); + }); + + it('should _not_ migrate outputs that are used with .next', () => { + verifyNoChange(` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive() + export class TestDir { + @Output() someChange = new EventEmitter(); + + someMethod() { + this.someChange.next('payload'); + } + } + `); + }); + + it('should _not_ migrate outputs that are used with .complete', () => { + verifyNoChange(` + import {Directive, Output, EventEmitter, OnDestroy} from '@angular/core'; + + @Directive() + export class TestDir implements OnDestroy { + @Output() someChange = new EventEmitter(); + + ngOnDestroy() { + this.someChange.complete(); + } + } + `); + }); + }); + }); + + describe('declarations other than EventEmitter', () => { + it('should _not_ migrate outputs that are initialized with sth else than EventEmitter', () => { + verify({ + before: populateDeclarationTestCase('@Output() protected someChange = new Subject();'), + after: populateDeclarationTestCase('@Output() protected someChange = new Subject();'), + }); + }); + }); +}); + +async function verifyDeclaration(testCase: {before: string; after: string}) { + verify({ + before: populateDeclarationTestCase(testCase.before), + after: populateExpectedResult(testCase.after), + }); +} + +async function verifyNoChange(beforeAndAfter: string) { + verify({before: beforeAndAfter, after: beforeAndAfter}); +} + +async function verify(testCase: {before: string; after: string}) { + const fs = await runTsurgeMigration(new OutputMigration(), [ + { + name: absoluteFrom('/app.component.ts'), + isProgramRootFile: true, + contents: testCase.before, + }, + ]); + + let actual = fs.readFile(absoluteFrom('/app.component.ts')); + + expect(actual).toBe(testCase.after); +} + +function populateDeclarationTestCase(declaration: string): string { + return ` + import { + Directive, + Output, + EventEmitter, + Subject + } from '@angular/core'; + + @Directive() + export class TestDir { + ${declaration} + } + `; +} + +function populateExpectedResult(declaration: string): string { + return ` + import { Directive, Subject, output } from '@angular/core'; + + @Directive() + export class TestDir { + ${declaration} + } + `; +} diff --git a/packages/core/schematics/migrations/output-migration/output-migration.ts b/packages/core/schematics/migrations/output-migration/output-migration.ts new file mode 100644 index 000000000000..420ef96a1f1e --- /dev/null +++ b/packages/core/schematics/migrations/output-migration/output-migration.ts @@ -0,0 +1,185 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import ts from 'typescript'; +import { + confirmAsSerializable, + ProgramInfo, + ProjectRelativePath, + Replacement, + Serializable, + TsurgeFunnelMigration, + projectRelativePath, +} from '../../utils/tsurge'; + +import {DtsMetadataReader} from '../../../../compiler-cli/src/ngtsc/metadata'; +import {TypeScriptReflectionHost} from '../../../../compiler-cli/src/ngtsc/reflection'; +import { + isOutputDeclaration, + OutputID, + getUniqueIdForProperty, + getTargetPropertyDeclaration, + extractSourceOutputDefinition, + isProblematicEventEmitterUsage, +} from './output_helpers'; +import {calculateImportReplacements, calculateDeclarationReplacements} from './output-replacements'; + +interface OutputMigrationData { + path: ProjectRelativePath; + replacements: Replacement[]; +} + +interface CompilationUnitData { + outputFields: Record; + problematicUsages: Record; + importReplacements: Record< + ProjectRelativePath, + {add: Replacement[]; addAndRemove: Replacement[]} + >; +} + +export class OutputMigration extends TsurgeFunnelMigration< + CompilationUnitData, + CompilationUnitData +> { + override async analyze({ + sourceFiles, + program, + projectDirAbsPath, + }: ProgramInfo): Promise> { + const outputFields: Record = {}; + const problematicUsages: Record = {}; + + const filesWithOutputDeclarations = new Set(); + + const checker = program.getTypeChecker(); + const reflector = new TypeScriptReflectionHost(checker); + const dtsReader = new DtsMetadataReader(checker, reflector); + + const outputMigrationVisitor = (node: ts.Node) => { + // detect output declarations + if (ts.isPropertyDeclaration(node)) { + const outputDef = extractSourceOutputDefinition(node, reflector, projectDirAbsPath); + if (outputDef !== null) { + const relativePath = projectRelativePath(node.getSourceFile(), projectDirAbsPath); + + filesWithOutputDeclarations.add(relativePath); + outputFields[outputDef.id] = { + path: relativePath, + replacements: calculateDeclarationReplacements( + projectDirAbsPath, + node, + outputDef.aliasParam, + ), + }; + } + } + + // detect unsafe access of the output property + if (isProblematicEventEmitterUsage(node)) { + const targetSymbol = checker.getSymbolAtLocation(node.expression); + if (targetSymbol !== undefined) { + const propertyDeclaration = getTargetPropertyDeclaration(targetSymbol); + if ( + propertyDeclaration !== null && + isOutputDeclaration(propertyDeclaration, reflector, dtsReader) + ) { + const id = getUniqueIdForProperty(projectDirAbsPath, propertyDeclaration); + problematicUsages[id] = true; + } + } + } + + ts.forEachChild(node, outputMigrationVisitor); + }; + + // calculate output migration replacements + for (const sf of sourceFiles) { + ts.forEachChild(sf, outputMigrationVisitor); + } + + // calculate import replacements but do so only for files that have output declarations + const importReplacements = calculateImportReplacements( + projectDirAbsPath, + sourceFiles.filter((sf) => + filesWithOutputDeclarations.has(projectRelativePath(sf, projectDirAbsPath)), + ), + ); + + return confirmAsSerializable({ + outputFields, + importReplacements, + problematicUsages, + }); + } + + override async merge(units: CompilationUnitData[]): Promise> { + const outputFields: Record = {}; + const importReplacements: Record< + ProjectRelativePath, + {add: Replacement[]; addAndRemove: Replacement[]} + > = {}; + const problematicUsages: Record = {}; + + for (const unit of units) { + for (const declIdStr of Object.keys(unit.outputFields)) { + const declId = declIdStr as OutputID; + // THINK: detect clash? Should we have an utility to merge data based on unique IDs? + outputFields[declId] = unit.outputFields[declId]; + } + + for (const pathStr of Object.keys(unit.importReplacements)) { + const path = pathStr as ProjectRelativePath; + importReplacements[path] = unit.importReplacements[path]; + } + + for (const declIdStr of Object.keys(unit.problematicUsages)) { + const declId = declIdStr as OutputID; + problematicUsages[declId] = unit.problematicUsages[declId]; + } + } + + return confirmAsSerializable({ + outputFields, + importReplacements, + problematicUsages, + }); + } + + override async migrate(globalData: CompilationUnitData): Promise { + const migratedFiles = new Set(); + const problematicFiles = new Set(); + + const replacements: Replacement[] = []; + for (const declIdStr of Object.keys(globalData.outputFields)) { + const declId = declIdStr as OutputID; + const outputField = globalData.outputFields[declId]; + + if (!globalData.problematicUsages[declId]) { + replacements.push(...outputField.replacements); + migratedFiles.add(outputField.path); + } else { + problematicFiles.add(outputField.path); + } + } + + for (const pathStr of Object.keys(globalData.importReplacements)) { + const path = pathStr as ProjectRelativePath; + if (migratedFiles.has(path)) { + const importReplacements = globalData.importReplacements[path]; + if (problematicFiles.has(path)) { + replacements.push(...importReplacements.add); + } else { + replacements.push(...importReplacements.addAndRemove); + } + } + } + + return replacements; + } +} diff --git a/packages/core/schematics/migrations/output-migration/output-replacements.ts b/packages/core/schematics/migrations/output-migration/output-replacements.ts new file mode 100644 index 000000000000..2d8a94ddf789 --- /dev/null +++ b/packages/core/schematics/migrations/output-migration/output-replacements.ts @@ -0,0 +1,102 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import ts from 'typescript'; +import { + Replacement, + TextUpdate, + ProjectRelativePath, + projectRelativePath, +} from '../../utils/tsurge'; +import {absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli'; +import {applyImportManagerChanges} from '../../utils/tsurge/helpers/apply_import_manager'; +import {ImportManager} from '../../../../compiler-cli/private/migrations'; + +const printer = ts.createPrinter(); + +export function calculateDeclarationReplacements( + projectDirAbsPath: AbsoluteFsPath, + node: ts.PropertyDeclaration, + aliasParam?: ts.Expression, +): Replacement[] { + const sf = node.getSourceFile(); + const payloadTypes = + node.initializer !== undefined && ts.isNewExpression(node.initializer) + ? node.initializer?.typeArguments + : undefined; + + const outputCall = ts.factory.createCallExpression( + ts.factory.createIdentifier('output'), + payloadTypes, + aliasParam ? [aliasParam] : [], + ); + + const existingModifiers = (node.modifiers ?? []).filter( + (modifier) => !ts.isDecorator(modifier) && modifier.kind !== ts.SyntaxKind.ReadonlyKeyword, + ); + + const updatedOutputDeclaration = ts.factory.updatePropertyDeclaration( + node, + // Think: this logic of dealing with modifiers is applicable to all signal-based migrations + ts.factory.createNodeArray([ + ...existingModifiers, + ts.factory.createModifier(ts.SyntaxKind.ReadonlyKeyword), + ]), + node.name, + undefined, + undefined, + outputCall, + ); + + return [ + new Replacement( + projectRelativePath(sf, projectDirAbsPath), + new TextUpdate({ + position: node.getStart(), + end: node.getEnd(), + toInsert: printer.printNode(ts.EmitHint.Unspecified, updatedOutputDeclaration, sf), + }), + ), + ]; +} + +export function calculateImportReplacements( + projectDirAbsPath: AbsoluteFsPath, + sourceFiles: ts.SourceFile[], +) { + const importReplacements: Record< + ProjectRelativePath, + {add: Replacement[]; addAndRemove: Replacement[]} + > = {}; + + const importManager = new ImportManager(); + + for (const sf of sourceFiles) { + const addOnly: Replacement[] = []; + const addRemove: Replacement[] = []; + + const absolutePath = absoluteFromSourceFile(sf); + importManager.addImport({ + requestedFile: sf, + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + }); + applyImportManagerChanges(importManager, addOnly, [sf], projectDirAbsPath); + + importManager.removeImport(sf, 'Output', '@angular/core'); + importManager.removeImport(sf, 'EventEmitter', '@angular/core'); + applyImportManagerChanges(importManager, addRemove, [sf], projectDirAbsPath); + + importReplacements[projectRelativePath(sf, projectDirAbsPath)] = { + add: addOnly, + addAndRemove: addRemove, + }; + } + + return importReplacements; +} diff --git a/packages/core/schematics/migrations/output-migration/output_helpers.ts b/packages/core/schematics/migrations/output-migration/output_helpers.ts new file mode 100644 index 000000000000..fe0917c5fd4f --- /dev/null +++ b/packages/core/schematics/migrations/output-migration/output_helpers.ts @@ -0,0 +1,125 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import ts from 'typescript'; +import * as path from 'path'; +import { + ReflectionHost, + ClassDeclaration, + Decorator, +} from '../../../../compiler-cli/src/ngtsc/reflection'; +import {DtsMetadataReader} from '../../../../compiler-cli/src/ngtsc/metadata'; +import {Reference} from '../../../../compiler-cli/src/ngtsc/imports'; +import {getAngularDecorators} from '../../../../compiler-cli/src/ngtsc/annotations'; + +import {UniqueID} from '../../utils/tsurge'; + +/** Branded type for unique IDs of Angular `@Output`s. */ +export type OutputID = UniqueID<'output-node'>; + +/** Type describing an extracted output query that can be migrated. */ +export interface ExtractedOutput { + id: OutputID; + aliasParam?: ts.Expression; +} + +/** + * Determines if the given node refers to a decorator-based output, and + * returns its resolved metadata if possible. + */ +export function extractSourceOutputDefinition( + node: ts.PropertyDeclaration, + reflector: ReflectionHost, + projectDirAbsPath: string, +): ExtractedOutput | null { + const outputDecorator = getOutputDecorator(node, reflector); + + if (outputDecorator !== null && isOutputDeclarationEligibleForMigration(node)) { + return { + id: getUniqueIdForProperty(projectDirAbsPath, node), + aliasParam: outputDecorator.args?.at(0), + }; + } + + return null; +} + +function isOutputDeclarationEligibleForMigration(node: ts.PropertyDeclaration) { + return ( + node.initializer !== undefined && + ts.isNewExpression(node.initializer) && + ts.isIdentifier(node.initializer.expression) && + node.initializer.expression.text === 'EventEmitter' + ); +} + +const problematicEventEmitterUsages = new Set(['pipe', 'next', 'complete']); +export function isProblematicEventEmitterUsage(node: ts.Node): node is ts.PropertyAccessExpression { + return ( + ts.isPropertyAccessExpression(node) && + ts.isIdentifier(node.name) && + problematicEventEmitterUsages.has(node.name.text) + ); +} + +/** Gets whether the given property is an Angular `@Output`. */ +export function isOutputDeclaration( + node: ts.PropertyDeclaration, + reflector: ReflectionHost, + dtsReader: DtsMetadataReader, +): boolean { + // `.d.ts` file, so we check the `static ecmp` metadata on the `declare class`. + if (node.getSourceFile().isDeclarationFile) { + if ( + !ts.isIdentifier(node.name) || + !ts.isClassDeclaration(node.parent) || + node.parent.name === undefined + ) { + return false; + } + + const ref = new Reference(node.parent as ClassDeclaration); + const directiveMeta = dtsReader.getDirectiveMetadata(ref); + return !!directiveMeta?.outputs.getByClassPropertyName(node.name.text); + } + + // `.ts` file, so we check for the `@Output()` decorator. + return getOutputDecorator(node, reflector) !== null; +} + +export function getTargetPropertyDeclaration( + targetSymbol: ts.Symbol, +): ts.PropertyDeclaration | null { + const valDeclaration = targetSymbol.valueDeclaration; + if (valDeclaration !== undefined && ts.isPropertyDeclaration(valDeclaration)) { + return valDeclaration; + } + return null; +} + +/** Returns Angular `@Output` decorator or null when a given property declaration is not an @Output */ +function getOutputDecorator( + node: ts.PropertyDeclaration, + reflector: ReflectionHost, +): Decorator | null { + const decorators = reflector.getDecoratorsOfDeclaration(node); + const ngDecorators = + decorators !== null ? getAngularDecorators(decorators, ['Output'], /* isCore */ false) : []; + + return ngDecorators.length > 0 ? ngDecorators[0] : null; +} + +// THINK: this utility + type is not specific to @Output, really, maybe move it to tsurge? +/** Computes an unique ID for a given Angular `@Output` property. */ +export function getUniqueIdForProperty( + projectDirAbsPath: string, + prop: ts.PropertyDeclaration, +): OutputID { + const fileId = path.relative(projectDirAbsPath, prop.getSourceFile().fileName); + return `${fileId}@@${prop.parent.name ?? 'unknown-class'}@@${prop.name.getText()}` as OutputID; +}