8000 fixup! feat(compiler-cli): detect missing structural directive imports · angular/angular@c76c88b · GitHub
[go: up one dir, main page]
Skip to content

Commit c76c88b

Browse files
JeanMecheJeanMeche
committed
fixup! feat(compiler-cli): detect missing structural directive imports
1 parent 8e5faa2 commit c76c88b

File tree

3 files changed

+91
-51
lines changed

3 files changed

+91
-51
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -168,25 +168,21 @@ export function findTypeCheckBlock(
168168
id: TypeCheckId,
169169
isDiagnosticRequest: boolean,
170170
): ts.Node | null {
171+
// This prioritised-level statements using a breadth-first search
172+
// This is usually sufficient to find the TCB we're looking for
171173
for (const stmt of file.statements) {
172174
if (ts.isFunctionDeclaration(stmt) && getTypeCheckId(stmt, file, isDiagnosticRequest) === id) {
173175
return stmt;
174176
}
175177
}
176178

177-
// In case the TCB we're looking for is nested
178-
// This happens for example when a directive is declared inside a function
179-
180-
// Pre-check if we can expect to find the id in the file
181-
if (file.text.includes(id)) {
182-
return findNodeInFile(
183-
file,
184-
(node) =>
185-
ts.isFunctionDeclaration(node) && getTypeCheckId(node, file, isDiagnosticRequest) === id,
186-
);
187-
}
188-
189-
return null;
179+
// In case the TCB we're looking for is nested (which is not common)
180+
// eg: when a directive is declared inside a function, as it can happen in test files
181+
return findNodeInFile(
182+
file,
183+
(node) =>
184+
ts.isFunctionDeclaration(node) && getTypeCheckId(node, file, isDiagnosticRequest) === id,
185+
);
190186
}
191187

192188
/**

‎packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts

Lines changed: 66 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -289,18 +289,20 @@ export class SymbolBuilder {
289289
NgOriginalFile
290290
];
291291

292-
// This is a preliminary check ahead of a more expensive search
293-
const hasPotentialCandidate = directives.find(
294-
(m) => m.ref.node.name.text === directiveDeclaration.name?.text,
295-
);
296-
297-
if (originalFile && hasPotentialCandidate) {
298-
// In case the TCB has been inlined because of a non exported declaration
299-
// We will instead find a matching class
300-
// And if found one, look for it in the directives array
301-
const classWithSameName = findMatchingDirective(originalFile, directiveDeclaration);
302-
if (classWithSameName) {
303-
return directives.find((m) => m.ref.node === classWithSameName) ?? null;
292+
if (originalFile !== undefined) {
293+
// This is a preliminary check ahead of a more expensive search
294+
const hasPotentialCandidate = directives.find(
295+
(m) => m.ref.node.name.text === directiveDeclaration.name?.text,
296+
);
297+
298+
if (hasPotentialCandidate) {
299+
// In case the TCB has been inlined,
300+
// We will look for a matching class
301+
// If we find one, we look for it in the directives array
302+
const classWithSameName = findMatchingDirective(originalFile, directiveDeclaration);
303+
if (classWithSameName !== null) {
304+
return directives.find((m) => m.ref.node === classWithSameName) ?? null;
305+
}
304306
}
305307
}
306308

@@ -889,28 +891,47 @@ function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): null
889891
};
890892
}
891893

894+
/**
895+
* Looks for a class declaration in the original source file that matches a given directive
896+
* from the type check source file.
897+
*
898+
* @param originalSourceFile The original source where the runtime code resides
899+
* @param directiveDeclarationInTypeCheckSourceFile The directive from the type check source file
900+
*/
892901
function findMatchingDirective(
893-
sourceFile: ts.SourceFile,
894-
directiveDeclaration: ts.ClassDeclaration,
895-
) {
896-
// Because of https://github.com/microsoft/typescript/issues/9998 we need to trick the compiler
897-
let classWithSameName: ts.ClassDeclaration = null!;
902+
originalSourceFile: ts.SourceFile,
903+
directiveDeclarationInTypeCheckSourceFile: ts.ClassDeclaration,
904+
): ts.ClassDeclaration | null {
905+
let sameDeclarationInOriginalFile: ts.ClassDeclaration | null = null;
906+
907+
const typecheckDirectiveDeclarationDepth = getDeclarationDepth(
908+
directiveDeclarationInTypeCheckSourceFile,
909+
);
910+
911+
const className = directiveDeclarationInTypeCheckSourceFile.name?.text ?? '';
912+
// We build an index of the class declarations with the same name
913+
// To then compare the indexes to confirm we found the right class declaration
914+
const ogClasses = collectClassesWithName(originalSourceFile, className);
915+
const typcheckClasses = collectClassesWithName(
916+
directiveDeclarationInTypeCheckSourceFile.getSourceFile(),
917+
className,
918+
);
898919

899920
function visit(node: ts.Node): void {
900921
if (
901922
ts.isClassDeclaration(node) &&
902-
node.name?.text === directiveDeclaration.name?.text &&
903-
getDeclarationDepth(node) === getDeclarationDepth(directiveDeclaration) &&
904-
node.getText() === directiveDeclaration.getText() // compares text content
923+
node.name?.text === directiveDeclarationInTypeCheckSourceFile.name?.text &&
924+
getDeclarationDepth(node) === typecheckDirectiveDeclarationDepth &&
925+
ogClasses.indexOf(node) === typcheckClasses.indexOf(directiveDeclarationInTypeCheckSourceFile)
905926
) {
906-
classWithSameName = node;
927+
sameDeclarationInOriginalFile = node;
907928
return;
908929
}
909930
ts.forEachChild(node, visit);
910931
}
911-
ts.forEachChild(sourceFile, visit);
932+
ts.forEachChild(originalSourceFile, visit);
912933

913-
return classWithSameName;
934+
return sameDeclarationInOriginalFile;
914935
}
915936

916937
function getDeclarationDepth(decl: ts.Declaration): number {
@@ -924,3 +945,25 @@ function getDeclarationDepth(decl: ts.Declaration): number {
924945

925946
return depth;
926947
}
948+
949+
/**
950+
* Builds a list of class declarations of a given name
951+
* Is used as a index based reference to compare class declarations
952+
* between the typecheck source file and the original source file
953+
*/
954+
function collectClassesWithName(
955+
sourceFile: ts.SourceFile,
956+
className: string,
957+
): ts.ClassDeclaration[] {
958+
const classes: ts.ClassDeclaration[] = [];
959+
function visit(node: ts.Node) {
960+
if (ts.isClassDeclaration(node) && node.name?.text === className) {
961+
classes.push(node);
962+
} else {
963+
ts.forEachChild(node, visit);
964+
}
965+
}
966+
sourceFile.forEachChild(visit);
967+
968+
return classes;
969+
}

‎packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,6 +2126,7 @@ runInEachFileSystem(() => {
21262126
'BarCmp': '',
21272127
},
21282128
source: `
2129+
/* Declare a non-exported component to force using an inline TCB */
21292130
class BarCmp{}
21302131
21312132
export class TestCmp {}
@@ -2232,28 +2233,28 @@ runInEachFileSystem(() => {
22322233
expect(symbol.tcbLocation.isShimFile).toBe(true);
22332234
});
22342235

2235-
it('should trigger diagnostic for nested component in function', () => {
2236+
it('find the directive when the class is nested in a function', () => {
22362237
// This test is more complex as we're testing the diagnostic against a component
22372238
// that can't be referenced because it's nested in a function.
22382239

22392240
const {compiler, sourceFile} = createNgCompilerForFile(`
2240-
import {Component, Directive} from '@angular/core';
2241-
2242-
@Directive({ selector: '[foo]' })
2243-
export class FooDir {}
2244-
2245-
export function foo() {
2246-
@Component({
2247-
imports: [FooDir],
2248-
template: '<div *foo></div>',
2249-
})
2250-
class MyCmp {}
2251-
}
2252-
`);
2241+
import {Component, Directive} from '@angular/core';
2242+
2243+
@Directive({ selector: '[foo]' })
2244+
export class FooDir {}
2245+
2246+
export function foo() {
2247+
@Component({
2248+
imports: [FooDir],
2249+
template: '<div *foo></div>',
2250+
})
2251+
class MyCmp {}
2252+
}
2253+
`);
22532254

22542255
const templateTypeChecker = compiler.getTemplateTypeChecker();
22552256

2256-
let myCmpClass = findNodeInFile(
2257+
const myCmpClass = findNodeInFile(
22572258
sourceFile,
22582259
(node): node is ts.ClassDeclaration =>
22592260
ts.isClassDeclaration(node) && node.name?.text === 'MyCmp',

0 commit comments

Comments
 (0)
0