Provide analyzer support for verifying `copyWith`-style implementations.

[gspencergoog]

In the absence of a good way to copy immutable objects with some field changes, the Flutter team often implements a copyWith function that copies the original object with optional overrides of individual fields. It's invaluable for being able to work with immutable classes.

However, in the implementation, it's really easy to fat finger something and not hook it up correctly, resulting in possible catastrophic bugs. We try to write unit tests to handle this, but sometimes even then the test can miss it if we choose the wrong test.

For instance, an example implementation looks like this:

  @override
  StarBorder copyWith({
    BorderSide? side,
    double? points,
    double? innerRadiusRatio,
    double? pointRounding,
    double? valleyRounding,
    double? rotation,
    double? squash,
  }) {
    return StarBorder(
      side: side ?? this.side,
      points: points ?? this.points,
      rotation: rotation ?? this.rotation,
      innerRadiusRatio: innerRadiusRatio ?? this.innerRadiusRatio,
      pointRounding: valleyRounding ?? this.pointRounding,
      valleyRounding: valleyRounding ?? this.valleyRounding,
      squash: squash ?? this.squash,
    );
  }

Can you spot the error? If you're looking for it, it's pretty obvious, but it is easy to miss in a review. (pointRounding is being assigned the value of valleyRounding).

The unnecessary_this analysis warning can sometimes help here if you've renamed a field but not the name of the parameter in the copyWith (another common error), but it doesn't catch the case above.

Ideally there'd be a general lint that could check all instances of a copyWith for these kinds of errors, but I realize that the variation in implementations probably makes that impossible.

Are there any good ways to improve the situation? Could we have an annotation that a parameter must be used in the implementation? Could we annotate these kinds of copying functions and force them to conform to a pattern (must accept a parameter with the same name as all public fields, must use all parameters in result, target parameters must match source parameters, etc.)? Could we have a way of auto-generating these copy functions instead of writing them?

I mean, it seems like the best solution is a language solution that lets us describe how to copy an immutable class with changes, but I don't know what that is, and it seems like it might be hard to come up with a good general solution. In the meantime, it would be good to be able to avoid these common errors.

[keertip]

/cc @bwilkerson

[bwilkerson]

Could we have an annotation that a parameter must be used in the implementation?

That's certainly feasible, though it seems like a lot of work to annotate every parameter.

Could we annotate these kinds of copying functions and force them to conform to a pattern (must accept a parameter with the same name as all public fields, must use all parameters in result, target parameters must match source parameters, etc.)?

That's also doable, but so is a lint and the lint doesn't need to be added to every method. If a user doesn't like the way the lint works they don't have to enable it (though we might want to give it a name making it clear that it follow's Flutter's conventions for such a method and not necessarily everyone's conventions).

Could we have a way of auto-generating these copy functions instead of writing them?

Yes. There are at least three code generators on the first page of results using the query https://pub.dev/packages?q=generate+copyWith that purport to generate a copyWith method.

I don't know what the technical issues would be with Flutter using a code generator, so I don't know whether that solution is feasible.

I mean, it seems like the best solution is a language solution ...

Generating a copyWith method is one of the motivating use cases for the macros feature: https://github.com/dart-lang/language/blob/eba8f41b2f06b229c59216c2e37aa3050aa46645/working/macros/motivation.md#L173.

It's also being discussed as a possibility for some other language features, such as https://github.com/dart-lang/language/blob/eb9a58d8a6b4204f15cfe5bd044b60897305b6fe/working/extension_structs/overview.md.

But you're right, neither of these will be supported anytime soon, even if they're approved at some future date.

[bwilkerson]

Sorry that this has languished.

This sounds like a great example of a place where a macro could solve the problem. Just add a @CopyWIth macro to the class and let it generate the (presumably) error-free implementation. And a macro sounds like a better solution than a lint because it would require less work on the developer's part.