refactor(compiler-cli): validate @content block names for conflicts

leonsenft · leonsenft · commit 0a088711059e · 2026-06-04T17:25:26.000-07:00
Ensures `@content` blocks on foreign components have unique names and do not
conflict with static attributes or input property bindings.

Specifically, this commit introduces two new template diagnostics:
1. `CONFLICTING_CONTENT_DECLARATION` (8028): Raised when multiple `@content`
   blocks with the same name are defined under the same foreign component.
2. `CONFLICTING_CONTENT_AND_PROPERTY` (8029): Raised when a `@content` block's
   name matches an attribute or input property binding on the parent foreign
   component.

Both diagnostics include related information pointing to the location of the
conflicting declaration or property.
diff --git a/goldens/public-api/compiler-cli/error_code.api.md b/goldens/public-api/compiler-cli/error_code.api.md
@@ -28,6 +28,8 @@ export enum ErrorCode {
     CONFIG_FLAT_MODULE_NO_INDEX = 4001,
     // (undocumented)
     CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
+    CONFLICTING_CONTENT_AND_PROPERTY = 8029,
+    CONFLICTING_CONTENT_DECLARATION = 8028,
     CONFLICTING_HOST_DIRECTIVE_BINDING = -8024,
     CONFLICTING_INPUT_TRANSFORM = 2020,
     CONFLICTING_LET_DECLARATION = 8017,
diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts
@@ -36,6 +36,11 @@ import {ForeignComponentMeta} from '../../../metadata';
 import {makeTemplateDiagnostic} from '../../../typecheck/diagnostics';
 import {SourceMapping} from '../../../typecheck/api';
 
+/**
+ * The intrinsic property name used to project children into a foreign component.
+ */
+const CHILDREN = 'children';
+
 /**
  * Analyzes the template for invalid use of features relating to foreign components.
  *
@@ -55,6 +60,10 @@ export function analyzeForeignComponentFeatures(
 class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
   private currentParent: TmplAstNode | null = null;
   readonly diagnostics: ts.Diagnostic[] = [];
+  // Tracks the named @content blocks defined for each foreign component element.
+  // This is used to detect duplicate @content declarations under the same parent
+  // during the recursive AST traversal.
+  private readonly seenContentBlocks = new Map<TmplAstElement, Map<string, TmplAstContentBlock>>();
 
   constructor(
     private readonly foreignMatcher: SelectorlessMatcher<ForeignComponentMeta> | null,
@@ -77,42 +86,7 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
 
   override visitElement(element: TmplAstElement): void {
     if (this.elementIsForeignComponent(element.name)) {
-      if (element.outputs.length > 0) {
-        this.diagnostics.push(
-          makeTemplateDiagnostic(
-            '' as TypeCheckId,
-            this.sourceMapping,
-            element.sourceSpan,
-            ts.DiagnosticCategory.Error,
-            ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
-            'Foreign components do not support event bindings.',
-          ),
-        );
-      }
-      if (element.references.length > 0) {
-        this.diagnostics.push(
-          makeTemplateDiagnostic(
-            '' as TypeCheckId,
-            this.sourceMapping,
-            element.sourceSpan,
-            ts.DiagnosticCategory.Error,
-            ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
-            'Foreign components do not support references.',
-          ),
-        );
-      }
-      if (element.inputs.some((input) => input.type !== BindingType.Property)) {
-        this.diagnostics.push(
-          makeTemplateDiagnostic(
-            '' as TypeCheckId,
-            this.sourceMapping,
-            element.sourceSpan,
-            ts.DiagnosticCategory.Error,
-            ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
-            'Foreign components only support static attributes and property bindings.',
-          ),
-        );
-      }
+      this.validateForeignComponent(element);
     }
 
     const prevParent = this.currentParent;
@@ -121,6 +95,83 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
     this.currentParent = prevParent;
   }
 
+  private validateForeignComponent(element: TmplAstElement): void {
+    if (element.outputs.length > 0) {
+      this.diagnostics.push(
+        makeTemplateDiagnostic(
+          '' as TypeCheckId,
+          this.sourceMapping,
+          element.sourceSpan,
+          ts.DiagnosticCategory.Error,
+          ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
+          'Foreign components do not support event bindings.',
+        ),
+      );
+    }
+    if (element.references.length > 0) {
+      this.diagnostics.push(
+        makeTemplateDiagnostic(
+          '' as TypeCheckId,
+          this.sourceMapping,
+          element.sourceSpan,
+          ts.DiagnosticCategory.Error,
+          ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
+          'Foreign components do not support references.',
+        ),
+      );
+    }
+    if (element.inputs.some((input) => input.type !== BindingType.Property)) {
+      this.diagnostics.push(
+        makeTemplateDiagnostic(
+          '' as TypeCheckId,
+          this.sourceMapping,
+          element.sourceSpan,
+          ts.DiagnosticCategory.Error,
+          ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
+          'Foreign components only support static attributes and property bindings.',
+        ),
+      );
+    }
+
+    // A foreign component maps implicit child nodes to a 'children' property.
+    // If the user also explicitly binds to '[children]' or sets a static 'children' attribute, this is a conflict.
+    const childrenInput = element.inputs.find(
+      (input) => input.type === BindingType.Property && input.name === CHILDREN,
+    );
+    const childrenAttr = element.attributes.find((attr) => attr.name === CHILDREN);
+    const conflictingSource = childrenInput ?? childrenAttr;
+    if (conflictingSource === undefined) {
+      return;
+    }
+
+    // Explicit `@content` blocks (TmplAstContentBlock) are mapped to properties by their name, so
+    // they do not conflict with the default 'children' property. We only care about child nodes
+    // that are not content blocks, as those are implicitly passed to the 'children' property.
+    const firstChild = element.children.find((child) => !(child instanceof TmplAstContentBlock));
+    if (firstChild === undefined) {
+      return;
+    }
+
+    this.diagnostics.push(
+      makeTemplateDiagnostic(
+        '' as TypeCheckId,
+        this.sourceMapping,
+        conflictingSource.sourceSpan,
+        ts.DiagnosticCategory.Error,
+        ngErrorCode(ErrorCode.CONFLICTING_CONTENT_AND_PROPERTY),
+        `A foreign component cannot have both a '${CHILDREN}' property and child nodes.`,
+        [
+          {
+            text: 'Child nodes are defined here.',
+            start: firstChild.sourceSpan.start.offset,
+            end: firstChild.sourceSpan.end.offset,
+            sourceFile: this.sourceMapping.node.getSourceFile(),
+          },
+        ],
+      ),
+    );
+  }
+
   override visitTemplate(template: TmplAstTemplate): void {
     const prevParent = this.currentParent;
     this.currentParent = template;
@@ -207,19 +258,7 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
 
   override visitContentBlock(block: TmplAstContentBlock): void {
     if (this.parentNodeIsForeignComponent()) {
-      if (block.name === 'children') {
-        this.diagnostics.push(
-          makeTemplateDiagnostic(
-            '' as TypeCheckId,
-            this.sourceMapping,
-            block.sourceSpan,
-            ts.DiagnosticCategory.Error,
-            ngErrorCode(ErrorCode.FOREIGN_COMPONENT_CONTENT_UNNECESSARY_FOR_CHILDREN),
-            'Defining a @content (children) block is unnecessary. ' +
-              'Pass children as direct nested content of the foreign component instead.',
-          ),
-        );
-      }
+      this.validateContentBlock(block);
     } else {
       this.diagnostics.push(
         makeTemplateDiagnostic(
@@ -232,9 +271,93 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
         ),
       );
     }
+
     const prevParent = this.currentParent;
     this.currentParent = block;
     super.visitContentBlock(block);
     this.currentParent = prevParent;
   }
+
+  private validateContentBlock(block: TmplAstContentBlock): void {
+    const parent = this.currentParent as TmplAstElement;
+
+    // Retrieve or initialize the map of @content blocks seen so far for this parent.
+    // Since the visitor is recursive, we must track declarations per-parent to
+    // only report duplicates within the scope of the same foreign component.
+    let seen = this.seenContentBlocks.get(parent);
+    if (seen === undefined) {
+      seen = new Map<string, TmplAstContentBlock>();
+      this.seenContentBlocks.set(parent, seen);
+    }
+
+    if (seen.has(block.name)) {
+      const firstDecl = seen.get(block.name)!;
+      this.diagnostics.push(
+        makeTemplateDiagnostic(
+          '' as TypeCheckId,
+          this.sourceMapping,
+          block.sourceSpan,
+          ts.DiagnosticCategory.Error,
+          ngErrorCode(ErrorCode.CONFLICTING_CONTENT_DECLARATION),
+          `A @content block with the name '${block.name}' has already been defined for this component.`,
+          [
+            {
+              text: `The @content block '${block.name}' was first defined here.`,
+              start: firstDecl.sourceSpan.start.offset,
+              end: firstDecl.sourceSpan.end.offset,
+              sourceFile: this.sourceMapping.node.getSourceFile(),
+            },
+          ],
+        ),
+      );
+    } else {
+      seen.set(block.name, block);
+    }
+
+    // A @content block projects content into a property of the foreign component.
+    // If the parent element also binds to this property (either via a property binding
+    // or a static attribute), it creates a conflict as both try to write to the same prop.
+    const conflictInput = parent.inputs.find(
+      (input) => input.type === BindingType.Property && input.name === block.name,
+    );
+    const conflictAttr = parent.attributes.find((attr) => attr.name === block.name);
+    const conflict = conflictInput ?? conflictAttr;
+
+    if (conflict !== undefined) {
+      this.diagnostics.push(
+        makeTemplateDiagnostic(
+          '' as TypeCheckId,
+          this.sourceMapping,
+          block.sourceSpan,
+          ts.DiagnosticCategory.Error,
+          ngErrorCode(ErrorCode.CONFLICTING_CONTENT_AND_PROPERTY),
+          `A @content block with the name '${block.name}' conflicts with a property on the parent component.`,
+          [
+            {
+              text: `The property '${block.name}' is defined here.`,
+              start: conflict.sourceSpan.start.offset,
+              end: conflict.sourceSpan.end.offset,
+              sourceFile: this.sourceMapping.node.getSourceFile(),
+            },
+          ],
+        ),
+      );
+    }
+
+    // Explicitly defining a `@content(children)` block is unnecessary because child nodes are
+    // implicitly passed to the `children` property.
+    if (block.name === CHILDREN) {
+      this.diagnostics.push(
+        makeTemplateDiagnostic(
+          '' as TypeCheckId,
+          this.sourceMapping,
+          block.sourceSpan,
+          ts.DiagnosticCategory.Error,
+          ngErrorCode(ErrorCode.FOREIGN_COMPONENT_CONTENT_UNNECESSARY_FOR_CHILDREN),
+          `Defining a @content (${CHILDREN}) block is unnecessary. ` +
+            'Pass children as direct nested content of the foreign component instead.',
+        ),
+      );
+    }
+  }
 }
diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
@@ -475,6 +475,16 @@ export enum ErrorCode {
    */
   FOREIGN_COMPONENT_CONTENT_UNNECESSARY_FOR_CHILDREN = 8027,
 
+  /**
+   * Raised when multiple `@content` blocks with the same name are defined for a foreign component.
+   */
+  CONFLICTING_CONTENT_DECLARATION = 8028,
+
+  /**
+   * Raised when a `@content` block name conflicts with an input binding on the parent foreign component.
+   */
+  CONFLICTING_CONTENT_AND_PROPERTY = 8029,
+
   /**
    * A two way binding in a template has an incorrect syntax,
    * parentheses outside brackets. For example:
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
