Offer a migration tool to convert map/when to switch-cases about freezed HOT 7 OPEN

This could also be added to freezed_lint to migrate only one specific when/map instead of the whole project at once.

from freezed.

If we were to offer such migration in freezed_lint, the various challenges are not as problematic.

We could do a dumb conversion, ignoring all those issues. And the user would fix those by hand.

from freezed.

Your mention of codemod reminds me that the linter could technically work as a migration in itself.

There's an issue in custom_lint to offer a custom_lint fix command. invertase/dart_custom_lint#161

If this is implemented, then implementing a fix in the IDE would also work as a widespread migration tool too.

So maybe we should focus on the linter.

from freezed.

Things to figure out:

There may be no local variable

In that scenario, there's no local "obj" variable equivalent when using a switch:

doSomething().map(
 data: (obj) => print(obj.data),
 ...
)

switch(doSomething()) {
  case ExampleData(): print(<where is "obj" here?>);
  ...
)

Actually there is

switch(doSomething()) {
  case ExampleData obj: print(obj.data);
// or
  case final ExampleData obj: print(obj.data);
// or
  case ExampleData() && final obj: print(obj.data)
  ...
)

Another thing to consider with this approach is: What should the variable name be. We could name it based on the interface class name. So here Example -> example

Just use the name in the map expression, or am I missing something?

Sometimes we need a switch "statement", and sometimes a switch "expression"

Notice how:

• switches with a case don't end with a ;, but those with => do
• switches with a case use a ; at the end of every expression, but switches with => use ,

This should be doable. Just a matter of determining whether any of the body of the lambdas are not an expression body, which you would need a switch statement for, otherwise just default to a switch expression. The more difficult part is if it is used in an expression context. In which case you need a switch expression. In this case, although it is less readable you can use an immediately invoked function application: ((){..body})().

Sometimes the factory type is not public

In the example, both ExampleError/ExampleData are public. But they could be private, and we could be in a file that does not have access to the private type.

In that scenario, we cannot generate the when/map equivalent. We could have a warning asking folks to make those objects public.

This is definitely a problem that needs a warning. However, the fix can potentially be automated.

The freezed class may not be sealed

If the annotated Example class is not "sealed", then switches cannot be exhaustive. In this case, we'll need to consider all whens/maps as "maybeWhen/maybeMap". But we don't know how the current code should handle the default case.

The default case for maybeWhen should obviously be a catch all. For the exhaustive when / map, a throw statement should be good, though I would also output a list of all the places this was added due to not having a sealed class - so the user knows which classes they need to make sealed, or have an option for the migration tool to do a dry run, and only output the warnings for the sealed classes, so they can be fixed prior to actually migrating. Some classes might potentially be outside the users control, so it still should be able to migrate without sealed classes.

Closures may use function modifiers

We could do:

• If the closure is async and the returned value is awaited, generate the switch anyway.
• If the closure is async but the returned value is not awaited, wrap the block in a Future(() async {...})

For sync*/async* functions, we could stick to considering them unsupported for now.

Wrapping in Future(()) should be fine for return values that are not awaited.

Callbacks may be function tear-offs

Your proposed solution looks fine.

Other Notes

As far as the way to offer the migration to the users, I just want to point out that when we used https://pub.dev/packages/codemod for riverpod 1.0 or 0.14.0?, we ran into lots of issues with overlapping edits due to nesting patterns. This causes the whole migration to fail unfortunately.

Either

• The edits need to be done and applied to the files one at a time (which means you have to re-analyze the files that have changed after each edit)
• You need to compute the subset of changes that are non overlapping and then apply all of those changes and iterate the migration from there until there are no more edits
• You need to manage edit buffers and string manipulation for overlapping edits yourself - easy to mess up something

Or you just do it via a lint / IDE refactoring which is probably most reliable for users. (This would also make it easy for users to alter the code and make small updates while they are reviewing it). However, due to the pervasive nature of .map and .when when using freezed, it is likely a long conversion process especially with larger codebases.

Although there is some pushback on deprecating it (which I was initially opposed to as well), I do think that there is a large benefit to switching code to use dart's pattern matching -> especially when considering nested matches, more flexible orElse logic for dealing with union classes that have a common parent interface etc. That's before even mentioning the smaller generated code size. I think the option of generating the methods (not by default) should stick around at least till a macro version of freezed. The biggest thing that I would say about this is to make it very clear on the README that this has happened. Not everyone follows you on Twitter / GitHub, also not everyone looks at CHANGELOGs when running pub upgrade (even though they should).

from freezed.

Actually there is

switch(doSomething()) {
  case ExampleData obj: print(obj.data);
// or
  case final ExampleData obj: print(obj.data);
// or
  case ExampleData() && final obj: print(obj.data)
  ...
)

Great!

Just use the name in the map expression, or am I missing something?

You're right. I was thinking that this could shadow existing variables. But when thinking about it that's what map does too anyway.

In fact the opposite case could be made. That the refactor could stop shadowing variables if we don't do this. So if someone does:

int value;
obj.whem(
  data: (value) {
    print(value++);
  }
)

Then doing:

int value;
switch (obj) {
  case ExampleData(data: final value):
    print(value++);
}

would not be equivalent. As the ++ is now changing a different variable.

About migrating non-sealed classes, I was thinking that:
If there's one (and only one) case where there's no used property, we could use that as last case.

An example is Riverpod's AsyncValue with its loading state;

It's not sealed quite yet, but folks can do:

switch (AsyncValue()) {
  AsyncData() =>...,
  AsyncError() => ...,
  _ => print('loading'),
}

from freezed.

Sometimes the factory type is not public
In the example, both ExampleError/ExampleData are public. But they could be private, and we could be in a file that does not have access to the private type.

In that scenario, we cannot generate the when/map equivalent. We could have a warning asking folks to make those objects public.

one thing to consider about private subtypes. It can actually be quite common case since it allows to avoid name conflicts as well as simplifies class names. Forcing devs to switch completely to pattern matching can make transition quite painful

from freezed.

The migration probably should look for name conflicts. And if so, use import aliasing to fix it.

from freezed.