From bb6e0aab1be7c2beaa7e4dc6e03e3e4d8bca5ee7 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Wed, 30 Oct 2024 10:02:40 +0100
Subject: [PATCH] refactor(migrations): make it easier to delete nodes
 including comments (#58427)

We were repeating the logic that deletes a node together with all its comments in a few different places. These changes consolidate the logic under `ChangeTracker.removeNode`.

PR Close #58427
---
 .../ng-generate/inject-migration/migration.ts       | 13 ++++---------
 packages/core/schematics/utils/change_tracker.ts    |  9 ++++++---
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/packages/core/schematics/ng-generate/inject-migration/migration.ts b/packages/core/schematics/ng-generate/inject-migration/migration.ts
index 8cb1cf50944..1f813512f0b 100644
--- a/packages/core/schematics/ng-generate/inject-migration/migration.ts
+++ b/packages/core/schematics/ng-generate/inject-migration/migration.ts
@@ -163,14 +163,14 @@ function migrateClass(
   for (const member of node.members) {
     if (ts.isConstructorDeclaration(member) && member !== constructor) {
       removedMembers.add(member);
-      tracker.replaceText(sourceFile, member.getFullStart(), member.getFullWidth(), '');
+      tracker.removeNode(member, true);
     }
   }
 
   if (canRemoveConstructor(options, constructor, removedStatementCount, superCall)) {
     // Drop the constructor if it was empty.
     removedMembers.add(constructor);
-    tracker.replaceText(sourceFile, constructor.getFullStart(), constructor.getFullWidth(), '');
+    tracker.removeNode(constructor, true);
   } else {
     // If the constructor contains any statements, only remove the parameters.
     // We always do this no matter what is passed into `backwardsCompatibleConstructors`.
@@ -714,12 +714,7 @@ function applyInternalOnlyChanges(
       property.type,
       initializer,
     );
-    tracker.replaceText(
-      statement.getSourceFile(),
-      statement.getFullStart(),
-      statement.getFullWidth(),
-      '',
-    );
+    tracker.removeNode(statement, true);
     tracker.replaceNode(property, newProperty);
     removedStatements.add(statement);
   });
@@ -728,7 +723,7 @@ function applyInternalOnlyChanges(
     prependToClass.push(
       memberIndentation + printer.printNode(ts.EmitHint.Unspecified, decl, decl.getSourceFile()),
     );
-    tracker.replaceText(decl.getSourceFile(), decl.getFullStart(), decl.getFullWidth(), '');
+    tracker.removeNode(decl, true);
   });
 
   // If we added any hoisted properties, separate them visually with a new line.
diff --git a/packages/core/schematics/utils/change_tracker.ts b/packages/core/schematics/utils/change_tracker.ts
index 3d1f5549237..5d35cbc1912 100644
--- a/packages/core/schematics/utils/change_tracker.ts
+++ b/packages/core/schematics/utils/change_tracker.ts
@@ -97,11 +97,14 @@ export class ChangeTracker {
   /**
    * Removes the text of an AST node from a file.
    * @param node Node whose text should be removed.
+   * @param useFullOffsets Whether to remove the node using its full offset (e.g. `getFullStart`
+   * rather than `fullStart`). This has the advantage of removing any comments that may be tied
+   * to the node, but can lead to too much code being deleted.
    */
-  removeNode(node: ts.Node): void {
+  removeNode(node: ts.Node, useFullOffsets = false): void {
     this._trackChange(node.getSourceFile(), {
-      start: node.getStart(),
-      removeLength: node.getWidth(),
+      start: useFullOffsets ? node.getFullStart() : node.getStart(),
+      removeLength: useFullOffsets ? node.getFullWidth() : node.getWidth(),
       text: '',
     });
   }