Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(language-service): generate forwardRef for same file imports (#48898)

Browse source 
Adds some logic that will generate a `forwardRef` if necessary when automatically fixing an import.

PR Close #48898
This commit is contained in:
Kristiyan Kostadinov 2023-02-01 10:01:35 +01:00 committed by Dylan Hunn
parent 59c0106654
commit d0145033bd
4 changed files with 86 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts
View file

@ -270,6 +270,7 @@ runInEachFileSystem(() => {
expect(imports.length).toBe(1);
expect(imports[0].moduleSpecifier).toBe('./two');
expect(imports[0].symbolName).toBe('TwoCmp');
expect(imports[0].isForwardReference).toBe(false);
});
it('for out of scope ngModules', () => {
@ -326,6 +327,41 @@ runInEachFileSystem(() => {
expect(imports.length).toBe(1);
expect(imports[0].moduleSpecifier).toBe('./twomod');
expect(imports[0].symbolName).toBe('TwoModule');
expect(imports[0].isForwardReference).toBe(false);
});
it('for forward references in the same file', () => {
env.write('decls.ts', `
import {Component} from '@angular/core';
@Component({
standalone: true,
selector: 'one-cmp',
template: '<div></div>',
})
export class OneCmp {}
@Component({
standalone: true,
selector: 'two-cmp',
template: '<div></div>',
})
export class TwoCmp {}
`);
const {program, checker} = env.driveTemplateTypeChecker();
const sfOne = program.getSourceFile(_('/decls.ts'));
expect(sfOne).not.toBeNull();
const OneCmpClass = getClass(sfOne!, 'OneCmp');
const TwoCmpDir = checker.getPotentialTemplateDirectives(OneCmpClass)
.filter(d => d.selector === 'two-cmp')[0];
const imports =
checker.getPotentialImportsFor(TwoCmpDir.ref, OneCmpClass, PotentialImportMode.Normal);
expect(imports.length).toBe(1);
expect(imports[0].moduleSpecifier).toBeUndefined();
expect(imports[0].symbolName).toBe('TwoCmp');
expect(imports[0].isForwardReference).toBe(true);
});
});
});

30
packages/language-service/src/codefixes/fix_missing_import.ts
View file

@ -74,11 +74,33 @@ function getCodeActions(
const currMatchSymbol = currMatch.tsSymbol.valueDeclaration!;
const potentialImports =
checker.getPotentialImportsFor(currMatch.ref, importOn, PotentialImportMode.Normal);
for (let potentialImport of potentialImports) {
let [fileImportChanges, importName] = updateImportsForTypescriptFile(
tsChecker, importOn.getSourceFile(), potentialImport, currMatchSymbol.getSourceFile());
for (const potentialImport of potentialImports) {
const fileImportChanges: ts.TextChange[] = [];
let importName: string;
let forwardRefName: string|null = null;
if (potentialImport.moduleSpecifier) {
const [importChanges, generatedImportName] = updateImportsForTypescriptFile(
tsChecker, importOn.getSourceFile(), potentialImport.symbolName,
potentialImport.moduleSpecifier, currMatchSymbol.getSourceFile());
importName = generatedImportName;
fileImportChanges.push(...importChanges);
} else {
if (potentialImport.isForwardReference) {
// Note that we pass the `importOn` file twice since we know that the potential import
// is within the same file, because it doesn't have a `moduleSpecifier`.
const [forwardRefImports, generatedForwardRefName] = updateImportsForTypescriptFile(
tsChecker, importOn.getSourceFile(), 'forwardRef', '@angular/core',
importOn.getSourceFile());
fileImportChanges.push(...forwardRefImports);
forwardRefName = generatedForwardRefName;
}
importName = potentialImport.symbolName;
}
// Always update the trait import, although the TS import might already be present.
let traitImportChanges = updateImportsForAngularTrait(checker, importOn, importName);
const traitImportChanges =
updateImportsForAngularTrait(checker, importOn, importName, forwardRefName);
if (traitImportChanges.length === 0) continue;
let description = `Import ${importName}`;

37
packages/language-service/src/ts_utils.ts
View file

@ -178,14 +178,15 @@ export function updateObjectValueForKey(
* If no update is needed, returns `null`.
*/
export function ensureArrayWithIdentifier(
identifier: ts.Identifier, arr?: ts.ArrayLiteralExpression): ts.ArrayLiteralExpression|null {
identifierText: string, expression: ts.Expression,
arr?: ts.ArrayLiteralExpression): ts.ArrayLiteralExpression|null {
if (arr === undefined) {
return ts.factory.createArrayLiteralExpression([identifier]);
return ts.factory.createArrayLiteralExpression([expression]);
}
if (arr.elements.find(v => ts.isIdentifier(v) && v.text === identifier.text)) {
if (arr.elements.find(v => ts.isIdentifier(v) && v.text === identifierText)) {
return null;
}
return ts.factory.updateArrayLiteralExpression(arr, [...arr.elements, identifier]);
return ts.factory.updateArrayLiteralExpression(arr, [...arr.elements, expression]);
}
export function moduleSpecifierPointsToFile(
@ -316,18 +317,13 @@ export function standaloneTraitOrNgModule(
* Returns the text changes, as well as the name with which the imported symbol can be referred to.
*/
export function updateImportsForTypescriptFile(
tsChecker: ts.TypeChecker, file: ts.SourceFile, newImport: PotentialImport,
tsChecker: ts.TypeChecker, file: ts.SourceFile, symbolName: string, moduleSpecifier: string,
tsFileToImport: ts.SourceFile): [ts.TextChange[], string] {
// If the expression is already imported, we can just return its name.
if (newImport.moduleSpecifier === undefined) {
return [[], newImport.symbolName];
}
// The trait might already be imported, possibly under a different name. If so, determine the
// local name of the imported trait.
const allImports = findAllMatchingNodes(file, {filter: ts.isImportDeclaration});
const existingImportName: string|null =
hasImport(tsChecker, allImports, newImport.symbolName, tsFileToImport);
hasImport(tsChecker, allImports, symbolName, tsFileToImport);
if (existingImportName !== null) {
return [[], existingImportName];
}
@ -335,7 +331,7 @@ export function updateImportsForTypescriptFile(
// If the trait has not already been imported, we need to insert the new import.
const existingImportDeclaration = allImports.find(
decl => moduleSpecifierPointsToFile(tsChecker, decl.moduleSpecifier, tsFileToImport));
const importName = nonCollidingImportName(allImports, newImport.symbolName);
const importName = nonCollidingImportName(allImports, symbolName);
if (existingImportDeclaration !== undefined) {
// Update an existing import declaration.
@ -347,7 +343,7 @@ export function updateImportsForTypescriptFile(
return [[], ''];
}
let span = {start: bindings.getStart(), length: bindings.getWidth()};
const updatedBindings = updateImport(bindings, newImport.symbolName, importName);
const updatedBindings = updateImport(bindings, symbolName, importName);
const importString = printNode(updatedBindings, file);
return [[{span, newText: importString}], importName];
}
@ -364,8 +360,7 @@ export function updateImportsForTypescriptFile(
if (lastImport as any !== null) { // TODO: Why does the compiler insist this is null?
span.start = lastImport!.getStart() + lastImport!.getWidth();
}
const newImportDeclaration =
generateImport(newImport.symbolName, importName, newImport.moduleSpecifier);
const newImportDeclaration = generateImport(symbolName, importName, moduleSpecifier);
const importString = '\n' + printNode(newImportDeclaration, file);
return [[{span, newText: importString}], importName];
}
@ -375,7 +370,8 @@ export function updateImportsForTypescriptFile(
* `importName` to the list of imports on the decorator arguments.
*/
export function updateImportsForAngularTrait(
checker: TemplateTypeChecker, trait: ts.ClassDeclaration, importName: string): ts.TextChange[] {
checker: TemplateTypeChecker, trait: ts.ClassDeclaration, importName: string,
forwardRefName: string|null): ts.TextChange[] {
// Get the object with arguments passed into the primary Angular decorator for this trait.
const decorator = checker.getPrimaryAngularDecorator(trait);
if (decorator === null) {
@ -393,7 +389,14 @@ export function updateImportsForAngularTrait(
if (oldValue && !ts.isArrayLiteralExpression(oldValue)) {
return oldValue;
}
const newArr = ensureArrayWithIdentifier(ts.factory.createIdentifier(importName), oldValue);
const identifier = ts.factory.createIdentifier(importName);
const expression = forwardRefName ?
ts.factory.createCallExpression(
ts.factory.createIdentifier(forwardRefName), undefined,
[ts.factory.createArrowFunction(
undefined, undefined, [], undefined, undefined, identifier)]) :
identifier;
const newArr = ensureArrayWithIdentifier(importName, expression, oldValue);
updateRequired = newArr !== null;
return newArr!;
});

8
packages/language-service/test/ts_utils_spec.ts
View file

@ -163,17 +163,17 @@ describe('TS util', () => {
});
it('addElementToArrayLiteral', () => {
let arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('foo'));
let arr = ensureArrayWithIdentifier('foo', ts.factory.createIdentifier('foo'));
arr = addElementToArrayLiteral(arr!, ts.factory.createIdentifier('bar'));
expect(print(arr)).toEqual('[foo, bar]');
});
it('ensureArrayWithIdentifier', () => {
let arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('foo'));
let arr = ensureArrayWithIdentifier('foo', ts.factory.createIdentifier('foo'));
expect(print(arr!)).toEqual('[foo]');
arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('bar'), arr!);
arr = ensureArrayWithIdentifier('bar', ts.factory.createIdentifier('bar'), arr!);
expect(print(arr!)).toEqual('[foo, bar]');
arr = ensureArrayWithIdentifier(ts.factory.createIdentifier('bar'), arr!);
arr = ensureArrayWithIdentifier('bar', ts.factory.createIdentifier('bar'), arr!);
expect(arr).toEqual(null);
});