diff --git a/lib/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart b/lib/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart index 0cf76e70..2ff07952 100644 --- a/lib/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart +++ b/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 { - 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); } } diff --git a/lib/src/dart3_suggestors/null_safety_prep/utils/class_component_required_fields.dart b/lib/src/dart3_suggestors/null_safety_prep/utils/class_component_required_fields.dart index 7e7ce2ec..71fc2ec3 100644 --- a/lib/src/dart3_suggestors/null_safety_prep/utils/class_component_required_fields.dart +++ b/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 Function(InterfaceElement) getAll, Iterable 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)); diff --git a/lib/src/dart3_suggestors/required_props/bin/codemod.dart b/lib/src/dart3_suggestors/required_props/bin/codemod.dart index 22fec655..c69fe738 100644 --- a/lib/src/dart3_suggestors/required_props/bin/codemod.dart +++ b/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, [ diff --git a/lib/src/dart3_suggestors/required_props/codemod/recommender.dart b/lib/src/dart3_suggestors/required_props/codemod/recommender.dart index c51b4d32..79a38f09 100644 --- a/lib/src/dart3_suggestors/required_props/codemod/recommender.dart +++ b/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; diff --git a/lib/src/executables/null_safety_migrator_companion.dart b/lib/src/executables/null_safety_migrator_companion.dart index 87567ab0..2055085c 100644 --- a/lib/src/executables/null_safety_migrator_companion.dart +++ b/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 args) async { dartPaths, aggregate([ CallbackRefHintSuggestor(), - ClassComponentRequiredDefaultPropsMigrator(), ClassComponentRequiredInitialStateMigrator(), ]), defaultYes: true, diff --git a/test/executables/required_props_collect_and_codemod_test.dart b/test/executables/required_props_collect_and_codemod_test.dart index a4e61a98..7a93fba0 100644 --- a/test/executables/required_props_collect_and_codemod_test.dart +++ b/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 diff --git a/test/test_fixtures/required_props/test_package/lib/entrypoint.dart b/test/test_fixtures/required_props/test_package/lib/entrypoint.dart index 71c5e221..2a2c1067 100644 --- a/test/test_fixtures/required_props/test_package/lib/entrypoint.dart +++ b/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'; diff --git a/test/test_fixtures/required_props/test_package/lib/src/test_class_component_defaults.dart b/test/test_fixtures/required_props/test_package/lib/src/test_class_component_defaults.dart new file mode 100644 index 00000000..0cef4e6f --- /dev/null +++ b/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 TestPrivate = + castUiFactory(_$TestPrivate); // ignore: undefined_identifier + +class TestPrivateComponent extends UiComponent2 { + @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 TestPublic2 = + castUiFactory(_$TestPublic2); // ignore: undefined_identifier + +class TestPublic2Component extends UiComponent2 { + @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 + )(); +}