rrousselGit / freezed

Code generation for immutable classes that has a simple syntax/API without compromising on the features.
https://pub.dev/packages/freezed
1.93k stars 237 forks source link

Incorrect copyWith for nullable fields using non-nullable generic types #847

Open shilangyu opened 1 year ago

shilangyu commented 1 year ago

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, [...].

aliaksei-liavonik commented 1 year ago

Any updates on this?

khylenko commented 2 months ago

Still no update?