freezed icon indicating copy to clipboard operation
freezed copied to clipboard

`copyWith` getter causing `inconsistent_inheritance_getter_and_method` when extending abstract class

Open Craftplacer opened this issue 2 years ago • 6 comments

Describe the bug The generated freezed code does not satisfy the required methods imposed by the extended abstract class. A reason why this can't be "just changed" by the developer doing this is when using the ThemeExtension class from Flutter.

To Reproduce Something around the lines of:

abstract class A<T extends A<T>> {
  A<T> copyWith();
}

@freezed
// 'copyWith' is inherited as a getter (from '_$B') and also a method (from 'A').
// Try adjusting the supertypes of this class to remove the inconsistency.
// dart(inconsistent_inheritance_getter_and_method)
class B extends A<B> with _$B {
  const factory B(int value) = _B;
}

Expected behavior A plain method to be generated (not a function provided by a getter) in order to resolve the Dart error lint.

Craftplacer avatar Jun 18 '22 19:06 Craftplacer

The getter is necessary to support deep copy. Removing it would remove deep copy support, which I don't think is desirable

Of course, we could have an option to disable deep-copy.

rrousselGit avatar Jun 18 '22 19:06 rrousselGit

I didn't say anything about removing deep copy. Just disabling it when extending on classes that don't support it is fine. Providing an option to disable would be helpful too -- I couldn't find one.

Craftplacer avatar Jun 18 '22 19:06 Craftplacer

This would likely conflict with https://github.com/rrousselGit/freezed/issues/642 though

https://github.com/rrousselGit/freezed/issues/642 would rely on a getter regardless of whether there's deep copy or not.

rrousselGit avatar Jun 18 '22 19:06 rrousselGit

I don't see how hard it is to implement a different behavior (that is the good old naive approach as detailed in #642) that produces simpler code to accommodate this kind of situation. There really isn't a problem in my view to just generate code that doesn't use getters whenever it's not compatible. Those two code generation methods can exist at the same time, unless I am not aware of something in this library.

Craftplacer avatar Jun 19 '22 09:06 Craftplacer

You're mistaken. #642 is not compatible with your interface

It still uses a getter, and always will. Even without deep copy.

rrousselGit avatar Jun 19 '22 10:06 rrousselGit

@rrousselGit How would you suggest me to do in this case?

I need to have an abstraction, here it is the class SuperPerson and I need to be able to use .copyWith(name: 'newName') from it.

I would like to use freezed on the classes that implement SuperPerson (Person and OtherPerson in the code example):

abstract class SuperPerson {
  const SuperPerson();
  String get name;
  String get nameUppercase => name.toUpperCase();
  String get nameLowercase;

  SuperPerson copyWith({String name});
}

@freezed
class Person extends SuperPerson with _$Person { // <- Lint: 'copyWith' is inherited as a getter (from '_$Person') and also a method (from 'SuperPerson').
Try adjusting the supertypes of this class to remove the inconsistency.dart(inconsistent_inheritance_getter_and_method)
  const factory Person({
    required String name,
    required String email,
    required String phone,
  }) = _Person;

  const Person._();

  factory Person.fromJson(Map<String, dynamic> json) => _$PersonFromJson(json);

  @override
  String get nameLowercase => name.toLowerCase();
}

@freezed
class OtherPerson extends SuperPerson with _$OtherPerson {  // <- 'copyWith' is inherited as a getter (from '_$OtherPerson') and also a method (from 'SuperPerson').
Try adjusting the supertypes of this class to remove the inconsistency.dart(inconsistent_inheritance_getter_and_method)
  const factory OtherPerson({
    required String name,
    required String otherEmail,
    required String otherPhone,
  }) = _OtherPerson;

  const OtherPerson._();

  factory OtherPerson.fromJson(Map<String, dynamic> json) =>
      _$OtherPersonFromJson(json);

  @override
  String get nameLowercase => name.toLowerCase();
}

What would be the correct approach?

ValentinVignal avatar Aug 23 '22 08:08 ValentinVignal

Closing as, although this would be valuable, this is not feasible.

Freezed relies on using a getter for differentiating copyWith() from copyWIth(foo: null). As such, this is not possible.

rrousselGit avatar Sep 27 '22 14:09 rrousselGit

in case anybody runs into this issue, I have migrated my entire app away from freezed and used this package https://pub.dev/packages/mek_data_class instead, which creates copyWith with no magic and follows normal class syntax. If you only need data classes which have copyWith hashcode and equals and you need to implement abstract copyWith, this is a good alternative.

clragon avatar Feb 05 '23 13:02 clragon