Makes rebuild take a void function.

[lrhn]

It's not very clear whether a value returned by the updates parameter to rebuild means anything.
All examples use => notation which returns the builder. The return type is currently dynamic, which suggests that the value is used for something, potentially something which cannot even be typed.

Marking the parameter as a void returning function makes it clear that you should not worry about the return value.

[dave26199]

Yes, this is an improvement :) ... originally I left off 'void' to make the code more concise. That was in the Dart 1 days when it didn't make much difference :)

Is this a breaking change, or should it still work if someone passes a function that returns something? Breaking changes to this library cause quite a lot of churn. I'll check it in google3, anyway.

Thanks :)

[lrhn]

A void return type is a top type, so any other return type is a subtype. It should accept any function.

The only issue could be non-=> function literals which do return something non-void/dynamic/Null with a return statement, and which now gets warnings about returning values from a void function (because the function literal's return type is now inferred to be void from the context type), like:

o.rebuild((b) {
  b.a = 42;
  b.b = 37;
  return 42;
});

Here the return 42; would now be an error because it's an explicit non-null/dynamic/void return from a void function.
You'd have to go out of your way to get that problem, and arguably, that is error-worthy code.

That said, someone as confused as me, and having looked at example code which all uses =>, could easily have written:

o.rebuild((b) {
  b.a = 42;
  b.b = 37;
  return b;
});

just to be safe. And since it was accepted, they'd probably think it was the right thing to do.

(An => function gets a free pass on that error because people like to use (e) => singleExpression as shorthand for (e) { singleExpression; }. If only the formatter had kept the latter on one line, fewer people would likely have used =>.)

[kuhnroyal]

I agree, this is breaking but likely only hitting a very small percentage. Well worth it for better typing.

[willlockwood]

Any interest in picking this back up/merging? I just ran into a small case where it would've been nice to avoid the dynamic typing here!

[davidmorgan]

Thanks for pinging Will.

I'd have to release as a major version increase, i.e. built_value 9, I'm not sure that's worth it just for this change; but if I make a breaking release for other reasons I'll include this one.

Note to self: we actually cleaned up google3 for violations and apply this change as a patch; there were a nontrivial number of violations, so I expect there are violations in the wild, too; they were of the form (b) { ...; return ...; }