freezed icon indicating copy to clipboard operation
freezed copied to clipboard

feat: make generated classes sealed/final

Open Yegair opened this issue 5 months ago • 8 comments

Adds a new configuration parameter finalize: bool that allows to mark generated classes as final in the case of concrete classes or sealed in the case of abstract classes. It is disabled by default.

Enabling it allows the Dart analyzer/compiler to report patterns that will never match the type of a freezed class at compile time.

For example:

@Freezed(finalize: true)
sealed class Foo with _$Foo {
  const Foo._();
  const factory Foo() = _Foo;
}

@Freezed(finalize: true)
sealed class Bar with _$Bar {
  const Bar._();
  const factory Bar() = Bar;
}

void main() {
  switch (Foo()) {
    // This will now cause an analyzer warning: pattern_never_matches_value_type.
    // Without finalize: true it would not cause any warning/error at all.
    case Bar():
      print("matched Bar()");
      return;

    case Foo():
      print("matched Foo()");
      return;
  }
}

Why do I think this is a good idea?

I have recently had an issue in one of my projects where I have a @freezed sealed class DataFromApi and I introduced a new @freezed sealed class DataFromDatabase that was intended to be used in many but not all cases where DataFromApi was being used so far.

It was a normal change, but I messed up due to the heavy use of pattern matching like

switch (data) {
  case DataFromApi(someCondition: true):
    // ...
    break;

  case DataFromApi(someCondition: false):
    // ...
    break;

  default:
    // ...
    break;
}

The changes mostly looked like this

- final data = await loadDataFromApi();
+ final data = await loadDataFromDatabase();

switch (data) {
  case DataFromApi(someCondition: true):
    // ...
    break;

    // ...
}

I was expecting the Dart analyzer/compiler to yell at me when changing the type of data in the example above to DataFromDatabase, but that was not the case. So I had to find all the now broken patterns by hand. Naturally I overlooked one case and the tests didn't catch it, so I got a Bug in production 😅.

IMHO the Dart analyzer/compiler should be able to help me in this case no matter if I use freezed or not, since the classes in question were marked as sealed. So I opened an issue over there: https://github.com/dart-lang/sdk/issues/56616

However, since this might never be implemented or maybe is completely impossible, I thought it should be possible and rather simple to make freezed help me out in that case. All that needs to be done is mark the generated classes as final or sealed, and then the analyzer will step in and report any pattern that can never match at compile time.

What needs to be done before it might be merged?

Good question, I think first of all someone needs to decide if this even is a good idea or if it would mess things up. In case it should be merged, I assume a few more tests need to be written to make sure the finalize flag works well with other configuration options. However, so far I have a hard time figuring out what even might break due to this change, so I would need help to decide what special/edge cases to consider when writing tests.

Yegair avatar Aug 31 '24 13:08 Yegair