fix(compiler-cli): missingStructuralDirective diagnostic produces false negatives (angular#64579)

Fixes a bug in the missingStructuralDirective diagnostic where structural directives with missing imports were not reported when the element using the structural directive contained other directives

Fixes angular#64467

co-authored-by: Matt Lewis <[email protected]>

PR Close angular#64579

Author: JeanMeche

packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/index.ts

@@ -25,6 +25,8 @@ export const KNOWN_CONTROL_FLOW_DIRECTIVES = new Set([
   'ngForTrackBy',
   'ngSwitchCase',
   'ngSwitchDefault',
+  'ngIfThen',
+  'ngIfElse',
 ]);
 
 /**
@@ -65,9 +67,13 @@ class MissingStructuralDirectiveCheck extends TemplateCheckWithVisitor<ErrorCode
     if (!customStructuralDirective) return [];
 
     const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component);
-    if (symbol?.directives.length) {
-      return [];
-    }
+    // Check if there's a directive that matches the structural directive we're checking.
+    // The structural directive *foo desugars to [foo], so we need to check if any
+    // directive's selector would match the attribute [foo].
+    const hasStructuralDirective = symbol?.directives.some((dir) =>
+      dir.selector?.includes(`[${customStructuralDirective.name}]`),
+    );
+    if (hasStructuralDirective) return [];
 
     const sourceSpan = customStructuralDirective.keySpan || customStructuralDirective.sourceSpan;
     const errorMessage =

packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/missing_structural_directive_spec.ts

@@ -337,5 +337,125 @@ runInEachFileSystem(() => {
 
       // What matters is that we don't get the missing structural directive diagnostic.
     });
+
+    it('should produce a warning when missing structural directive is used alongside an imported directive', () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup([
+        {
+          fileName,
+          templates: {
+            'TestCmp': `
+              <div *foo="bar" [bar]></div>
+            `,
+          },
+          source: `
+            export class TestCmp {}
+            export class BarDirective {}
+          `,
+          declarations: [
+            {
+              type: 'directive',
+              name: 'BarDirective',
+              selector: '[bar]',
+              inputs: {bar: 'bar'},
+            },
+            {
+              name: 'TestCmp',
+              type: 'directive',
+              selector: `test-cmp`,
+              isStandalone: true,
+            },
+          ],
+        },
+      ]);
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+        templateTypeChecker,
+        program.getTypeChecker(),
+        [missingStructuralDirectiveCheck],
+        {strictNullChecks: true} /* options */,
+      );
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+
+      expect(diags.length).toBe(1);
+      expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
+      expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_STRUCTURAL_DIRECTIVE));
+    });
+
+    it('should *not* produce a warning when structural directive with more specific selector is imported', () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup([
+        {
+          fileName,
+          templates: {
+            'TestCmp': `
+              <div *bar></div>
+            `,
+          },
+          source: `
+            export class TestCmp {}
+            export class BarDirective {}
+          `,
+          declarations: [
+            {
+              type: 'directive',
+              name: 'BarDirective',
+              selector: 'ng-template[bar]',
+            },
+            {
+              name: 'TestCmp',
+              type: 'directive',
+              selector: `test-cmp`,
+              isStandalone: true,
+            },
+          ],
+        },
+      ]);
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+        templateTypeChecker,
+        program.getTypeChecker(),
+        [missingStructuralDirectiveCheck],
+        {strictNullChecks: true} /* options */,
+      );
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+
+      expect(diags.length).toBe(0);
+    });
+
+    it('should *not* produce a warning for ngIf with template references using then/else', () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup([
+        {
+          fileName,
+          templates: {
+            'TestCmp': `
+              <ng-container *ngIf="foo; then bar; else bam"></ng-container>
+            `,
+          },
+          declarations: [
+            {
+              name: 'TestCmp',
+              type: 'directive',
+              selector: `test-cmp`,
+              isStandalone: true,
+            },
+          ],
+        },
+      ]);
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+        templateTypeChecker,
+        program.getTypeChecker(),
+        [missingStructuralDirectiveCheck],
+        {strictNullChecks: true} /* options */,
+      );
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+
+      expect(diags.length).toBe(0);
+    });
   });
 });