Merge pull request #297 from Workiva/public-defaulted-props

FED-2994 Update class component defaults codemod to take public-ness into account

Author: rmconsole3-wf

lib/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart

@@ -21,6 +21,7 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:over_react_codemod/src/vendor/over_react_analyzer_plugin/get_all_props.dart';
 import 'package:pub_semver/pub_semver.dart';
 
+import '../required_props/codemod/recommender.dart';
 import 'utils/class_component_required_fields.dart';
 
 /// Suggestor to assist with preparations for null-safety by adding
@@ -76,7 +77,10 @@ import 'utils/class_component_required_fields.dart';
 /// ```
 class ClassComponentRequiredDefaultPropsMigrator
     extends ClassComponentRequiredFieldsMigrator<PropAssignment> {
-  ClassComponentRequiredDefaultPropsMigrator([Version? sdkVersion])
+  final PropRequirednessRecommender? _propRequirednessRecommender;
+
+  ClassComponentRequiredDefaultPropsMigrator(
+      [Version? sdkVersion, this._propRequirednessRecommender])
       : super('defaultProps', 'getDefaultProps', sdkVersion);
 
   @override
@@ -105,6 +109,7 @@ class ClassComponentRequiredDefaultPropsMigrator
         .map((assignment) => PropAssignment(assignment))
         .where((prop) => prop.node.writeElement?.displayName != null);
 
-    patchFieldDeclarations(getAllProps, cascadedDefaultProps, node);
+    patchFieldDeclarations(
+        getAllProps, cascadedDefaultProps, node, _propRequirednessRecommender);
   }
 }

lib/src/dart3_suggestors/null_safety_prep/utils/class_component_required_fields.dart

@@ -25,6 +25,7 @@ import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:pub_semver/pub_semver.dart';
 
 import '../../../util/class_suggestor.dart';
+import '../../required_props/codemod/recommender.dart';
 import '../analyzer_plugin_utils.dart';
 
 /// A class shared by the suggestors that manage defaultProps/initialState.
@@ -52,7 +53,8 @@ abstract class ClassComponentRequiredFieldsMigrator<
   void patchFieldDeclarations(
       Iterable<FieldElement> Function(InterfaceElement) getAll,
       Iterable<Assignment> cascadedDefaultPropsOrInitialState,
-      CascadeExpression node) {
+      CascadeExpression node,
+      [PropRequirednessRecommender? _propRequirednessRecommender]) {
     for (final field in cascadedDefaultPropsOrInitialState) {
       final isDefaultedToNull =
           field.node.rightHandSide.staticType!.isDartCoreNull;
@@ -66,6 +68,14 @@ abstract class ClassComponentRequiredFieldsMigrator<
       // The field declaration is likely in another file which our logic currently doesn't handle.
       // In this case, don't add an entry to `fieldData`.
       if (fieldDeclaration == null) continue;
+      final element = fieldDeclaration.declaredElement;
+
+      // Don't set as required if the prop is publicly exported.
+      if (_propRequirednessRecommender != null && element is FieldElement) {
+        final isPublic = _propRequirednessRecommender
+            .isPropsPublicForMixingIn(element.enclosingElement);
+        if (isPublic) continue;
+      }
 
       fieldData.add(DefaultedOrInitializedDeclaration(
           fieldDeclaration, fieldEl, isDefaultedToNull));

lib/src/dart3_suggestors/required_props/bin/codemod.dart

@@ -24,6 +24,7 @@ import 'package:over_react_codemod/src/util/args.dart';
 import 'package:over_react_codemod/src/util/command_runner.dart';
 import 'package:over_react_codemod/src/util/package_util.dart';
 
+import '../../null_safety_prep/class_component_required_default_props.dart';
 import '../codemod/recommender.dart';
 import '../collect/aggregated_data.sg.dart';
 
@@ -148,6 +149,16 @@ class CodemodCommand extends Command {
           parsedArgs.argValueAsNumber(_Options.publicMaxAllowedSkipRate),
     );
 
+    exitCode = await runInteractiveCodemodSequence(
+      dartPaths,
+      [
+        ClassComponentRequiredDefaultPropsMigrator(null, recommender),
+      ],
+      defaultYes: true,
+      args: codemodArgs,
+      additionalHelpOutput: argParser.usage,
+    );
+
     exitCode = await runInteractiveCodemodSequence(
       dartPaths,
       [

lib/src/dart3_suggestors/required_props/codemod/recommender.dart

@@ -78,6 +78,9 @@ class PropRequirednessRecommender {
         ?[propsId];
   }
 
+  bool isPropsPublicForMixingIn(Element propsElement) =>
+      _getMixinResult(propsElement)?.visibility.isPublicForMixingIn ?? false;
+
   SkipRateOptionalReason? _getMixinSkipRateReason(MixinResult mixinResults) {
     final skipRate = mixinResults.usageSkipRate;
 

lib/src/executables/null_safety_migrator_companion.dart

@@ -16,7 +16,6 @@ import 'dart:io';
 
 import 'package:args/args.dart';
 import 'package:codemod/codemod.dart';
-import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart';
 import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/class_component_required_initial_state.dart';
 import 'package:over_react_codemod/src/util.dart';
 
@@ -43,7 +42,6 @@ void main(List<String> args) async {
     dartPaths,
     aggregate([
       CallbackRefHintSuggestor(),
-      ClassComponentRequiredDefaultPropsMigrator(),
       ClassComponentRequiredInitialStateMigrator(),
     ]),
     defaultYes: true,

test/executables/required_props_collect_and_codemod_test.dart

@@ -86,6 +86,24 @@ mixin TestPrivateProps on UiProps {
   String/*?*/ set20percent;
   $noDataTodoComment
   String/*?*/ set0percent;
+}''')),
+              d.file('test_class_component_defaults.dart', contains('''
+mixin TestPrivatePropsMixin on UiProps {
+  String/*?*/ notDefaultedOptional;
+  /*late*/ String notDefaultedAlwaysSet;
+  /*late*/ String/*?*/ defaultedNullable;
+  /*late*/ num/*!*/ defaultedNonNullable;
+}
+
+mixin SomeOtherPropsMixin on UiProps {
+  /*late*/ num/*!*/ anotherDefaultedNonNullable;
+}''')),
+              d.file('test_class_component_defaults.dart', contains('''
+mixin TestPublic2PropsMixin on UiProps {
+  String/*?*/ notDefaultedOptional;
+  /*late*/ String notDefaultedAlwaysSet;
+  String/*?*/ defaultedNullable;
+  num/*?*/ defaultedNonNullable;
 }''')),
               d.file('test_private_dynamic.dart', contains('''
 // TODO(orcm.required_props): This codemod couldn't reliably determine requiredness for these props

test/test_fixtures/required_props/test_package/lib/entrypoint.dart

@@ -1,3 +1,4 @@
+export 'src/test_class_component_defaults.dart' show TestPublic2, TestPublic2PropsMixin;
 export 'src/test_public.dart';
 export 'src/test_public_dynamic.dart';
 export 'src/test_public_multiple_components.dart';

test/test_fixtures/required_props/test_package/lib/src/test_class_component_defaults.dart

@@ -0,0 +1,76 @@
+import 'package:over_react/over_react.dart';
+
+part 'test_class_component_defaults.over_react.g.dart';
+
+mixin TestPrivatePropsMixin on UiProps {
+  String notDefaultedOptional;
+  String notDefaultedAlwaysSet;
+  String defaultedNullable;
+  num defaultedNonNullable;
+}
+
+mixin SomeOtherPropsMixin on UiProps {
+  num anotherDefaultedNonNullable;
+}
+
+class TestPrivateProps = UiProps
+    with TestPrivatePropsMixin, SomeOtherPropsMixin;
+
+UiFactory<TestPrivateProps> TestPrivate =
+    castUiFactory(_$TestPrivate); // ignore: undefined_identifier
+
+class TestPrivateComponent extends UiComponent2<TestPrivateProps> {
+  @override
+  get defaultProps => (newProps()
+    ..defaultedNullable = null
+    ..defaultedNonNullable = 2.1
+    ..anotherDefaultedNonNullable = 1.1
+  );
+
+  @override
+  render() {}
+}
+
+mixin TestPublic2PropsMixin on UiProps {
+  String notDefaultedOptional;
+  String notDefaultedAlwaysSet;
+  String defaultedNullable;
+  num defaultedNonNullable;
+}
+
+class TestPublic2Props = UiProps
+    with TestPublic2PropsMixin, SomeOtherPropsMixin;
+
+UiFactory<TestPublic2Props> TestPublic2 =
+    castUiFactory(_$TestPublic2); // ignore: undefined_identifier
+
+class TestPublic2Component extends UiComponent2<TestPublic2Props> {
+  @override
+  get defaultProps => (newProps()
+    ..defaultedNullable = null
+    ..defaultedNonNullable = 2.1
+    ..anotherDefaultedNonNullable = 1.1
+  );
+
+  @override
+  render() {}
+}
+
+usages() {
+  (TestPrivate()..notDefaultedAlwaysSet = 'abc')();
+  (TestPrivate()
+    ..notDefaultedOptional = 'abc'
+    ..notDefaultedAlwaysSet = 'abc'
+    ..defaultedNullable = 'abc'
+    ..defaultedNonNullable = 1
+    ..anotherDefaultedNonNullable = 2
+  )();
+  (TestPublic2()..notDefaultedAlwaysSet = 'abc')();
+  (TestPublic2()
+    ..notDefaultedAlwaysSet = 'abc'
+    ..notDefaultedOptional = 'abc'
+    ..defaultedNullable = 'abc'
+    ..defaultedNonNullable = 1
+    ..anotherDefaultedNonNullable = 2
+  )();
+}