Skip to content

Commit d014503

Browse files
crisbetodylhunn
crisbeto
authored and
dylhunn
committed
fix(language-service): generate forwardRef for same file imports (#48898)
Adds some logic that will generate a `forwardRef` if necessary when automatically fixing an import. PR Close #48898
1 parent 59c0106 commit d014503

File tree

4 files changed

+86
-25
lines changed

4 files changed

+86
-25
lines changed

‎packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ runInEachFileSystem(() => {
270270
expect(imports.length).toBe(1);
271271
expect(imports[0].moduleSpecifier).toBe('./two');
272272
expect(imports[0].symbolName).toBe('TwoCmp');
273+
expect(imports[0].isForwardReference).toBe(false);
273274
});
274275

275276
it('for out of scope ngModules', () => {
@@ -326,6 +327,41 @@ runInEachFileSystem(() => {
326327
expect(imports.length).toBe(1);
327328
expect(imports[0].moduleSpecifier).toBe('./twomod');
328329
expect(imports[0].symbolName).toBe('TwoModule');
330+
expect(imports[0].isForwardReference).toBe(false);
331+
});
332+
333+
it('for forward references in the same file', () => {
334+
env.write('decls.ts', `
335+
import {Component} from '@angular/core';
336+
337+
@Component({
338+
standalone: true,
339+
selector: 'one-cmp',
340+
template: '<div></div>',
341+
})
342+
export class OneCmp {}
343+
344+
@Component({
345+
standalone: true,
346+
selector: 'two-cmp',
347+
template: '<div></div>',
348+
})
349+
export class TwoCmp {}
350+
`);
351+
const {program, checker} = env.driveTemplateTypeChecker();
352+
const sfOne = program.getSourceFile(_('/decls.ts'));
353+
expect(sfOne).not.toBeNull();
354+
const OneCmpClass = getClass(sfOne!, 'OneCmp');
355+
356+
const TwoCmpDir = checker.getPotentialTemplateDirectives(OneCmpClass)
357+
.filter(d => d.selector === 'two-cmp')[0];
358+
const imports =
359+
checker.getPotentialImportsFor(TwoCmpDir.ref, OneCmpClass, PotentialImportMode.Normal);
360+
361+
expect(imports.length).toBe(1);
362+
expect(imports[0].moduleSpecifier).toBeUndefined();
363+
expect(imports[0].symbolName).toBe('TwoCmp');
364+
expect(imports[0].isForwardReference).toBe(true);
329365
});
330366
});
331367
});

‎packages/language-service/src/codefixes/fix_missing_import.ts‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,33 @@ function getCodeActions(
7474
const currMatchSymbol = currMatch.tsSymbol.valueDeclaration!;
7575
const potentialImports =
7676
checker.getPotentialImportsFor(currMatch.ref, importOn, PotentialImportMode.Normal);
77-
for (let potentialImport of potentialImports) {
78-
let [fileImportChanges, importName] = updateImportsForTypescriptFile(
79-
tsChecker, importOn.getSourceFile(), potentialImport, currMatchSymbol.getSourceFile());
77+
for (const potentialImport of potentialImports) {
78+
const fileImportChanges: ts.TextChange[] = [];
79+
let importName: string;
80+
let forwardRefName: string|null = null;
81+
82+
if (potentialImport.moduleSpecifier) {
83+
const [importChanges, generatedImportName] = updateImportsForTypescriptFile(
84+
tsChecker, importOn.getSourceFile(), potentialImport.symbolName,
85+
potentialImport.moduleSpecifier, currMatchSymbol.getSourceFile());
86+
importName = generatedImportName;
87+
fileImportChanges.push(...importChanges);
88+
} else {
89+
if (potentialImport.isForwardReference) {
90+
// Note that we pass the `importOn` file twice since we know that the potential import
91+
// is within the same file, because it doesn't have a `moduleSpecifier`.
92+
const [forwardRefImports, generatedForwardRefName] = updateImportsForTypescriptFile(
93+
tsChecker, importOn.getSourceFile(), 'forwardRef', '@angular/core',
94+
importOn.getSourceFile());
95+
fileImportChanges.push(...forwardRefImports);
96+
forwardRefName = generatedForwardRefName;
97+
}
98+
importName = potentialImport.symbolName;
99+
}
100+
80101
// Always update the trait import, although the TS import might already be present.
81-
let traitImportChanges = updateImportsForAngularTrait(checker, importOn, importName);
102+
const traitImportChanges =
103+
updateImportsForAngularTrait(checker, importOn, importName, forwardRefName);
82104
if (traitImportChanges.length === 0) continue;
83105

84106
let description = `Import ${importName}`;

‎packages/language-service/src/ts_utils.ts‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,15 @@ export function updateObjectValueForKey(
178178
* If no update is needed, returns `null`.
179179
*/
180180
export function ensureArrayWithIdentifier(
181-
identifier: ts.Identifier, arr?: ts.ArrayLiteralExpression): ts.ArrayLiteralExpression|null {
181+
identifierText: string, expression: ts.Expression,
182+
arr?: ts.ArrayLiteralExpression): ts.ArrayLiteralExpression|null {
182183
if (arr === undefined) {
183-
return ts.factory.createArrayLiteralExpression([identifier]);
184+
return ts.factory.createArrayLiteralExpression([expression]);
184185
}
185-
if (arr.elements.find(v => ts.isIdentifier(v) && v.text === identifier.text)) {
186+
if (arr.elements.find(v => ts.isIdentifier(v) && v.text === identifierText)) {
186187
return null;
187188
}
188-
return ts.factory.updateArrayLiteralExpression(arr, [...arr.elements, identifier]);
189+
return ts.factory.updateArrayLiteralExpression(arr, [...arr.elements, expression]);
189190
}
190191

191192
export function moduleSpecifierPointsToFile(
@@ -316,26 +317,21 @@ export function standaloneTraitOrNgModule(
316317
* Returns the text changes, as well as the name with which the imported symbol can be referred to.
317318
*/
318319
export function updateImportsForTypescriptFile(
319-
tsChecker: ts.TypeChecker, file: ts.SourceFile, newImport: PotentialImport,
320+
tsChecker: ts.TypeChecker, file: ts.SourceFile, symbolName: string, moduleSpecifier: string,
320321
tsFileToImport: ts.SourceFile): [ts.TextChange[], string] {
321-
// If the expression is already imported, we can just return its name.
322-
if (newImport.moduleSpecifier === undefined) {
323-
return [[], newImport.symbolName];
324-
}
325-
326322
// The trait might already be imported, possibly under a different name. If so, determine the
327323
// local name of the imported trait.
328324
const allImports = findAllMatchingNodes(file, {filter: ts.isImportDeclaration});
329325
const existingImportName: string|null =
330-
hasImport(tsChecker, allImports, newImport.symbolName, tsFileToImport);
326+
hasImport(tsChecker, allImports, symbolName, tsFileToImport);
331327
if (existingImportName !== null) {
332328
return [[], existingImportName];
333329
}
334330

335331
// If the trait has not already been imported, we need to insert the new import.
336332
const existingImportDeclaration = allImports.find(
337333
decl => moduleSpecifierPointsToFile(tsChecker, decl.moduleSpecifier, tsFileToImport));
338-
const importName = nonCollidingImportName(allImports, newImport.symbolName);
334+
const importName = nonCollidingImportName(allImports, symbolName);
339335

340336
if (existingImportDeclaration !== undefined) {
341337
// Update an existing import declaration.
@@ -347,7 +343,7 @@ export function updateImportsForTypescriptFile(
347343
return [[], ''];
348344
}
349345
let span = {start: bindings.getStart(), length: bindings.getWidth()};
350-
const updatedBindings = updateImport(bindings, newImport.symbolName, importName);
346+
const updatedBindings = updateImport(bindings, symbolName, importName);
351347
const importString = printNode(updatedBindings, file);
352348
return [[{span, newText: importString}], importName];
353349
}
@@ -364,8 +360,7 @@ export function updateImportsForTypescriptFile(
364360
if (lastImport as any !== null) { // TODO: Why does the compiler insist this is null?
365361
span.start = lastImport!.getStart() + lastImport!.getWidth();
366362
}
367-
const newImportDeclaration =
368-
generateImport(newImport.symbolName, importName, newImport.moduleSpecifier);
363+
const newImportDeclaration = generateImport(symbolName, importName, moduleSpecifier);
369364
const importString = '\n' + printNode(newImportDeclaration, file);
370365
return [[{span, newText: importString}], importName];
371366
}
@@ -375,7 +370,8 @@ export function updateImportsForTypescriptFile(
375370
* `importName` to the list of imports on the decorator arguments.
376371
*/
377372
export function updateImportsForAngularTrait(
378-
checker: TemplateTypeChecker, trait: ts.ClassDeclaration, importName: string): ts.TextChange[] {
373+
checker: TemplateTypeChecker, trait: ts.ClassDeclaration, importName: string,
374+
forwardRefName: string|null): ts.TextChange[] {
379375
// Get the object with arguments passed into the primary Angular decorator for this trait.
380376
const decorator = checker.getPrimaryAngularDecorator(trait);
381377
if (decorator === null) {
@@ -393,7 +389,14 @@ export function updateImportsForAngularTrait(
393389
if (oldValue && !ts.isArrayLiteralExpression(oldValue)) {
394390
return oldValue;
395391
}
396-
const newArr = ensureArrayWithIdentifier(ts.factory.createIdentifier(importName), oldValue);
392+
const identifier = ts.factory.createIdentifier(importName);
393+
const expression = forwardRefName ?
394+
ts.factory.createCallExpression(
395+
ts.factory.createIdentifier(forwardRefName), undefined,
396+
[ts.factory.createArrowFunction(
397+
undefined, undefined, [], undefined, undefined, identifier)]) :
398+
identifier;
399+
const newArr = ensureArrayWithIdentifier(importName, expression, oldValue);
397400
updateRequired = newArr !== null;
398401
return newArr!;
399402
});

‎packages/language-service/test/ts_utils_spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,17 +163,17 @@ describe('TS util', () => {
163163
});
164164

165165
it('addElementToArrayLiteral', () => {
166-
let arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('foo'));
166+
let arr = ensureArrayWithIdentifier('foo', ts.factory.createIdentifier('foo'));
167167
arr = addElementToArrayLiteral(arr!, ts.factory.createIdentifier('bar'));
168168
expect(print(arr)).toEqual('[foo, bar]');
169169
});
170170

171171
it('ensureArrayWithIdentifier', () => {
172-
let arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('foo'));
172+
let arr = ensureArrayWithIdentifier('foo', ts.factory.createIdentifier('foo'));
173173
expect(print(arr!)).toEqual('[foo]');
174-
arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('bar'), arr!);
174+
arr = ensureArrayWithIdentifier('bar', ts.factory.createIdentifier('bar'), arr!);
175175
expect(print(arr!)).toEqual('[foo, bar]');
176-
arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('bar'), arr!);
176+
arr = ensureArrayWithIdentifier('bar', ts.factory.createIdentifier('bar'), arr!);
177177
expect(arr).toEqual(null);
178178
});
179179

0 commit comments

Comments
 (0)