FED-3160 Improve null safety docs and messages for wrapper components and connect

doc/null_safety/null_safe_migration.md

@@ -138,6 +138,14 @@ Run the null safety migrator tool:
 
 Below are some common cases that might come up while running the migrator tool on a repo using over_react.
 
+* [Prop requiredness and nullability](#prop-requiredness-and-nullability)
+* [Wrapper and `connect`ed components and required props](#wrapper-and-connected-components-and-required-props)
+* [Implementing abstract `Ref`s](#implementing-abstract-refs)
+* [Incorrect `getDerivedStateFromProps` Return Signature](#incorrect-getderivedstatefromprops-return-signature)
+* [Verbose function component return signature for `uiForwardRef`](#verbose-function-component-return-signature-for-uiforwardref)
+* [Nullable store/actions generics on `FluxUiPropsMixin`](#nullable-storeactions-generics-on-fluxuipropsmixin)
+* [Nullable props generic on `UiComponent2` mixins](#nullable-props-generic-on-uicomponent2-mixins)
+
 #### Prop requiredness and nullability
 
 First, check out our documentation around [null safety and required props](../null_safety_and_required_props.md).
@@ -188,6 +196,21 @@ flowchart TD
   End_Required[/"Make it <strong>required</strong>\n<code>late SomeType propName;</code>"\]
 ```
 
+### Wrapper and `connect`ed components and required props
+
+There may be some cases where you have a wrapper component or `connect`ed component that sets some required props, 
+but you get lints and runtime errors about missing props when consuming them.
+
+See [this section](../null_safety_and_required_props.md#disabling-required-prop-validation-for-certain-props)
+of the null safety and required props docs for instructions on how to handle these cases and suppress this validation.
+
+#### connect
+
+We'll be adding a codemod to help migrate the `connect` case: https://github.com/Workiva/over_react_codemod/issues/295
+
+Alternatively, you could refactor your component to instead utilize [OverReact Redux hooks](./over_react_redux_documentation.md#hooks), 
+which avoid this problem by accessing store data and dispatchers directly in the component as opposed to passing it in via props.
+
 #### Implementing abstract `Ref`s
 
 After migrating to null-safety, it may be necessary to add left side typing on `Ref`s when overriding abstract getters as shown in the example below:

doc/null_safety_and_required_props.md

@@ -120,8 +120,9 @@ This mechanism does not apply to function components, which use a different prop
 
 Sometimes, you want to declare a prop as non-nullable and required, but not enforce that consumers explicitly set it.
 
-There are two ways to opt out of prop validation for certain props, targeted toward to main use-cases:
+There are two ways to opt out of prop validation for certain props, targeted toward these main use-cases:
 - [wrapper components](#disabling-validation-use-case-wrapper-components)
+- [connect](#disabling-validation-use-case-connect)
 - [cloned props](#disabling-validation-use-case-cloned-props)
 
 #### Disabling validation use-case: wrapper components
@@ -180,6 +181,40 @@ class WrapperProps = UiProps with FooProps, WrapperPropsMixin;
 > See the [unsafe required prop reads](#unsafe-required-prop-reads) section for more info 
 
 
+#### Disabling validation use-case: `connect`
+Similar to [the wrapper component case](#disabling-validation-use-case-wrapper-components) in the previous section, 
+we'll want to disable validation similarly using `@Props(disableRequiredPropValidation: {...})` 
+for any late required props assigned within connect.
+
+For example:
+```dart
+mixin CounterPropsMixin on UiProps {
+  // Set in connect.
+  late int count;
+  late void Function() increment;
+
+  // Must be set by consumers of the connected compoennt.
+  late String requiredByConsumer;
+}
+
+@Props(disableRequiredPropValidation: {'count', 'increment'})
+class CounterProps = UiProps with CounterPropsMixin, OtherPropsMixin;
+
+UiFactory<CounterProps> Counter = connect<CounterState, CounterProps>(
+  mapStateToProps: (state) => (Counter()
+    ..count = state.count
+  ),
+  mapDispatchToProps: (dispatch) => (Counter()
+    ..increment = (() => dispatch(IncrementAction()))
+  ),
+)(_$Counter);
+
+example() => (Counter()..requiredByConsumer = 'foo')();
+```
+
+Note that [OverReact Redux hooks](./over_react_redux_documentation.md#hooks) 
+avoid this problem by accessing store data and dispatchers directly in the component as opposed to passing it in via props.
+
 #### Disabling validation use-case: cloned props
 
 Sometimes, you want to declare a prop that's always cloned onto it by a parent component. 

doc/over_react_redux_documentation.md

@@ -181,6 +181,24 @@ UiFactory<CounterProps> Counter = connect<CounterState, CounterProps>(
 )(_$Counter);
 ```
 
+Note that any required props assigned in connect must have their validation disabled; see docs [here](./null_safety_and_required_props.md#disabling-validation-use-case-connect)
+for more info. 
+
+For example:
+```dart
+mixin CounterPropsMixin on UiProps {
+  // Set in connect.
+  late int count;
+  late void Function() increment;
+
+  // Must be set by consumers of the connected compoennt.
+  late String requiredByConsumer;
+}
+
+@Props(disableRequiredPropValidation: {'count', 'increment'})
+class CounterProps = UiProps with CounterPropsMixin, OtherPropsMixin;
+```
+
 ### `connect` Parameters
 
 - #### `mapStateToProps`

lib/src/component_declaration/builder_helpers.dart

@@ -158,8 +158,13 @@ class MissingRequiredPropsError extends Error {
 
   MissingRequiredPropsError(this._message);
 
+  static const _messageSuffix = ' Ensure this prop is either directly set, or indirectly set via prop forwarding.'
+      'If this error seems unexpected and this component uses connect or mixes in required props from another component,'
+      ' please refer to the null safety migration guide for instructions on how to proceed:'
+      ' https://github.com/Workiva/over_react/blob/master/doc/null_safety/null_safe_migration.md#wrapper-and-connected-components-and-required-props';
+
   @override
-  String toString() => 'RequiredPropsError: $_message';
+  String toString() => 'RequiredPropsError: $_message$_messageSuffix';
 }
 
 /// Helper static extension methods to make forwarding props easier.

tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart

@@ -20,45 +20,93 @@ const _desc = r'Always provide required props.';
 // <editor-fold desc="Documentation Details">
 const _details = r'''
 
-**ALWAYS** provide a value for `late` required props.
+**ALWAYS** provide a value for `late` required props, either directly or by forwarding props.
 
-If a component has a props interface like this:
+Please see the documentation for [null safety and required props](https://github.com/Workiva/over_react/blob/master/doc/null_safety_and_required_props.md)
+for more information on required prop validation, which, in addition to this lint, also includes runtime `assert`s.
 
-```dart
-mixin NavItemProps on UiProps {
-  bool? isActive;
+Those docs also note exceptions to this rule under the [Disabling required prop validation for certain props](https://github.com/Workiva/over_react/blob/master/doc/null_safety_and_required_props.md#disabling-required-prop-validation-for-certain-props) 
+section, and include instructions for handling those. One common case where this doesn't apply are "wrapper" components that render another component and set some of its props within its render.
+
+### Examples:
+
+Given the following component with the required prop `user`:
 
-  late void Function() onDidActivate;
+```dart
+mixin UserChipProps on UiProps {
+  late User user;
+  bool? isSelected;
 }
+
+UiFactory<UserChipProps> UserChip = uiFunction((props) {
+  // ...
+}, _$UserChipConfig);
 ```
 
-Then the late required prop must always be set by the consumer:
+Then whenever UserChip is render, that required prop must always be set by the consumer.
 
 **GOOD:**
 ```dart
-@override
-render() {
-  return (NavItem()
-    ..onDidActivate = () {
-      // Do something
-    }
-  )(
-    'Activate me',
-  );
-}
+    (UserChip()..user = user)()
 ```
 
 **BAD:**
 ```dart
-@override
-render() {
-  return NavItem()(
-    'You probably cannot activate me :(',
-  );
+   UserChip()()
+// ^^^^^^^^^^
+// warning: Missing required late prop 'user' from 'UserChipProps'.
+// (over_react_late_required_prop)
+```
+and, that code will also throw a runtime error when asserts are enabled:
+
+```
+Uncaught Error: RequiredPropsError: Required prop `user` is missing.
+   at Object.throw_ [as throw]
+   at _$$UserChipProps$JsMap.new.validateRequiredProps
+```
+
+#### Prop forwarding
+
+**GOOD:**
+```dart
+mixin CustomUserChipPropsMixin on UiProps {
+  String? color;
 }
+
+class CustomUserChipProps = UiProps with UserChipProps, CustomUserChipPropsMixin;
+
+UiFactory<CustomUserChipProps> CustomUserChip = uiFunction((props) {
+  final color = props.color;
+
+  return (UserChip()
+    // Required props are correctly forwarded here by the wrapper component 
+    ..addProps(props.getPropsToForward(exclude: {CustomUserChipPropsMixin})
+    ..style = {
+      if (color != null) 'border': '2px solid $color', 
+      ...?props.style,
+    } 
+  )();
+}, _$CustomUserChipConfig);
 ```
 
+**BAD:**
+```dart
+UiFactory<CustomUserChipProps> CustomUserChip = uiFunction((props) {
+  final color = props.color;
+
+  // Required props are not forwarded, so we get:
+  //   warning: Missing required late prop 'user' from 'UserChipProps'.
+  //   (over_react_late_required_prop)
+  return (UserChip()
+    ..style = {
+      if (color != null) 'border': '2px solid $color', 
+      ...?props.style,
+    } 
+  )();
+}, _$CustomUserChipConfig);
+```
 ''';
+
 // </editor-fold>
 
 class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor {
@@ -82,7 +130,9 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor
   );
 
   static const _correctionMessage =
-      "Either set this prop, or mix it into the enclosing component's props and forward it.";
+      "Either set this prop, or mix it into the enclosing component's props and forward it."
+      " Or, if this is a wrapper component that sets this prop within its render,"
+      " disable validation for this prop (see instructions in documentation link).";
 
   static DiagnosticCode _codeForRequiredness(PropRequiredness requiredness) {
     switch (requiredness) {