refactor(core): add host directive definitions validation (angular#47589)

Adds some logic to ensure that host directives are configured correctly.

PR Close angular#47589

Author: crisbeto

goldens/public-api/core/errors.md

@@ -25,12 +25,24 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     CYCLIC_DI_DEPENDENCY = -200,
     // (undocumented)
+    DUPLICATE_DIRECTITVE = 309,
+    // (undocumented)
     ERROR_HANDLER_NOT_FOUND = 402,
     // (undocumented)
     EXPORT_NOT_FOUND = -301,
     // (undocumented)
     EXPRESSION_CHANGED_AFTER_CHECKED = -100,
     // (undocumented)
+    HOST_DIRECTIVE_COMPONENT = 310,
+    // (undocumented)
+    HOST_DIRECTIVE_CONFLICTING_ALIAS = 312,
+    // (undocumented)
+    HOST_DIRECTIVE_NOT_STANDALONE = 308,
+    // (undocumented)
+    HOST_DIRECTIVE_UNDEFINED_BINDING = 311,
+    // (undocumented)
+    HOST_DIRECTIVE_UNRESOLVABLE = 307,
+    // (undocumented)
     IMPORT_PROVIDERS_FROM_STANDALONE = 800,
     // (undocumented)
     INJECTOR_ALREADY_DESTROYED = 205,

packages/core/src/errors.ts

@@ -42,6 +42,12 @@ export const enum RuntimeErrorCode {
   UNKNOWN_ELEMENT = 304,
   TEMPLATE_STRUCTURE_ERROR = 305,
   INVALID_EVENT_BINDING = 306,
+  HOST_DIRECTIVE_UNRESOLVABLE = 307,
+  HOST_DIRECTIVE_NOT_STANDALONE = 308,
+  DUPLICATE_DIRECTITVE = 309,
+  HOST_DIRECTIVE_COMPONENT = 310,
+  HOST_DIRECTIVE_UNDEFINED_BINDING = 311,
+  HOST_DIRECTIVE_CONFLICTING_ALIAS = 312,
 
   // Bootstrap Errors
   MULTIPLE_PLATFORMS = 400,

packages/core/src/render3/features/host_directives_feature.ts

@@ -6,10 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {resolveForwardRef} from '../../di';
+import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {Type} from '../../interface/type';
 import {EMPTY_OBJ} from '../../util/empty';
-import {getDirectiveDef} from '../definition';
-import {DirectiveDef, HostDirectiveBindingMap, HostDirectiveDefs} from '../interfaces/definition';
+import {getComponentDef, getDirectiveDef} from '../definition';
+import {DirectiveDef, HostDirectiveBindingMap, HostDirectiveDef, HostDirectiveDefs} from '../interfaces/definition';
 
 /** Values that can be used to define a host directive through the `HostDirectivesFeature`. */
 type HostDirectiveConfig = Type<unknown>|{
@@ -62,7 +63,9 @@ function findHostDirectiveDefs(
     for (const hostDirectiveConfig of currentDef.hostDirectives) {
       const hostDirectiveDef = getDirectiveDef(hostDirectiveConfig.directive)!;
 
-      // TODO(crisbeto): assert that the def exists.
+      if (typeof ngDevMode === 'undefined' || ngDevMode) {
+        validateHostDirective(hostDirectiveConfig, hostDirectiveDef, matchedDefs);
+      }
 
       // Host directives execute before the host so that its host bindings can be overwritten.
       findHostDirectiveDefs(hostDirectiveDef, matchedDefs, hostDirectiveDefs);
@@ -89,3 +92,80 @@ function bindingArrayToMap(bindings: string[]|undefined): HostDirectiveBindingMa
 
   return result;
 }
+
+/**
+ * Verifies that the host directive has been configured correctly.
+ * @param hostDirectiveConfig Host directive configuration object.
+ * @param directiveDef Directive definition of the host directive.
+ * @param matchedDefs Directives that have been matched so far.
+ */
+function validateHostDirective(
+    hostDirectiveConfig: HostDirectiveDef<unknown>, directiveDef: DirectiveDef<any>|null,
+    matchedDefs: DirectiveDef<unknown>[]): asserts directiveDef is DirectiveDef<unknown> {
+  // TODO(crisbeto): implement more of these checks in the compiler.
+  const type = hostDirectiveConfig.directive;
+
+  if (directiveDef === null) {
+    if (getComponentDef(type) !== null) {
+      throw new RuntimeError(
+          RuntimeErrorCode.HOST_DIRECTIVE_COMPONENT,
+          `Host directive ${type.name} cannot be a component.`);
+    }
+
+    throw new RuntimeError(
+        RuntimeErrorCode.HOST_DIRECTIVE_UNRESOLVABLE,
+        `Could not resolve metadata for host directive ${type.name}. ` +
+            `Make sure that the ${type.name} class is annotated with an @Directive decorator.`);
+  }
+
+  if (!directiveDef.standalone) {
+    throw new RuntimeError(
+        RuntimeErrorCode.HOST_DIRECTIVE_NOT_STANDALONE,
+        `Host directive ${directiveDef.type.name} must be standalone.`);
+  }
+
+  if (matchedDefs.indexOf(directiveDef) > -1) {
+    throw new RuntimeError(
+        RuntimeErrorCode.DUPLICATE_DIRECTITVE,
+        `Directive ${directiveDef.type.name} matches multiple times on the same element. ` +
+            `Directives can only match an element once.`);
+  }
+
+  validateMappings('input', directiveDef, hostDirectiveConfig.inputs);
+  validateMappings('output', directiveDef, hostDirectiveConfig.outputs);
+}
+
+/**
+ * Checks that the host directive inputs/outputs configuration is valid.
+ * @param bindingType Kind of binding that is being validated. Used in the error message.
+ * @param def Definition of the host directive that is being validated against.
+ * @param hostDirectiveDefs Host directive mapping object that shold be validated.
+ */
+function validateMappings(
+    bindingType: 'input'|'output', def: DirectiveDef<unknown>,
+    hostDirectiveDefs: HostDirectiveBindingMap) {
+  const className = def.type.name;
+  const bindings: Record<string, string> = bindingType === 'input' ? def.inputs : def.outputs;
+
+  for (const publicName in hostDirectiveDefs) {
+    if (hostDirectiveDefs.hasOwnProperty(publicName)) {
+      if (!bindings.hasOwnProperty(publicName)) {
+        throw new RuntimeError(
+            RuntimeErrorCode.HOST_DIRECTIVE_UNDEFINED_BINDING,
+            `Directive ${className} does not have an ${bindingType} with a public name of ${
+                publicName}.`);
+      }
+
+      const remappedPublicName = hostDirectiveDefs[publicName];
+
+      if (bindings.hasOwnProperty(remappedPublicName) &&
+          bindings[remappedPublicName] !== publicName) {
+        throw new RuntimeError(
+            RuntimeErrorCode.HOST_DIRECTIVE_CONFLICTING_ALIAS,
+            `Cannot alias ${bindingType} ${publicName} of host directive ${className} to ${
+                remappedPublicName}, because it already has a different ${
+                bindingType} with the same public name.`);
+      }
+    }
+  }
+}

packages/core/src/render3/instructions/shared.ts

@@ -25,7 +25,7 @@ import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} f
 import {throwMultipleComponentError} from '../errors';
 import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
 import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container';
-import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, HostBindingsFunction, HostDirectiveDefs, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
+import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, HostBindingsFunction, HostDirectiveBindingMap, HostDirectiveDefs, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
 import {NodeInjectorFactory} from '../interfaces/injector';
 import {getUniqueLViewId} from '../interfaces/lview_tracking';
 import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliases, PropertyAliasValue, TAttributes, TConstantsOrFactory, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeType, TProjectionNode} from '../interfaces/node';
@@ -873,7 +873,7 @@ export function createTNode(
 function generatePropertyAliases(
     aliasMap: {[publicName: string]: string}, directiveIndex: number,
     propertyAliases: PropertyAliases|null,
-    hostDirectiveAliasMap: {[internalName: string]: string}|null): PropertyAliases|null {
+    hostDirectiveAliasMap: HostDirectiveBindingMap|null): PropertyAliases|null {
   for (let publicName in aliasMap) {
     if (aliasMap.hasOwnProperty(publicName)) {
       propertyAliases = propertyAliases === null ? {} : propertyAliases;