Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit ed70a21

Browse filesBrowse files
devversionthePunderWoman
authored andcommitted
Revert "fix(migrations): avoid applying the same replacements twice when cleaning up unused imports (#59656)" (#61421)
This reverts commit d66881d. PR Close #61421
1 parent 1d68113 commit ed70a21
Copy full SHA for ed70a21

File tree

Expand file treeCollapse file tree

1 file changed

+20
-60
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+20
-60
lines changed

‎packages/core/schematics/ng-generate/cleanup-unused-imports/unused_imports_migration.ts

Copy file name to clipboardExpand all lines: packages/core/schematics/ng-generate/cleanup-unused-imports/unused_imports_migration.ts
+20-60Lines changed: 20 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
confirmAsSerializable,
1212
ProgramInfo,
1313
projectFile,
14-
ProjectFileID,
1514
Replacement,
1615
Serializable,
1716
TextUpdate,
@@ -27,8 +26,8 @@ export interface CompilationUnitData {
2726
/** Text changes that should be performed. */
2827
replacements: Replacement[];
2928

30-
/** Identifiers that have been removed from each file. */
31-
removedIdentifiers: NodeID[];
29+
/** Total number of imports that were removed. */
30+
removedImports: number;
3231

3332
/** Total number of files that were changed. */
3433
changedFiles: number;
@@ -43,7 +42,7 @@ interface RemovalLocations {
4342
partialRemovals: Map<ts.ArrayLiteralExpression, Set<ts.Expression>>;
4443

4544
/** Text of all identifiers that have been removed. */
46-
allRemovedIdentifiers: Set<ts.Identifier>;
45+
allRemovedIdentifiers: Set<string>;
4746
}
4847

4948
/** Tracks how identifiers are used across a single file. */
@@ -59,9 +58,6 @@ interface UsageAnalysis {
5958
identifierCounts: Map<string, number>;
6059
}
6160

62-
/** ID of a node based on its location. */
63-
type NodeID = string & {__nodeID: true};
64-
6561
/** Migration that cleans up unused imports from a project. */
6662
export class UnusedImportsMigration extends TsurgeFunnelMigration<
6763
CompilationUnitData,
@@ -83,7 +79,7 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
8379
override async analyze(info: ProgramInfo): Promise<Serializable<CompilationUnitData>> {
8480
const nodePositions = new Map<ts.SourceFile, Set<string>>();
8581
const replacements: Replacement[] = [];
86-
const removedIdentifiers: NodeID[] = [];
82+
let removedImports = 0;
8783
let changedFiles = 0;
8884

8985
info.ngCompiler?.getDiagnostics().forEach((diag) => {
@@ -101,7 +97,7 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
10197
if (!nodePositions.has(diag.file)) {
10298
nodePositions.set(diag.file, new Set());
10399
}
104-
nodePositions.get(diag.file)!.add(this.getNodeID(diag.start, diag.length));
100+
nodePositions.get(diag.file)!.add(this.getNodeKey(diag.start, diag.length));
105101
}
106102
});
107103

@@ -110,15 +106,14 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
110106
const usageAnalysis = this.analyzeUsages(sourceFile, resolvedLocations);
111107

112108
if (resolvedLocations.allRemovedIdentifiers.size > 0) {
109+
removedImports += resolvedLocations.allRemovedIdentifiers.size;
113110
changedFiles++;
114-
resolvedLocations.allRemovedIdentifiers.forEach((identifier) => {
115-
removedIdentifiers.push(this.getNodeID(identifier.getStart(), identifier.getWidth()));
116-
});
117111
}
112+
118113
this.generateReplacements(sourceFile, resolvedLocations, usageAnalysis, info, replacements);
119114
});
120115

121-
return confirmAsSerializable({replacements, removedIdentifiers, changedFiles});
116+
return confirmAsSerializable({replacements, removedImports, changedFiles});
122117
}
123118

124119
override async migrate(globalData: CompilationUnitData) {
@@ -129,34 +124,10 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
129124
unitA: CompilationUnitData,
130125
unitB: CompilationUnitData,
131126
): Promise<Serializable<CompilationUnitData>> {
132-
const combinedReplacements: Replacement[] = [];
133-
const combinedRemovedIdentifiers: NodeID[] = [];
134-
const seenReplacements = new Set<string>();
135-
const seenIdentifiers = new Set<NodeID>();
136-
const changedFileIds = new Set<ProjectFileID>();
137-
138-
[unitA, unitB].forEach((unit) => {
139-
for (const replacement of unit.replacements) {
140-
const key = this.getReplacementID(replacement);
141-
changedFileIds.add(replacement.projectFile.id);
142-
if (!seenReplacements.has(key)) {
143-
seenReplacements.add(key);
144-
combinedReplacements.push(replacement);
145-
}
146-
}
147-
148-
for (const identifier of unit.removedIdentifiers) {
149-
if (!seenIdentifiers.has(identifier)) {
150-
seenIdentifiers.add(identifier);
151-
combinedRemovedIdentifiers.push(identifier);
152-
}
153-
}
154-
});
155-
156127
return confirmAsSerializable({
157-
replacements: combinedReplacements,
158-
removedIdentifiers: combinedRemovedIdentifiers,
159-
changedFiles: changedFileIds.size,
128+
replacements: [...unitA.replacements, ...unitB.replacements],
129+
removedImports: unitA.removedImports + unitB.removedImports,
130+
changedFiles: unitA.changedFiles + unitB.changedFiles,
160131
});
161132
}
162133

@@ -168,20 +139,14 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
168139

169140
override async stats(globalMetadata: CompilationUnitData) {
170141
return confirmAsSerializable({
171-
removedImports: globalMetadata.removedIdentifiers.length,
142+
removedImports: globalMetadata.removedImports,
172143
changedFiles: globalMetadata.changedFiles,
173144
});
174145
}
175146

176-
/** Gets an ID that can be used to look up a node based on its location. */
177-
private getNodeID(start: number, length: number): NodeID {
178-
return `${start}/${length}` as NodeID;
179-
}
180-
181-
/** Gets a unique ID for a replacement. */
182-
private getReplacementID(replacement: Replacement): string {
183-
const {position, end, toInsert} = replacement.update.data;
184-
return replacement.projectFile.id + '/' + position + '/' + end + '/' + toInsert;
147+
/** Gets a key that can be used to look up a node based on its location. */
148+
private getNodeKey(start: number, length: number): string {
149+
return `${start}/${length}`;
185150
}
186151

187152
/**
@@ -212,7 +177,7 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
212177
return;
213178
}
214179

215-
if (locations.has(this.getNodeID(node.getStart(), node.getWidth()))) {
180+
if (locations.has(this.getNodeKey(node.getStart(), node.getWidth()))) {
216181
// When the entire array needs to be cleared, the diagnostic is
217182
// reported on the property assignment, rather than an array element.
218183
if (
@@ -223,15 +188,15 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
223188
result.fullRemovals.add(parent.initializer);
224189
parent.initializer.elements.forEach((element) => {
225190
if (ts.isIdentifier(element)) {
226-
result.allRemovedIdentifiers.add(element);
191+
result.allRemovedIdentifiers.add(element.text);
227192
}
228193
});
229194
} else if (ts.isArrayLiteralExpression(parent)) {
230195
if (!result.partialRemovals.has(parent)) {
231196
result.partialRemovals.set(parent, new Set());
232197
}
233198
result.partialRemovals.get(parent)!.add(node);
234-
result.allRemovedIdentifiers.add(node);
199+
result.allRemovedIdentifiers.add(node.text);
235200
}
236201
}
237202
};
@@ -362,13 +327,8 @@ export class UnusedImportsMigration extends TsurgeFunnelMigration<
362327
names.forEach((symbolName, localName) => {
363328
// Note that in the `identifierCounts` lookup both zero and undefined
364329
// are valid and mean that the identifiers isn't being used anymore.
365-
if (!identifierCounts.get(localName)) {
366-
for (const identifier of allRemovedIdentifiers) {
367-
if (identifier.text === localName) {
368-
importManager.removeImport(sourceFile, symbolName, moduleName);
369-
break;
370-
}
371-
}
330+
if (allRemovedIdentifiers.has(localName) && !identifierCounts.get(localName)) {
331+
importManager.removeImport(sourceFile, symbolName, moduleName);
372332
}
373333
});
374334
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.