From 26313c1b111ca9209ec0aed965fb531b31889054 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 15 May 2024 11:37:52 -0700 Subject: [PATCH 1/9] Cherry pick Greg's spike to only validate on null safety --- lib/src/builder/codegen/typed_map_impl_generator.dart | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/src/builder/codegen/typed_map_impl_generator.dart b/lib/src/builder/codegen/typed_map_impl_generator.dart index e7d20c261..6b9679e58 100644 --- a/lib/src/builder/codegen/typed_map_impl_generator.dart +++ b/lib/src/builder/codegen/typed_map_impl_generator.dart @@ -265,6 +265,17 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator { ..writeln(' @override') ..writeln( ' String \$getPropKey(void Function(Map m) accessMap) => $topLevelGetPropKeyAliasName(accessMap, (map) => ${names.implName}(map));'); + + if (!nullSafety) { + buffer + ..writeln() + ..writeln(' @override') + ..writeln(' // ignore: must_call_super') + ..writeln(' validateRequiredProps() {') + ..writeln(' // Disable required prop validation, until this component is null safe, by not calling super.') + ..writeln(' }'); + + } } final requiredPropNamesToSkipValidation = this.requiredPropNamesToSkipValidation; if (requiredPropNamesToSkipValidation != null && requiredPropNamesToSkipValidation.isNotEmpty) { From 3e4ea32ba2a03e9c0fec91803f5fd2bd3b471bc7 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 15 May 2024 11:38:07 -0700 Subject: [PATCH 2/9] Generate files --- .../over_react_flux.over_react.g.dart | 2 + test/mockito.mocks.dart | 70 ++++++++++++++----- ...n_error_integration_test.over_react.g.dart | 24 +++++++ ...mponent_integration_test.over_react.g.dart | 18 +++++ ...ccessor_integration_test.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ .../private_props_ddc_bug.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ ...mponent_integration_test.over_react.g.dart | 6 ++ ...ed_prop_integration_test.over_react.g.dart | 6 ++ ...mponent_integration_test.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ .../private_props_ddc_bug.over_react.g.dart | 6 ++ ...ccessor_integration_test.over_react.g.dart | 6 ++ ...mponent_integration_test.over_react.g.dart | 6 ++ ...ed_prop_integration_test.over_react.g.dart | 6 ++ 19 files changed, 187 insertions(+), 17 deletions(-) diff --git a/lib/src/over_react_redux/over_react_flux.over_react.g.dart b/lib/src/over_react_redux/over_react_flux.over_react.g.dart index afd92756d..f3887e9e7 100644 --- a/lib/src/over_react_redux/over_react_flux.over_react.g.dart +++ b/lib/src/over_react_redux/over_react_flux.over_react.g.dart @@ -16,11 +16,13 @@ abstract class ConnectFluxPropsMixin /// @override + @disableRequiredPropValidation TActions get actions => (props[_$key__actions___$ConnectFluxPropsMixin] ?? null) as TActions; /// @override + @disableRequiredPropValidation set actions(TActions value) => props[_$key__actions___$ConnectFluxPropsMixin] = value; /* GENERATED CONSTANTS */ diff --git a/test/mockito.mocks.dart b/test/mockito.mocks.dart index abff00e9d..d839bdb20 100644 --- a/test/mockito.mocks.dart +++ b/test/mockito.mocks.dart @@ -1,4 +1,4 @@ -// Mocks generated by Mockito 5.4.0 from annotations +// Mocks generated by Mockito 5.4.2 from annotations // in over_react/test/mockito.dart. // Do not manually edit this file. @@ -42,16 +42,19 @@ class MockLogger extends _i1.Mock implements _i3.Logger { Invocation.getter(#name), returnValue: '', ) as String); + @override Map get children => (super.noSuchMethod( Invocation.getter(#children), returnValue: {}, ) as Map); + @override String get fullName => (super.noSuchMethod( Invocation.getter(#fullName), returnValue: '', ) as String); + @override _i2.Level get level => (super.noSuchMethod( Invocation.getter(#level), @@ -60,6 +63,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { Invocation.getter(#level), ), ) as _i2.Level); + @override set level(_i2.Level? value) => super.noSuchMethod( Invocation.setter( @@ -68,11 +72,19 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + + @override + _i4.Stream<_i2.Level?> get onLevelChanged => (super.noSuchMethod( + Invocation.getter(#onLevelChanged), + returnValue: _i4.Stream<_i2.Level?>.empty(), + ) as _i4.Stream<_i2.Level?>); + @override _i4.Stream<_i5.LogRecord> get onRecord => (super.noSuchMethod( Invocation.getter(#onRecord), returnValue: _i4.Stream<_i5.LogRecord>.empty(), ) as _i4.Stream<_i5.LogRecord>); + @override void clearListeners() => super.noSuchMethod( Invocation.method( @@ -81,6 +93,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override bool isLoggable(_i2.Level? value) => (super.noSuchMethod( Invocation.method( @@ -89,6 +102,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValue: false, ) as bool); + @override void log( _i2.Level? logLevel, @@ -110,6 +124,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void finest( Object? message, [ @@ -127,6 +142,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void finer( Object? message, [ @@ -144,6 +160,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void fine( Object? message, [ @@ -161,6 +178,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void config( Object? message, [ @@ -178,6 +196,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void info( Object? message, [ @@ -195,6 +214,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void warning( Object? message, [ @@ -212,6 +232,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void severe( Object? message, [ @@ -229,6 +250,7 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); + @override void shout( Object? message, [ @@ -257,31 +279,37 @@ class MockMap extends _i1.Mock implements Map { Invocation.getter(#entries), returnValue: >[], ) as Iterable>); + @override Iterable get keys => (super.noSuchMethod( Invocation.getter(#keys), returnValue: [], ) as Iterable); + @override Iterable get values => (super.noSuchMethod( Invocation.getter(#values), returnValue: [], ) as Iterable); + @override int get length => (super.noSuchMethod( Invocation.getter(#length), returnValue: 0, ) as int); + @override bool get isEmpty => (super.noSuchMethod( Invocation.getter(#isEmpty), returnValue: false, ) as bool); + @override bool get isNotEmpty => (super.noSuchMethod( Invocation.getter(#isNotEmpty), returnValue: false, ) as bool); + @override Map cast() => (super.noSuchMethod( Invocation.method( @@ -290,6 +318,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: {}, ) as Map); + @override bool containsValue(Object? value) => (super.noSuchMethod( Invocation.method( @@ -298,6 +327,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: false, ) as bool); + @override bool containsKey(Object? key) => (super.noSuchMethod( Invocation.method( @@ -306,6 +336,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: false, ) as bool); + @override void operator []=( K? key, @@ -321,13 +352,13 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); + @override Map map( MapEntry Function( - K, - V, - )? - convert) => + K, + V, + )? convert) => (super.noSuchMethod( Invocation.method( #map, @@ -335,6 +366,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: {}, ) as Map); + @override void addEntries(Iterable>? newEntries) => super.noSuchMethod( Invocation.method( @@ -343,6 +375,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); + @override V update( K? key, @@ -364,13 +397,13 @@ class MockMap extends _i1.Mock implements Map { ifAbsent: ifAbsent, ), ) as V); + @override void updateAll( V Function( - K, - V, - )? - update) => + K, + V, + )? update) => super.noSuchMethod( Invocation.method( #updateAll, @@ -378,13 +411,13 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); + @override void removeWhere( bool Function( - K, - V, - )? - test) => + K, + V, + )? test) => super.noSuchMethod( Invocation.method( #removeWhere, @@ -392,6 +425,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); + @override V putIfAbsent( K? key, @@ -410,6 +444,7 @@ class MockMap extends _i1.Mock implements Map { ifAbsent, ), ) as V); + @override void addAll(Map? other) => super.noSuchMethod( Invocation.method( @@ -418,6 +453,7 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); + @override void clear() => super.noSuchMethod( Invocation.method( @@ -426,13 +462,13 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); + @override void forEach( void Function( - K, - V, - )? - action) => + K, + V, + )? action) => super.noSuchMethod( Invocation.method( #forEach, diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/annotation_error_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/annotation_error_integration_test.over_react.g.dart index e861bbd8d..280b528eb 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/annotation_error_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/annotation_error_integration_test.over_react.g.dart @@ -81,6 +81,12 @@ class _$$AnnotationErrorDefaultPropsProps _$getPropKey$_$$AnnotationErrorDefaultPropsProps( accessMap, (map) => _$$AnnotationErrorDefaultPropsProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } @@ -183,6 +189,12 @@ class _$$AnnotationErrorProps extends _$AnnotationErrorProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$AnnotationErrorProps( accessMap, (map) => _$$AnnotationErrorProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -282,6 +294,12 @@ class _$$AnnotationErrorStatefulProps extends _$AnnotationErrorStatefulProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$AnnotationErrorStatefulProps( accessMap, (map) => _$$AnnotationErrorStatefulProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -436,6 +454,12 @@ class _$$AnnotationErrorStatefulDefaultPropsProps _$getPropKey$_$$AnnotationErrorStatefulDefaultPropsProps( accessMap, (map) => _$$AnnotationErrorStatefulDefaultPropsProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/component_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/component_integration_test.over_react.g.dart index 8556de25a..47d7ab1d3 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/component_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/component_integration_test.over_react.g.dart @@ -219,6 +219,12 @@ abstract class _$$ComponentTestProps extends _$ComponentTestProps _$getPropKey$_$$ComponentTestProps( accessMap, (map) => _$$ComponentTestProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id', 'shouldSetPropsDirectly', 'shouldUseJsFactory'}; @@ -379,6 +385,12 @@ abstract class _$$IsErrorBoundaryProps extends _$IsErrorBoundaryProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$IsErrorBoundaryProps( accessMap, (map) => _$$IsErrorBoundaryProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -535,6 +547,12 @@ abstract class _$$IsNotErrorBoundaryProps extends _$IsNotErrorBoundaryProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$IsNotErrorBoundaryProps( accessMap, (map) => _$$IsNotErrorBoundaryProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/constant_required_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/constant_required_accessor_integration_test.over_react.g.dart index 0d34583d6..df8aa9061 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/constant_required_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/constant_required_accessor_integration_test.over_react.g.dart @@ -137,6 +137,12 @@ abstract class _$$ComponentTestProps extends _$ComponentTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$ComponentTestProps( accessMap, (map) => _$$ComponentTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/do_not_generate_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/do_not_generate_accessor_integration_test.over_react.g.dart index 9f347c39f..2034393b6 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/do_not_generate_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/do_not_generate_accessor_integration_test.over_react.g.dart @@ -140,6 +140,12 @@ abstract class _$$DoNotGenerateAccessorTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$DoNotGenerateAccessorTestProps( accessMap, (map) => _$$DoNotGenerateAccessorTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/namespaced_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/namespaced_accessor_integration_test.over_react.g.dart index b8f0e02d3..6a3cc75dd 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/namespaced_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/namespaced_accessor_integration_test.over_react.g.dart @@ -197,6 +197,12 @@ abstract class _$$NamespacedAccessorTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$NamespacedAccessorTestProps( accessMap, (map) => _$$NamespacedAccessorTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/private_props_ddc_bug.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/private_props_ddc_bug.over_react.g.dart index b4525b768..0e4b7aa67 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/private_props_ddc_bug.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/private_props_ddc_bug.over_react.g.dart @@ -87,6 +87,12 @@ abstract class _$$FooProps extends _$FooProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$FooProps(accessMap, (map) => _$$FooProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'_privateProp'}; } diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/required_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/required_accessor_integration_test.over_react.g.dart index 362e8d117..c9384f8d0 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/required_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/required_accessor_integration_test.over_react.g.dart @@ -158,6 +158,12 @@ abstract class _$$ComponentTestProps extends _$ComponentTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$ComponentTestProps( accessMap, (map) => _$$ComponentTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/stateful_component_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/stateful_component_integration_test.over_react.g.dart index 981e92744..e6306cda3 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/stateful_component_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/stateful_component_integration_test.over_react.g.dart @@ -103,6 +103,12 @@ abstract class _$$StatefulComponentTestProps _$getPropKey$_$$StatefulComponentTestProps( accessMap, (map) => _$$StatefulComponentTestProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'setStateDirectly'}; diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/unassigned_prop_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/unassigned_prop_integration_test.over_react.g.dart index ca5029a6b..c19b33235 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/unassigned_prop_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component2/unassigned_prop_integration_test.over_react.g.dart @@ -106,6 +106,12 @@ abstract class _$$FooProps extends _$FooProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$FooProps(accessMap, (map) => _$$FooProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component_integration_test.over_react.g.dart index 24c461b47..5fd1f8ca3 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/component_integration_test.over_react.g.dart @@ -184,6 +184,12 @@ class _$$ComponentTestProps extends _$ComponentTestProps _$getPropKey$_$$ComponentTestProps( accessMap, (map) => _$$ComponentTestProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/constant_required_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/constant_required_accessor_integration_test.over_react.g.dart index d8d40d493..23bbc64d1 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/constant_required_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/constant_required_accessor_integration_test.over_react.g.dart @@ -115,6 +115,12 @@ class _$$ComponentTestProps extends _$ComponentTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$ComponentTestProps( accessMap, (map) => _$$ComponentTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/do_not_generate_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/do_not_generate_accessor_integration_test.over_react.g.dart index d6ce4f2c2..2497eb58d 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/do_not_generate_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/do_not_generate_accessor_integration_test.over_react.g.dart @@ -138,6 +138,12 @@ class _$$DoNotGenerateAccessorTestProps extends _$DoNotGenerateAccessorTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$DoNotGenerateAccessorTestProps( accessMap, (map) => _$$DoNotGenerateAccessorTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/namespaced_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/namespaced_accessor_integration_test.over_react.g.dart index 4fee24372..74a533cba 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/namespaced_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/namespaced_accessor_integration_test.over_react.g.dart @@ -195,6 +195,12 @@ class _$$NamespacedAccessorTestProps extends _$NamespacedAccessorTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$NamespacedAccessorTestProps( accessMap, (map) => _$$NamespacedAccessorTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/private_props_ddc_bug.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/private_props_ddc_bug.over_react.g.dart index 706e06fc5..9b0596f0c 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/private_props_ddc_bug.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/private_props_ddc_bug.over_react.g.dart @@ -86,6 +86,12 @@ class _$$FooProps extends _$FooProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$FooProps(accessMap, (map) => _$$FooProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'_privateProp'}; } diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/required_accessor_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/required_accessor_integration_test.over_react.g.dart index 8ea1d770e..620cef07c 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/required_accessor_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/required_accessor_integration_test.over_react.g.dart @@ -129,6 +129,12 @@ class _$$ComponentTestProps extends _$ComponentTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$ComponentTestProps( accessMap, (map) => _$$ComponentTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/stateful_component_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/stateful_component_integration_test.over_react.g.dart index 4df792e4c..c31f665b0 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/stateful_component_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/stateful_component_integration_test.over_react.g.dart @@ -77,6 +77,12 @@ class _$$StatefulComponentTestProps extends _$StatefulComponentTestProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$StatefulComponentTestProps( accessMap, (map) => _$$StatefulComponentTestProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/unassigned_prop_integration_test.over_react.g.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/unassigned_prop_integration_test.over_react.g.dart index 81225c519..3ff1e8529 100644 --- a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/unassigned_prop_integration_test.over_react.g.dart +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/unassigned_prop_integration_test.over_react.g.dart @@ -105,6 +105,12 @@ class _$$FooProps extends _$FooProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$FooProps(accessMap, (map) => _$$FooProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } From 0a49c882c3d2513f06cfc675ae532ca8a1cb805b Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 15 May 2024 11:41:30 -0700 Subject: [PATCH 3/9] Update test golds --- .../basic.over_react.g.dart.goldFile | 6 ++++ .../basic_library.over_react.g.dart.goldFile | 12 ++++++++ .../basic.over_react.g.dart.goldFile | 6 ++++ .../basic_library.over_react.g.dart.goldFile | 12 ++++++++ .../basic.over_react.g.dart.goldFile | 6 ++++ .../basic_library.over_react.g.dart.goldFile | 12 ++++++++ .../library.over_react.g.dart.goldFile | 6 ++++ .../basic.over_react.g.dart.goldFile | 6 ++++ .../basic_library.over_react.g.dart.goldFile | 12 ++++++++ .../basic_two_nine.over_react.g.dart.goldFile | 6 ++++ .../library.over_react.g.dart.goldFile | 6 ++++ ...tiple_factories.over_react.g.dart.goldfile | 6 ++++ ...type_parameters.over_react.g.dart.goldFile | 30 +++++++++++++++++++ 13 files changed, 126 insertions(+) diff --git a/test_fixtures/gold_output_files/backwards_compatible/basic.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/backwards_compatible/basic.over_react.g.dart.goldFile index 4af3bb11b..924a97bf2 100644 --- a/test_fixtures/gold_output_files/backwards_compatible/basic.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/backwards_compatible/basic.over_react.g.dart.goldFile @@ -158,6 +158,12 @@ class _$$BasicProps extends _$BasicProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicProps(accessMap, (map) => _$$BasicProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id', 'basicProp'}; diff --git a/test_fixtures/gold_output_files/backwards_compatible/basic_library.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/backwards_compatible/basic_library.over_react.g.dart.goldFile index e811f761e..bc380821c 100644 --- a/test_fixtures/gold_output_files/backwards_compatible/basic_library.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/backwards_compatible/basic_library.over_react.g.dart.goldFile @@ -174,6 +174,12 @@ class _$$BasicPartOfLibProps extends _$BasicPartOfLibProps _$getPropKey$_$$BasicPartOfLibProps( accessMap, (map) => _$$BasicPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } @@ -345,6 +351,12 @@ class _$$SubPartOfLibProps extends _$SubPartOfLibProps _$getPropKey$_$$SubPartOfLibProps( accessMap, (map) => _$$SubPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test_fixtures/gold_output_files/dart2_only/basic.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/dart2_only/basic.over_react.g.dart.goldFile index ca98058ad..d55ef3341 100644 --- a/test_fixtures/gold_output_files/dart2_only/basic.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/dart2_only/basic.over_react.g.dart.goldFile @@ -166,6 +166,12 @@ class _$$BasicProps extends _$BasicProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicProps(accessMap, (map) => _$$BasicProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id', 'basicProp'}; diff --git a/test_fixtures/gold_output_files/dart2_only/basic_library.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/dart2_only/basic_library.over_react.g.dart.goldFile index 5e0edbab9..c4931ffed 100644 --- a/test_fixtures/gold_output_files/dart2_only/basic_library.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/dart2_only/basic_library.over_react.g.dart.goldFile @@ -179,6 +179,12 @@ class _$$BasicPartOfLibProps extends _$BasicPartOfLibProps _$getPropKey$_$$BasicPartOfLibProps( accessMap, (map) => _$$BasicPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } @@ -360,6 +366,12 @@ class _$$SubPartOfLibProps extends _$SubPartOfLibProps _$getPropKey$_$$SubPartOfLibProps( accessMap, (map) => _$$SubPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test_fixtures/gold_output_files/dart2_only/component2/basic.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/dart2_only/component2/basic.over_react.g.dart.goldFile index 4cfc6bb51..d1c678ff7 100644 --- a/test_fixtures/gold_output_files/dart2_only/component2/basic.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/dart2_only/component2/basic.over_react.g.dart.goldFile @@ -165,6 +165,12 @@ abstract class _$$BasicProps extends _$BasicProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicProps(accessMap, (map) => _$$BasicProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id', 'basicProp'}; diff --git a/test_fixtures/gold_output_files/dart2_only/component2/basic_library.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/dart2_only/component2/basic_library.over_react.g.dart.goldFile index 28baf040b..99f0ffc4b 100644 --- a/test_fixtures/gold_output_files/dart2_only/component2/basic_library.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/dart2_only/component2/basic_library.over_react.g.dart.goldFile @@ -180,6 +180,12 @@ abstract class _$$BasicPartOfLibProps extends _$BasicPartOfLibProps _$getPropKey$_$$BasicPartOfLibProps( accessMap, (map) => _$$BasicPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } @@ -467,6 +473,12 @@ abstract class _$$SubPartOfLibProps extends _$SubPartOfLibProps _$getPropKey$_$$SubPartOfLibProps( accessMap, (map) => _$$SubPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test_fixtures/gold_output_files/dart2_only/missing_over_react_g_part/library.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/dart2_only/missing_over_react_g_part/library.over_react.g.dart.goldFile index 7f8a12f0d..0d549a568 100644 --- a/test_fixtures/gold_output_files/dart2_only/missing_over_react_g_part/library.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/dart2_only/missing_over_react_g_part/library.over_react.g.dart.goldFile @@ -94,6 +94,12 @@ class _$$BasicPartOfLibProps extends _$BasicPartOfLibProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicPartOfLibProps( accessMap, (map) => _$$BasicPartOfLibProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test_fixtures/gold_output_files/mixin_based/basic.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/mixin_based/basic.over_react.g.dart.goldFile index f1b67ab9b..08bb767f2 100644 --- a/test_fixtures/gold_output_files/mixin_based/basic.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/mixin_based/basic.over_react.g.dart.goldFile @@ -68,6 +68,12 @@ abstract class _$$BasicProps extends UiProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicProps(accessMap, (map) => _$$BasicProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id', 'basicProp'}; diff --git a/test_fixtures/gold_output_files/mixin_based/basic_library.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/mixin_based/basic_library.over_react.g.dart.goldFile index 0e694d21a..51b213146 100644 --- a/test_fixtures/gold_output_files/mixin_based/basic_library.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/mixin_based/basic_library.over_react.g.dart.goldFile @@ -77,6 +77,12 @@ abstract class _$$BasicPartOfLibProps extends UiProps _$getPropKey$_$$BasicPartOfLibProps( accessMap, (map) => _$$BasicPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } @@ -466,6 +472,12 @@ abstract class _$$SubPartOfLibProps extends UiProps _$getPropKey$_$$SubPartOfLibProps( accessMap, (map) => _$$SubPartOfLibProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id'}; } diff --git a/test_fixtures/gold_output_files/mixin_based/basic_two_nine.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/mixin_based/basic_two_nine.over_react.g.dart.goldFile index 387beddcb..f00d1ce44 100644 --- a/test_fixtures/gold_output_files/mixin_based/basic_two_nine.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/mixin_based/basic_two_nine.over_react.g.dart.goldFile @@ -68,6 +68,12 @@ abstract class _$$BasicProps extends UiProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicProps(accessMap, (map) => _$$BasicProps(map)); + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } + @override Set get requiredPropNamesToSkipValidation => const {'id', 'basicProp'}; diff --git a/test_fixtures/gold_output_files/mixin_based/missing_over_react_g_part/library.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/mixin_based/missing_over_react_g_part/library.over_react.g.dart.goldFile index fb156ca2a..a946f723a 100644 --- a/test_fixtures/gold_output_files/mixin_based/missing_over_react_g_part/library.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/mixin_based/missing_over_react_g_part/library.over_react.g.dart.goldFile @@ -69,6 +69,12 @@ abstract class _$$BasicPartOfLibProps extends UiProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$BasicPartOfLibProps( accessMap, (map) => _$$BasicPartOfLibProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test_fixtures/gold_output_files/mixin_based/two_nine_with_multiple_factories.over_react.g.dart.goldfile b/test_fixtures/gold_output_files/mixin_based/two_nine_with_multiple_factories.over_react.g.dart.goldfile index 7342553c6..870f87626 100644 --- a/test_fixtures/gold_output_files/mixin_based/two_nine_with_multiple_factories.over_react.g.dart.goldfile +++ b/test_fixtures/gold_output_files/mixin_based/two_nine_with_multiple_factories.over_react.g.dart.goldfile @@ -74,6 +74,12 @@ abstract class _$$CounterProps extends UiProps @override String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$CounterProps(accessMap, (map) => _$$CounterProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl diff --git a/test_fixtures/gold_output_files/mixin_based/type_parameters.over_react.g.dart.goldFile b/test_fixtures/gold_output_files/mixin_based/type_parameters.over_react.g.dart.goldFile index 62510e51e..3c0691ce3 100644 --- a/test_fixtures/gold_output_files/mixin_based/type_parameters.over_react.g.dart.goldFile +++ b/test_fixtures/gold_output_files/mixin_based/type_parameters.over_react.g.dart.goldFile @@ -234,6 +234,12 @@ abstract class _$$SingleProps extends UiProps @override String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$SingleProps(accessMap, (map) => _$$SingleProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -320,6 +326,12 @@ abstract class _$$SingleWithBoundProps extends UiProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$SingleWithBoundProps( accessMap, (map) => _$$SingleWithBoundProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -406,6 +418,12 @@ abstract class _$$DoubleProps extends UiProps @override String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$DoubleProps(accessMap, (map) => _$$DoubleProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -513,6 +531,12 @@ abstract class _$$ConcreteNoneProps extends UiProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$ConcreteNoneProps( accessMap, (map) => _$$ConcreteNoneProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl @@ -620,6 +644,12 @@ abstract class _$$ConcreteArgsProps extends UiProps String $getPropKey(void Function(Map m) accessMap) => _$getPropKey$_$$ConcreteArgsProps( accessMap, (map) => _$$ConcreteArgsProps(map)); + + @override + // ignore: must_call_super + validateRequiredProps() { + // Disable required prop validation, until this component is null safe, by not calling super. + } } /// An alias for [getPropKey] so it can be referenced within the props class impl From aa337ea2e146f2d865f9493be0fdafa9e2375eba Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 15 May 2024 16:50:01 -0700 Subject: [PATCH 4/9] Add tests --- .../codegen/typed_map_impl_generator.dart | 20 +++--- ...l_safety_validate_required_props_test.dart | 46 +++++++------- ...l_safety_validate_required_props_test.dart | 61 +++++++++++++++++++ ...ponent_declaration_non_null_safe_test.dart | 2 + ...ponent_declaration_non_null_safe_test.html | 1 + 5 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 test/over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart diff --git a/lib/src/builder/codegen/typed_map_impl_generator.dart b/lib/src/builder/codegen/typed_map_impl_generator.dart index 6b9679e58..6027db6ca 100644 --- a/lib/src/builder/codegen/typed_map_impl_generator.dart +++ b/lib/src/builder/codegen/typed_map_impl_generator.dart @@ -266,16 +266,16 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator { ..writeln( ' String \$getPropKey(void Function(Map m) accessMap) => $topLevelGetPropKeyAliasName(accessMap, (map) => ${names.implName}(map));'); - if (!nullSafety) { - buffer - ..writeln() - ..writeln(' @override') - ..writeln(' // ignore: must_call_super') - ..writeln(' validateRequiredProps() {') - ..writeln(' // Disable required prop validation, until this component is null safe, by not calling super.') - ..writeln(' }'); - - } + // if (!nullSafety) { + // buffer + // ..writeln() + // ..writeln(' @override') + // ..writeln(' // ignore: must_call_super') + // ..writeln(' validateRequiredProps() {') + // ..writeln( + // ' // Disable required prop validation, until this component is null safe, by not calling super.') + // ..writeln(' }'); + // } } final requiredPropNamesToSkipValidation = this.requiredPropNamesToSkipValidation; if (requiredPropNamesToSkipValidation != null && requiredPropNamesToSkipValidation.isNotEmpty) { diff --git a/test/over_react/component_declaration/builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart b/test/over_react/component_declaration/builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart index e2f945072..bd0f9ac2a 100644 --- a/test/over_react/component_declaration/builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart +++ b/test/over_react/component_declaration/builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart @@ -27,7 +27,7 @@ void main() { group('on invocation', () { test('via call()', () { expect(() { - (ComponentTest() + (RequiredPropsTest() ..requiredNullable = true )(); }, @@ -40,7 +40,7 @@ void main() { test('via build()', () { expect(() { - (ComponentTest() + (RequiredPropsTest() ..requiredNullable = true ).build(); }, @@ -54,7 +54,7 @@ void main() { test('on mount', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..requiredNullable = true )()); }, @@ -69,14 +69,14 @@ void main() { late rtl.RenderResult view; expect(() { - view = rtl.render((ComponentTest() + view = rtl.render((RequiredPropsTest() ..requiredNonNullable = true ..requiredNullable = true )()); }, returnsNormally); expect(() { - view.rerender((ComponentTest() + view.rerender((RequiredPropsTest() ..requiredNullable = true )()); }, @@ -90,7 +90,7 @@ void main() { test('does not throw when the prop is set', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..requiredNonNullable = true ..requiredNullable = true )()); @@ -103,7 +103,7 @@ void main() { group('on invocation', () { test('via call()', () { expect(() { - (ComponentTest() + (RequiredPropsTest() ..requiredNonNullable = true )(); }, @@ -115,7 +115,7 @@ void main() { test('via build()', () { expect(() { - (ComponentTest() + (RequiredPropsTest() ..requiredNonNullable = true ).build(); }, @@ -128,7 +128,7 @@ void main() { test('on mount', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..requiredNonNullable = true )()); }, @@ -142,14 +142,14 @@ void main() { late rtl.RenderResult view; expect(() { - view = rtl.render((ComponentTest() + view = rtl.render((RequiredPropsTest() ..requiredNonNullable = true ..requiredNullable = true )()); }, returnsNormally); expect(() { - view.rerender((ComponentTest() + view.rerender((RequiredPropsTest() ..requiredNonNullable = true )()); }, @@ -162,7 +162,7 @@ void main() { test('does not throw when the prop is set to null', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..requiredNonNullable = true ..requiredNullable = null )()); @@ -171,7 +171,7 @@ void main() { test('does not throw when the prop is set', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..requiredNonNullable = true ..requiredNullable = true )()); @@ -181,7 +181,7 @@ void main() { test('@disableRequiredPropValidation annotation turns off validation for specific props', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..requiredNonNullable = true ..requiredNullable = true )()); @@ -190,7 +190,7 @@ void main() { test('disableRequiredPropValidation method turns off validation for component usage', () { expect(() { - rtl.render((ComponentTest() + rtl.render((RequiredPropsTest() ..disableRequiredPropValidation() )()); }, allOf(returnsNormally, logsNoPropTypeWarnings)); @@ -301,18 +301,18 @@ void main() { test('(New boilerplate) validates required props: does not throw in dart2js', () { expect(() { - rtl.render(ComponentTest()()); - rtl.render(ComponentTest().build()); + rtl.render(RequiredPropsTest()()); + rtl.render(RequiredPropsTest().build()); }, returnsNormally); }, tags: 'no-ddc'); } // ignore: undefined_identifier, invalid_assignment -UiFactory ComponentTest = _$ComponentTest; +UiFactory RequiredPropsTest = _$RequiredPropsTest; @Props(disableRequiredPropValidation: {'testIgnoreOnMixin'}) -mixin ComponentTestProps on UiProps { +mixin RequiredPropsTestProps on UiProps { late bool requiredNonNullable; late bool? requiredNullable; @@ -332,7 +332,7 @@ mixin ComponentTestProps on UiProps { late bool testIgnoreOnMixin; } -class ComponentTestComponent extends UiComponent2 { +class RequiredPropsTestComponent extends UiComponent2 { @override render() => Dom.div()(); } @@ -347,7 +347,7 @@ mixin MultipleMixinsTestPropsMixin on UiProps { } @Props(disableRequiredPropValidation: {'testIgnoreOnMixin'}) -class MultipleMixinsTestProps = UiProps with MultipleMixinsTestPropsMixin, ComponentTestProps; +class MultipleMixinsTestProps = UiProps with MultipleMixinsTestPropsMixin, RequiredPropsTestProps; mixin WrapperTestPropsMixin on UiProps { @@ -361,7 +361,7 @@ UiFactory FunctionWrapperTest = uiFunction( @Props(disableRequiredPropValidation: {'thirdRequiredProp', 'secondRequiredProp', 'requiredNonNullable', 'requiredNullable', 'testIgnoreOnMixin'}) -class FunctionWrapperTestProps = UiProps with WrapperTestPropsMixin, MultipleMixinsTestPropsMixin, ComponentTestProps; +class FunctionWrapperTestProps = UiProps with WrapperTestPropsMixin, MultipleMixinsTestPropsMixin, RequiredPropsTestProps; @Factory() // ignore: undefined_identifier, invalid_assignment @@ -396,7 +396,7 @@ mixin ClassWrapperTest2PropsMixin on UiProps { } @Props(disableRequiredPropValidation: {'thirdRequiredProp', 'secondRequiredProp', 'requiredNonNullable', 'requiredNullable', 'testIgnoreOnMixin'}) -class ClassWrapperTest2Props = UiProps with ClassWrapperTest2PropsMixin, MultipleMixinsTestPropsMixin, ComponentTestProps; +class ClassWrapperTest2Props = UiProps with ClassWrapperTest2PropsMixin, MultipleMixinsTestPropsMixin, RequiredPropsTestProps; class ClassWrapperTest2Component extends UiComponent2 { @override diff --git a/test/over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart new file mode 100644 index 000000000..2d8709f4d --- /dev/null +++ b/test/over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart @@ -0,0 +1,61 @@ +// @dart=2.11 +// Copyright 2024 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'package:over_react/over_react.dart'; +import 'package:react_testing_library/react_testing_library.dart' as rtl; +import 'package:test/test.dart'; + +import '../../builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart'; + +part 'null_safety_validate_required_props_test.over_react.g.dart'; + +void main() { + group('(New boilerplate) required prop validation: does not throw for non-null safe components', + () { + group('on invocation', () { + test('via call()', () { + expect(() { + (Test()..requiredNullable = true)(); + }, returnsNormally); + }); + + test('via build()', () { + expect(() { + (Test()..requiredNullable = true).build(); + }, returnsNormally); + }); + }); + + test('on mount', () { + expect(() { + rtl.render((Test()..requiredNullable = true)()); + }, returnsNormally); + }); + }); +} + +UiFactory Test = castUiFactory(_$Test); // ignore: undefined_identifier + +mixin TestPropsMixin on UiProps {} + +class TestProps = UiProps with TestPropsMixin, RequiredPropsTestProps; + +class TestComponent extends UiComponent2 { + @override + get defaultProps => (newProps()); + + @override + render() {} +} diff --git a/test/over_react_component_declaration_non_null_safe_test.dart b/test/over_react_component_declaration_non_null_safe_test.dart index 6d88db08b..e272e5026 100644 --- a/test/over_react_component_declaration_non_null_safe_test.dart +++ b/test/over_react_component_declaration_non_null_safe_test.dart @@ -74,6 +74,7 @@ import 'over_react/component_declaration/non_null_safe_builder_integration_tests import 'over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/required_accessor_integration_test.dart' as new_boilerplate_required_accessor_integration_test; import 'over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/stateful_component_integration_test.dart' as new_boilerplate_stateful_component_integration_test; import 'over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/unassigned_prop_integration_test.dart' as new_boilerplate_unassigned_prop_integration_test; +import 'over_react/component_declaration/non_null_safe_builder_integration_tests/new_boilerplate/null_safety_validate_required_props_test.dart' as new_boilerplate_null_safety_validate_required_props_test; main() { enableTestMode(); @@ -130,4 +131,5 @@ main() { new_boilerplate_required_accessor_integration_test.main(); new_boilerplate_stateful_component_integration_test.main(); new_boilerplate_unassigned_prop_integration_test.main(); + new_boilerplate_null_safety_validate_required_props_test.main(); } diff --git a/test/over_react_component_declaration_non_null_safe_test.html b/test/over_react_component_declaration_non_null_safe_test.html index e4992d77e..b84f22ea7 100644 --- a/test/over_react_component_declaration_non_null_safe_test.html +++ b/test/over_react_component_declaration_non_null_safe_test.html @@ -21,6 +21,7 @@ + From 5398ae7c99754d95430acd8e84de0fcc123d1034 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 15 May 2024 17:13:49 -0700 Subject: [PATCH 5/9] Update analyzer plugin --- .../codegen/typed_map_impl_generator.dart | 20 +++--- test/mockito.mocks.dart | 70 +++++-------------- .../src/diagnostic/missing_required_prop.dart | 28 ++++---- .../non_null_safe/missing_required_props.dart | 6 +- 4 files changed, 48 insertions(+), 76 deletions(-) diff --git a/lib/src/builder/codegen/typed_map_impl_generator.dart b/lib/src/builder/codegen/typed_map_impl_generator.dart index 6027db6ca..89c796f4b 100644 --- a/lib/src/builder/codegen/typed_map_impl_generator.dart +++ b/lib/src/builder/codegen/typed_map_impl_generator.dart @@ -266,16 +266,16 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator { ..writeln( ' String \$getPropKey(void Function(Map m) accessMap) => $topLevelGetPropKeyAliasName(accessMap, (map) => ${names.implName}(map));'); - // if (!nullSafety) { - // buffer - // ..writeln() - // ..writeln(' @override') - // ..writeln(' // ignore: must_call_super') - // ..writeln(' validateRequiredProps() {') - // ..writeln( - // ' // Disable required prop validation, until this component is null safe, by not calling super.') - // ..writeln(' }'); - // } + if (!nullSafety) { + buffer + ..writeln() + ..writeln(' @override') + ..writeln(' // ignore: must_call_super') + ..writeln(' validateRequiredProps() {') + ..writeln( + ' // Disable required prop validation, until this component is null safe, by not calling super.') + ..writeln(' }'); + } } final requiredPropNamesToSkipValidation = this.requiredPropNamesToSkipValidation; if (requiredPropNamesToSkipValidation != null && requiredPropNamesToSkipValidation.isNotEmpty) { diff --git a/test/mockito.mocks.dart b/test/mockito.mocks.dart index d839bdb20..abff00e9d 100644 --- a/test/mockito.mocks.dart +++ b/test/mockito.mocks.dart @@ -1,4 +1,4 @@ -// Mocks generated by Mockito 5.4.2 from annotations +// Mocks generated by Mockito 5.4.0 from annotations // in over_react/test/mockito.dart. // Do not manually edit this file. @@ -42,19 +42,16 @@ class MockLogger extends _i1.Mock implements _i3.Logger { Invocation.getter(#name), returnValue: '', ) as String); - @override Map get children => (super.noSuchMethod( Invocation.getter(#children), returnValue: {}, ) as Map); - @override String get fullName => (super.noSuchMethod( Invocation.getter(#fullName), returnValue: '', ) as String); - @override _i2.Level get level => (super.noSuchMethod( Invocation.getter(#level), @@ -63,7 +60,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { Invocation.getter(#level), ), ) as _i2.Level); - @override set level(_i2.Level? value) => super.noSuchMethod( Invocation.setter( @@ -72,19 +68,11 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - - @override - _i4.Stream<_i2.Level?> get onLevelChanged => (super.noSuchMethod( - Invocation.getter(#onLevelChanged), - returnValue: _i4.Stream<_i2.Level?>.empty(), - ) as _i4.Stream<_i2.Level?>); - @override _i4.Stream<_i5.LogRecord> get onRecord => (super.noSuchMethod( Invocation.getter(#onRecord), returnValue: _i4.Stream<_i5.LogRecord>.empty(), ) as _i4.Stream<_i5.LogRecord>); - @override void clearListeners() => super.noSuchMethod( Invocation.method( @@ -93,7 +81,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override bool isLoggable(_i2.Level? value) => (super.noSuchMethod( Invocation.method( @@ -102,7 +89,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValue: false, ) as bool); - @override void log( _i2.Level? logLevel, @@ -124,7 +110,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void finest( Object? message, [ @@ -142,7 +127,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void finer( Object? message, [ @@ -160,7 +144,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void fine( Object? message, [ @@ -178,7 +161,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void config( Object? message, [ @@ -196,7 +178,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void info( Object? message, [ @@ -214,7 +195,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void warning( Object? message, [ @@ -232,7 +212,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void severe( Object? message, [ @@ -250,7 +229,6 @@ class MockLogger extends _i1.Mock implements _i3.Logger { ), returnValueForMissingStub: null, ); - @override void shout( Object? message, [ @@ -279,37 +257,31 @@ class MockMap extends _i1.Mock implements Map { Invocation.getter(#entries), returnValue: >[], ) as Iterable>); - @override Iterable get keys => (super.noSuchMethod( Invocation.getter(#keys), returnValue: [], ) as Iterable); - @override Iterable get values => (super.noSuchMethod( Invocation.getter(#values), returnValue: [], ) as Iterable); - @override int get length => (super.noSuchMethod( Invocation.getter(#length), returnValue: 0, ) as int); - @override bool get isEmpty => (super.noSuchMethod( Invocation.getter(#isEmpty), returnValue: false, ) as bool); - @override bool get isNotEmpty => (super.noSuchMethod( Invocation.getter(#isNotEmpty), returnValue: false, ) as bool); - @override Map cast() => (super.noSuchMethod( Invocation.method( @@ -318,7 +290,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: {}, ) as Map); - @override bool containsValue(Object? value) => (super.noSuchMethod( Invocation.method( @@ -327,7 +298,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: false, ) as bool); - @override bool containsKey(Object? key) => (super.noSuchMethod( Invocation.method( @@ -336,7 +306,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: false, ) as bool); - @override void operator []=( K? key, @@ -352,13 +321,13 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); - @override Map map( MapEntry Function( - K, - V, - )? convert) => + K, + V, + )? + convert) => (super.noSuchMethod( Invocation.method( #map, @@ -366,7 +335,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValue: {}, ) as Map); - @override void addEntries(Iterable>? newEntries) => super.noSuchMethod( Invocation.method( @@ -375,7 +343,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); - @override V update( K? key, @@ -397,13 +364,13 @@ class MockMap extends _i1.Mock implements Map { ifAbsent: ifAbsent, ), ) as V); - @override void updateAll( V Function( - K, - V, - )? update) => + K, + V, + )? + update) => super.noSuchMethod( Invocation.method( #updateAll, @@ -411,13 +378,13 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); - @override void removeWhere( bool Function( - K, - V, - )? test) => + K, + V, + )? + test) => super.noSuchMethod( Invocation.method( #removeWhere, @@ -425,7 +392,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); - @override V putIfAbsent( K? key, @@ -444,7 +410,6 @@ class MockMap extends _i1.Mock implements Map { ifAbsent, ), ) as V); - @override void addAll(Map? other) => super.noSuchMethod( Invocation.method( @@ -453,7 +418,6 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); - @override void clear() => super.noSuchMethod( Invocation.method( @@ -462,13 +426,13 @@ class MockMap extends _i1.Mock implements Map { ), returnValueForMissingStub: null, ); - @override void forEach( void Function( - K, - V, - )? action) => + K, + V, + )? + action) => super.noSuchMethod( Invocation.method( #forEach, diff --git a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart index ace040450..2122b329d 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart @@ -4,6 +4,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:over_react_analyzer_plugin/src/diagnostic/analyzer_debug_helper.dart'; import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart'; import 'package:over_react_analyzer_plugin/src/util/ast_util.dart'; +import 'package:over_react_analyzer_plugin/src/util/null_safety_utils.dart'; import 'package:over_react_analyzer_plugin/src/util/pretty_print.dart'; import 'package:over_react_analyzer_plugin/src/util/prop_declarations/props_set_by_factory.dart'; import 'package:over_react_analyzer_plugin/src/util/util.dart'; @@ -204,18 +205,21 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor continue; } - await collector.addErrorWithFix( - _codeForRequiredness(requiredness), - result.locationFor(usage.builder), - errorMessageArgs: ["'$name' from '${field.enclosingElement.name}'"], - fixKind: fixKind, - fixMessageArgs: [name], - computeFix: () => buildFileEdit(result, (builder) { - addProp(usage, builder, result.content, result.lineInfo, name: name, buildValueEdit: (builder) { - builder.selectHere(); - }); - }), - ); + if(withNullability(result) || requiredness != PropRequiredness.late) { + await collector.addErrorWithFix( + _codeForRequiredness(requiredness), + result.locationFor(usage.builder), + errorMessageArgs: ["'$name' from '${field.enclosingElement.name}'"], + fixKind: fixKind, + fixMessageArgs: [name], + computeFix: () => buildFileEdit(result, (builder) { + addProp(usage, builder, result.content, result.lineInfo, name: name, + buildValueEdit: (builder) { + builder.selectHere(); + }); + }), + ); + } } // Include debug info for each invocation ahout all the props and their requirednesses. diff --git a/tools/analyzer_plugin/playground/web/non_null_safe/missing_required_props.dart b/tools/analyzer_plugin/playground/web/non_null_safe/missing_required_props.dart index d467a49bf..f348db704 100644 --- a/tools/analyzer_plugin/playground/web/non_null_safe/missing_required_props.dart +++ b/tools/analyzer_plugin/playground/web/non_null_safe/missing_required_props.dart @@ -2,6 +2,8 @@ // This file is a non-null safe copy of playground examples to enable QAing backwards compatibility. import 'package:over_react/over_react.dart'; +import '../missing_required_props.dart'; + part 'missing_required_props.over_react.g.dart'; main() { @@ -12,7 +14,7 @@ main() { UiFactory Bar = castUiFactory(_$Bar); // ignore: undefined_identifier -mixin BarProps on UiProps { +mixin BarPropsMixin on UiProps { @requiredProp String barRequired; @@ -20,6 +22,8 @@ mixin BarProps on UiProps { String bar; } +class BarProps = UiProps with BarPropsMixin, WithLateRequiredProps; + class BarComponent extends UiComponent2 { @override render() => Dom.div()(); From 45c4d3a6a6c8f20b59bad1657568fecd2399768d Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Thu, 16 May 2024 10:06:21 -0700 Subject: [PATCH 6/9] Add analyzer plugin test --- .../src/diagnostic/missing_required_prop.dart | 4 +- .../missing_required_prop_test.dart | 195 ++++++++++++++++++ 2 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart diff --git a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart index 2122b329d..f2f508f73 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart @@ -205,7 +205,7 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor continue; } - if(withNullability(result) || requiredness != PropRequiredness.late) { + // if(withNullability(result) || requiredness != PropRequiredness.late) { await collector.addErrorWithFix( _codeForRequiredness(requiredness), result.locationFor(usage.builder), @@ -219,7 +219,7 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor }); }), ); - } + // } } // Include debug info for each invocation ahout all the props and their requirednesses. diff --git a/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart b/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart new file mode 100644 index 000000000..17c7c178b --- /dev/null +++ b/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart @@ -0,0 +1,195 @@ +// ignore_for_file: camel_case_types +import 'dart:async'; + +import 'package:over_react_analyzer_plugin/src/diagnostic/missing_required_prop.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../matchers.dart'; +import '../../test_bases/diagnostic_test_base.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(MissingRequiredPropTest_NoErrors); + defineReflectiveTests(MissingRequiredPropTest_MissingAnnotationRequired); + }); +} + +abstract class MissingRequiredPropTest extends DiagnosticTestBase { + @override + get fixKindUnderTest => null; + + Source newSourceWithPrefix(String sourceFragment) => newSource(sourcePrefix + sourceFragment); + + static const sourcePrefix = /*language=dart*/ r''' +// @dart=2.11 +import 'package:over_react/over_react.dart'; + +part '{{FILE_BASENAME_WITHOUT_EXTENSION}}.over_react.g.dart'; + +// ignore_for_file: unused_local_variable + +UiFactory GenericFactory = uiFunction((_) {}, UiFactoryConfig()); + +UiFactory NoRequired = uiFunction((_) {}, _$NoRequiredConfig); +mixin NoRequiredProps on UiProps { + String optional1; + String optional2; +} + +UiFactory InheritsLateRequired = uiFunction((_) {}, _$InheritsLateRequiredConfig); +mixin InheritsLateRequiredPropsMixin on UiProps { + String optional1; +} +class InheritsLateRequiredProps = UiProps with FluxUiPropsMixin, InheritsLateRequiredPropsMixin; + +UiFactory WithAnnotationRequired = uiFunction((_) {}, _$WithAnnotationRequiredConfig); +mixin WithAnnotationRequiredProps on UiProps { + @requiredProp String required1; + @requiredProp String required2; + String optional1; + String optional2; +} +'''; +} + +@reflectiveTest +class MissingRequiredPropTest_NoErrors extends MissingRequiredPropTest { + @override + get errorUnderTest => null; + + Future test_noErrorsWhenNoRequiredProps() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + test() => Fragment()( + NoRequired()(), + (NoRequired())(), + (NoRequired()..optional1 = '1')(), + (NoRequired() + ..optional1 = '1' + ..optional2 = '2' + )(), + (NoRequired()..id = 'id')(), + ); + ''')); + } + + Future test_noErrorsForDomComponents() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + test() => Fragment()( + // DomProps + Dom.div()(), + (Dom.div()..id = 'dom')(), + // SvgProps + Dom.ellipse()(), + (Dom.ellipse()..id = 'svg')(), + ); + ''')); + } + + Future test_noErrorsForGenericUiProps() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + test() => Fragment()( + GenericFactory()(), + (GenericFactory()..id = 'dom')(), + ); + ''')); + } + + Future test_noErrorsWhenIncompleteBuilder() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + testLate() { + final incompleteBuilder = InheritsLateRequired()..optional1 = '1'; + + final incompleteBuilderInvokedLater = InheritsLateRequired()..optional1 = '1'; + incompleteBuilderInvokedLater(); + } + + testAnnotation() { + final incompleteBuilder = WithAnnotationRequired()..optional1 = '1'; + + final incompleteBuilderInvokedLater = WithAnnotationRequired()..optional1 = '1'; + incompleteBuilderInvokedLater(); + } + ''')); + } + + Future test_noErrorsAnnotationRequiredByDefault() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + test() => WithAnnotationRequired()(); + ''')); + } + + Future test_noErrorsInheritsLateRequired() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + test() => InheritsLateRequired()(); + ''')); + } +} + +@reflectiveTest +class MissingRequiredPropTest_MissingAnnotationRequired extends MissingRequiredPropTest { + @override + get errorUnderTest => MissingRequiredPropDiagnostic.annotationRequiredCode; + + @override + get fixKindUnderTest => MissingRequiredPropDiagnostic.fixKind; + + // This lint is disabled by default, so we have to opt into it. + @override + String get analysisOptionsYamlContents => r''' +analyzer: + plugins: + over_react + +over_react: + errors: + over_react_annotation_required_prop: info + '''; + + Future test_singleMissingAndFix() async { + final source = newSourceWithPrefix(/*language=dart*/ r''' + test() => (WithAnnotationRequired() + ..optional1 = '1' + ..required1 = '1' + )(); + '''); + final selection = createSelection(source, '#WithAnnotationRequired()#'); + final error = await expectSingleErrorAt(selection, hasFix: true); + expect(error.message, "Missing @requiredProp 'required2' from 'WithAnnotationRequiredProps'."); + + final errorFix = await expectSingleErrorFix(selection); + final fixedSource = applyErrorFixes(errorFix, source); + expect(fixedSource.contents.data, contains(/*language=dart*/ r''' + test() => (WithAnnotationRequired() + ..optional1 = '1' + ..required1 = '1' + ..required2 = + )(); + ''')); + } + + Future test_multipleMissing() async { + final source = newSourceWithPrefix(/*language=dart*/ r''' + test() => (WithAnnotationRequired()..optional1 = '1')(); + '''); + final selection = createSelection(source, '#WithAnnotationRequired()#'); + final allErrors = await getAllErrors(source); + expect( + allErrors, + unorderedEquals([ + isAnErrorUnderTest(locatedAt: selection) + .havingMessage(contains("'required1' from 'WithAnnotationRequiredProps'")), + isAnErrorUnderTest(locatedAt: selection) + .havingMessage(contains("'required2' from 'WithAnnotationRequiredProps'")), + ])); + } + + Future test_noErrorsAllRequiredPropsSpecified() async { + await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' + test() => (WithAnnotationRequired() + ..required1 = '' + ..required2 = '' + )(); + ''')); + } +} From 4966462130f2aad6db3de404d216c8df0b60e175 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Thu, 16 May 2024 10:15:23 -0700 Subject: [PATCH 7/9] Add analyzer plugin fix back --- .../lib/src/diagnostic/missing_required_prop.dart | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart index f2f508f73..b586fd5fa 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart @@ -205,7 +205,7 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor continue; } - // if(withNullability(result) || requiredness != PropRequiredness.late) { + if (withNullability(result) || requiredness != PropRequiredness.late) { await collector.addErrorWithFix( _codeForRequiredness(requiredness), result.locationFor(usage.builder), @@ -213,13 +213,12 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor fixKind: fixKind, fixMessageArgs: [name], computeFix: () => buildFileEdit(result, (builder) { - addProp(usage, builder, result.content, result.lineInfo, name: name, - buildValueEdit: (builder) { + addProp(usage, builder, result.content, result.lineInfo, name: name, buildValueEdit: (builder) { builder.selectHere(); }); }), ); - // } + } } // Include debug info for each invocation ahout all the props and their requirednesses. From b6f90ea0ef490aff14fc21bb0e70285bbd0bec1b Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Thu, 16 May 2024 10:18:46 -0700 Subject: [PATCH 8/9] Clean up --- .../over_react_flux.over_react.g.dart | 2 -- .../non_null_safe/missing_required_prop_test.dart | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/src/over_react_redux/over_react_flux.over_react.g.dart b/lib/src/over_react_redux/over_react_flux.over_react.g.dart index f3887e9e7..afd92756d 100644 --- a/lib/src/over_react_redux/over_react_flux.over_react.g.dart +++ b/lib/src/over_react_redux/over_react_flux.over_react.g.dart @@ -16,13 +16,11 @@ abstract class ConnectFluxPropsMixin /// @override - @disableRequiredPropValidation TActions get actions => (props[_$key__actions___$ConnectFluxPropsMixin] ?? null) as TActions; /// @override - @disableRequiredPropValidation set actions(TActions value) => props[_$key__actions___$ConnectFluxPropsMixin] = value; /* GENERATED CONSTANTS */ diff --git a/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart b/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart index 17c7c178b..98fa0c87f 100644 --- a/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart +++ b/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart @@ -1,4 +1,18 @@ // ignore_for_file: camel_case_types +// Copyright 2024 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + import 'dart:async'; import 'package:over_react_analyzer_plugin/src/diagnostic/missing_required_prop.dart'; From 594fc271c8d23ecc7c425a73fe50d45e5318334a Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 22 May 2024 10:58:50 -0700 Subject: [PATCH 9/9] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63dbc96b6..e0d8cb6ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Use the `useRefInit` hook instead if you need to set an initial value. - [#909] Deprecate the optional `defaultValue` argument in `createContext`. - Use `createContextInit` instead if you need to set a default value. +- [#917] Only validate required props in null-safe components ## 5.0.1 - Consume over_react_test 3.0.0