From f2bfa3151ee7ecb335665d55741387bd67ebee9d Mon Sep 17 00:00:00 2001 From: aparziale Date: Mon, 7 Apr 2025 22:18:46 +0200 Subject: [PATCH] fix(core): fix ng generate @angular/core:output-migration. Fixes angular#58650 (#60763) Fixes #58650 - Insert a TODO comment for empty emit (without parameter). PR Close #60763 --- .../output-migration/output-migration.spec.ts | 59 ++++++++++++++ .../output-migration/output-migration.ts | 77 +++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/packages/core/schematics/migrations/output-migration/output-migration.spec.ts b/packages/core/schematics/migrations/output-migration/output-migration.spec.ts index 0078d636ca8..fc9f059c8ee 100644 --- a/packages/core/schematics/migrations/output-migration/output-migration.spec.ts +++ b/packages/core/schematics/migrations/output-migration/output-migration.spec.ts @@ -169,6 +169,65 @@ describe('outputs', () => { }); }); + it('should not insert a TODO comment for emit function with no type', async () => { + await verify({ + before: ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive() + export class TestDir { + @Output() someChange = new EventEmitter(); + + someMethod(): void { + this.someChange.emit(); + } + } + `, + after: ` + import {Directive, output} from '@angular/core'; + + @Directive() + export class TestDir { + readonly someChange = output(); + + someMethod(): void { + this.someChange.emit(); + } + } + `, + }); + }); + + it('should insert a TODO comment for emit function with type', async () => { + await verify({ + before: ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive() + export class TestDir { + @Output() someChange = new EventEmitter(); + + someMethod(): void { + this.someChange.emit(); + } + } + `, + after: ` + import {Directive, output} from '@angular/core'; + + @Directive() + export class TestDir { + readonly someChange = output(); + + someMethod(): void { + // TODO: The 'emit' function requires a mandatory string argument + this.someChange.emit(); + } + } + `, + }); + }); + it('should migrate multiple outputs', async () => { await verifyDeclaration({ before: diff --git a/packages/core/schematics/migrations/output-migration/output-migration.ts b/packages/core/schematics/migrations/output-migration/output-migration.ts index 04522b30271..3a34dca85b6 100644 --- a/packages/core/schematics/migrations/output-migration/output-migration.ts +++ b/packages/core/schematics/migrations/output-migration/output-migration.ts @@ -16,6 +16,7 @@ import { ProjectFileID, Replacement, Serializable, + TextUpdate, TsurgeFunnelMigration, } from '../../utils/tsurge'; @@ -216,6 +217,8 @@ export class OutputMigration extends TsurgeFunnelMigration< } } + addCommentForEmptyEmit(node, info, checker, reflector, dtsReader, outputFieldReplacements); + // detect imports of test runners if (isTestRunnerImport(node)) { isTestFile = true; @@ -449,3 +452,77 @@ function addOutputReplacement( } existingReplacements.replacements.push(...replacements); } + +function addCommentForEmptyEmit( + node: ts.Node, + info: ProgramInfo, + checker: ts.TypeChecker, + reflector: TypeScriptReflectionHost, + dtsReader: DtsMetadataReader, + outputFieldReplacements: Record, +): void { + if (!isEmptyEmitCall(node)) return; + + const propertyAccess = getPropertyAccess(node); + if (!propertyAccess) return; + + const symbol = checker.getSymbolAtLocation(propertyAccess.name); + if (!symbol || !symbol.declarations?.length) return; + + const propertyDeclaration = isTargetOutputDeclaration( + propertyAccess, + checker, + reflector, + dtsReader, + ); + if (!propertyDeclaration) return; + + const eventEmitterType = getEventEmitterArgumentType(propertyDeclaration); + if (!eventEmitterType) return; + + const id = getUniqueIdForProperty(info, propertyDeclaration); + const file = projectFile(node.getSourceFile(), info); + const formatter = getFormatterText(node); + const todoReplacement: TextUpdate = new TextUpdate({ + toInsert: `${formatter.indent}// TODO: The 'emit' function requires a mandatory ${eventEmitterType} argument\n`, + end: formatter.lineStartPos, + position: formatter.lineStartPos, + }); + + addOutputReplacement(outputFieldReplacements, id, file, new Replacement(file, todoReplacement)); +} + +function isEmptyEmitCall(node: ts.Node): node is ts.CallExpression { + return ( + ts.isCallExpression(node) && + ts.isPropertyAccessExpression(node.expression) && + node.expression.name.text === 'emit' && + node.arguments.length === 0 + ); +} + +function getPropertyAccess(node: ts.CallExpression): ts.PropertyAccessExpression | null { + const propertyAccessExpression = (node.expression as ts.PropertyAccessExpression).expression; + return ts.isPropertyAccessExpression(propertyAccessExpression) ? propertyAccessExpression : null; +} + +function getEventEmitterArgumentType(propertyDeclaration: ts.PropertyDeclaration): string | null { + const initializer = propertyDeclaration.initializer; + if (!initializer || !ts.isNewExpression(initializer)) return null; + + const isEventEmitter = + ts.isIdentifier(initializer.expression) && initializer.expression.getText() === 'EventEmitter'; + + if (!isEventEmitter) return null; + + const [typeArg] = initializer.typeArguments ?? []; + return typeArg ? typeArg.getText() : null; +} + +function getFormatterText(node: ts.Node): {indent: string; lineStartPos: number} { + const sourceFile = node.getSourceFile(); + const {line} = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + const lineStartPos = sourceFile.getPositionOfLineAndCharacter(line, 0); + const indent = sourceFile.text.slice(lineStartPos, node.getStart()); + return {indent, lineStartPos}; +}