freezed icon indicating copy to clipboard operation
freezed copied to clipboard

Incorrect copyWith for nullable fields using non-nullable generic types

Open shilangyu opened this issue 1 year ago • 1 comments

Describe the bug

Continuation of #807

When using a non-nullable (ie. its type bound forces it to be non-nullable) type parameter, but then marking a field of this type nullable, the copywith implementation defaults fields to null which is incorrect since it can achieve a value of null. This violates a feature of freezed: copyWith can set nulls.

To Reproduce

Given a freezed object

@freezed
class State<T extends Object> with _$State<T> {
  const factory State({
    required T? value,
  }) = _State<T>;
}

The generated copyWith defaults to null

// ...
  @pragma('vm:prefer-inline')
  @override
  $Res call({
    Object? value = null,
  }) {
    return _then(_value.copyWith(
      value: null == value
          ? _value.value
          : value // ignore: cast_nullable_to_non_nullable
              as T?,
    ) as $Val);
  }
// ...

Which breaks usages such as

  test('copy null', () {
    const state = State<int>(value: 123);

    // all good
    expect(state.value, 123);

    // ugh-oh, that should work, but it does not
    final copied = state.copyWith(value: null);
    expect(copied.value, null);
  });

Expected behavior

The above copyWith with a null value should work, as previous versions of freezed worked like that and no such breaking change was stated in the changelog. Luckily when upgrading freezed in our project tests caught this issue.

Additional info

Quoting my original issue:

The problem originates from this https://github.com/rrousselGit/freezed/pull/735 PR. [..] The speed gain is negligible and creates many edge cases, [...].

shilangyu avatar Jan 26 '23 13:01 shilangyu

Any updates on this?

aliaksei-liavonik avatar Jul 12 '23 11:07 aliaksei-liavonik