Merge pull request #862 from Workiva/validate-required-props-null-safety

FED-1886 Null-safe props: runtime validation of required props

lib/src/builder/codegen/accessors_generator.dart

@@ -99,6 +99,7 @@ abstract class TypedMapAccessorsGenerator extends BoilerplateDeclarationGenerato
     }
 
     generatedClass.write(_generateAccessors());
+
     generatedClass.writeln('}');
     generatedClass.writeln();
     return generatedClass.toString();
@@ -149,6 +150,8 @@ abstract class TypedMapAccessorsGenerator extends BoilerplateDeclarationGenerato
 
     StringBuffer output = StringBuffer();
 
+    final requiredPropChecks = <String>[];
+
     node.members.whereType<FieldDeclaration>().where((field) => !field.isStatic).forEach((field) {
       T? getConstantAnnotation<T>(AnnotatedNode member, String name, T value) {
         return member.metadata.any((annotation) => annotation.name.name == name) ? value : null;
@@ -158,6 +161,8 @@ abstract class TypedMapAccessorsGenerator extends BoilerplateDeclarationGenerato
       final requiredProp = getConstantAnnotation(field, 'requiredProp', annotations.requiredProp);
       final nullableRequiredProp =
           getConstantAnnotation(field, 'nullableRequiredProp', annotations.nullableRequiredProp);
+      final disableRequiredPropValidation = getConstantAnnotation(
+          field, 'disableRequiredPropValidation', annotations.disableRequiredPropValidation);
 
       if (accessorMeta?.doNotGenerate == true) {
         return;
@@ -198,6 +203,12 @@ abstract class TypedMapAccessorsGenerator extends BoilerplateDeclarationGenerato
           annotationCount++;
           isRequired = true;
           isPotentiallyNullable = true;
+
+          if (type.isProps && disableRequiredPropValidation == null) {
+            requiredPropChecks.add('  if(!props.containsKey($keyValue)) {\n'
+                '  throw MissingRequiredPropsError(${stringLiteral('Required prop `$accessorName` is missing.')});\n'
+                '}\n');
+          }
         }
 
         if (accessorMeta != null) {
@@ -352,6 +363,18 @@ abstract class TypedMapAccessorsGenerator extends BoilerplateDeclarationGenerato
 
     output.write(staticVariablesImpl);
 
+    if (type.isProps &&
+        version != Version.v3_legacyDart2Only &&
+        version != Version.v2_legacyBackwardsCompat) {
+      final validateRequiredPropsMethod = '\n  @override\n'
+          '  @mustCallSuper\n'
+          '  void validateRequiredProps() {\n'
+          '    super.validateRequiredProps();\n'
+          '    ${requiredPropChecks.join('\n')}\n'
+          '  }\n';
+      output.write(validateRequiredPropsMethod);
+    }
+
     return output.toString();
   }
 }

lib/src/component_declaration/annotations.dart

@@ -15,6 +15,9 @@
 // Dummy annotations that would be used by Pub code generator
 library over_react.component_declaration.annotations;
 
+// Exported for use in generated code.
+export 'package:meta/meta.dart' show mustCallSuper;
+
 /// Annotation used with the `over_react` builder to declare a `UiFactory` for a component.
 ///
 ///     @Factory()
@@ -274,7 +277,7 @@ class AbstractComponent2 implements AbstractComponent { // ignore: deprecated_me
 ///
 /// Classes using this annotation must include the abstract `props` getter.
 ///
-/// __Deprecated.__ Use the `@Props()` annotation instead if you need to make use of an annotation argument. 
+/// __Deprecated.__ Use the `@Props()` annotation instead if you need to make use of an annotation argument.
 /// Otherwise, this can be removed completely. Will be removed in the 4.0.0 release of over_react.
 @Deprecated('Use the @Props() annotation if you need to make use of an annotation argument. Otherwise, this can be removed completely. Will be removed in the 4.0.0 release of over_react.')
 class PropsMixin implements TypedMap {
@@ -298,7 +301,7 @@ class PropsMixin implements TypedMap {
 ///
 /// Classes using this annotation must include the abstract `state` getter.
 ///
-/// __Deprecated.__ Use the `@State()` annotation instead if you need to make use of an annotation argument. 
+/// __Deprecated.__ Use the `@State()` annotation instead if you need to make use of an annotation argument.
 /// Otherwise, this can be removed completely. Will be removed in the 4.0.0 release of over_react.
 @Deprecated('Use the @State() annotation if you need to make use of an annotation argument. Otherwise, this can be removed completely. Will be removed in the 4.0.0 release of over_react.')
 class StateMixin implements TypedMap {
@@ -381,3 +384,10 @@ class Accessor {
 abstract class TypedMap {
   String? get keyNamespace;
 }
+
+/// Prevents required prop validation from being performed on a prop.
+const _DisableRequiredPropValidation disableRequiredPropValidation = _DisableRequiredPropValidation();
+
+class _DisableRequiredPropValidation {
+  const _DisableRequiredPropValidation();
+}

lib/src/component_declaration/builder_helpers.dart

@@ -14,7 +14,6 @@
 
 library over_react.component_declaration.builder_helpers;
 
-import 'package:meta/meta.dart';
 import '../../over_react.dart';
 import './component_base.dart' as component_base;
 import './annotations.dart' as annotations;
@@ -131,63 +130,6 @@ abstract class UiProps extends component_base.UiProps with GeneratedClass {
   /// This can be used to derive consumed props by usage in conjunction with [addUnconsumedProps]
   /// and [addUnconsumedDomProps].
   @toBeGenerated PropsMetaCollection get staticMeta => throw UngeneratedError(member: #meta);
-
-  @override
-  @visibleForOverriding
-  @mustCallSuper
-  void validateRequiredProps() {
-    super.validateRequiredProps();
-    // This fails when staticMeta isn't generated, so return early for now so tests don't fail.
-    // FIXME(null-safety) generate a static implementation of this instead in FED-1886, and remove this
-    return;
-
-    // ignore: dead_code
-    List<PropDescriptor>? missingRequiredProps;
-    List<PropDescriptor>? nullNonNullableRequiredProps;
-
-    for (final meta in staticMeta.all) {
-      for (final prop in meta.props) {
-        if (prop.isRequired) {
-          if (prop.isNullable) {
-            if (!props.containsKey(prop.key)) {
-              (missingRequiredProps ??= []).add(prop);
-            }
-          } else {
-            // Avoid looking up the key twice.
-            if (props[prop.key] == null) {
-              if (props.containsKey(prop.key)) {
-                (nullNonNullableRequiredProps ??= []).add(prop);
-              } else {
-                (missingRequiredProps ??= []).add(prop);
-              }
-            }
-          }
-        }
-      }
-    }
-
-    if (missingRequiredProps == null && nullNonNullableRequiredProps == null) {
-      return;
-    }
-
-    String formatPropKey(String propKey) => '`$propKey`';
-
-    final messageSegments = <String>[];
-    if (missingRequiredProps != null) {
-      messageSegments.add('Required props are missing: ${missingRequiredProps.map((prop) {
-        var propMessage = formatPropKey(prop.key);
-        if (prop.isNullable) propMessage += ' (can be null, but must be specified)';
-        return propMessage;
-      }).join(' ,')}.');
-    }
-    if (nullNonNullableRequiredProps != null) {
-      messageSegments
-          .add('Required non-nullable props are null: ${nullNonNullableRequiredProps.map((prop) {
-        return formatPropKey(prop.key);
-      }).join(' ,')}.');
-    }
-    throw MissingRequiredPropsError(messageSegments.join(' '));
-  }
 }
 
 class MissingRequiredPropsError extends Error {

lib/src/component_declaration/component_base.dart

@@ -621,9 +621,10 @@ abstract class UiProps extends MapBase
 
     assert(_validateChildren(childArguments.length == 1 ? childArguments.single : childArguments));
 
-    // FIXME(null-safety) finalize this implementation and add escape-hatch to opt out in FED-1886
     assert(() {
-      validateRequiredProps();
+      if (_shouldValidateRequiredProps) {
+        validateRequiredProps();
+      }
       return true;
     }());
 
@@ -676,10 +677,22 @@ abstract class UiProps extends MapBase
       : const {};
   }
 
-  // FIXME(null-safety) document and generate overrides in FED-1886
+  /// Validate at run-time that all required props are set.
+  ///
+  /// This method is overridden in generated files.
   @visibleForOverriding
   @mustCallSuper
   void validateRequiredProps() {}
+
+  /// Whether [validateRequiredProps] should be run.
+  var _shouldValidateRequiredProps = true;
+
+  /// Prevents [validateRequiredProps] from being called.
+  ///
+  /// Allows validation to be skipped to support cases where required props are cloned onto an element.
+  void disableRequiredPropValidation() {
+    _shouldValidateRequiredProps = false;
+  }
 }
 
 /// A class that declares the `_map` getter shared by [PropsMapViewMixin]/[StateMapViewMixin] and [MapViewMixin].

lib/src/over_react_redux/over_react_redux.dart

@@ -21,6 +21,7 @@ import 'package:meta/meta.dart';
 import 'package:over_react/component_base.dart';
 import 'package:over_react/src/component_declaration/annotations.dart';
 import 'package:over_react/src/component_declaration/builder_helpers.dart' as builder_helpers;
+import 'package:over_react/src/component_declaration/builder_helpers.dart' show MissingRequiredPropsError;
 import 'package:over_react/src/component_declaration/component_type_checking.dart';
 import 'package:over_react/src/component_declaration/function_component.dart';
 import 'package:over_react/src/util/context.dart';
@@ -51,7 +52,9 @@ abstract class _$ConnectPropsMixin implements UiProps {
   @override
   Map get props;
 
-  dynamic Function(dynamic action)? dispatch;
+  // Disable validation since this prop is set by the `connect` HOC, and does not need to be set by consumers.
+  @disableRequiredPropValidation
+  late dynamic Function(dynamic action) dispatch;
 }
 
 // ignore: prefer_generic_function_type_aliases

lib/src/util/react_util.dart

@@ -101,4 +101,8 @@ class UiPropsMapView extends MapView
 
   @override
   void validateRequiredProps() => throw UnimplementedError('@PropsMixin instances do not implement validateRequiredProps');
+
+  @override
+  void disableRequiredPropValidation() =>
+      throw UnimplementedError('@PropsMixin instances do not implement disableRequiredPropValidation');
 }