Add UseResult annotation to rebuild

[osa1]

This catches issues like #630 where we forgot to use return value of
rebuild. Since rebuild does not update in-place, not using the
return value is often a bug.

This adds meta as a dependency but it should not affect generated
code. We may also want to use meta in the future for things like:

• Annotating GeneratedMessage methods that are supposed to be used by
  the generated code with @protected.

• Annotating generated message classes with @sealed. This help with
  maintaining backwards compatibility when adding new members to
  generated messages or to GeneratedMessage.

[sigurdm]

LGTM

Seems there is a test that needs to be fixed.

Maybe @rakudrama has something to say about adding the dependency.

I think package:meta is so ubiquitous that package:protobuf can afford to depend on it.

[osa1]

Seems there is a test that needs to be fixed.

Hm.. I'm a bit surprised that this line generates a unused_result warning:

protobuf.dart/protoc_plugin/test/oneof_test.dart

Line 164 in 2043b14

var copy1 = foo.deepCopy().freeze().rebuild((_) {}) as Foo;

The result is clearly used. It's casted to Foo and then assigned to copy1. Is this an SDK bug?

So this generates a warning:

    var copy1 = foo.deepCopy().freeze().rebuild((_) {}) as Foo;
    expectOneofNotSet(copy1);

But this does not:

    var copy1 = foo.deepCopy().freeze().rebuild((_) {});
    expectOneofNotSet(copy1 as Foo);

[osa1]

Hm.. I'm a bit surprised that this line generates a unused_result warning:

Reported this as dart-lang/sdk#48879.

[osa1]

The issue with the warning described above is fixed in Dart main branch but it will be a while until it's released. I'm not sure whether to wait for the release with the fix before merging this or work around the issue.

[sigurdm]

The issue with the warning described above is fixed in Dart main branch but it will be a while until it's released. I'm not sure whether to wait for the release with the fix before merging this or work around the issue.

I think it is ok to add a // ignore: comment in the test.

[osa1]

@rakudrama any thoughts on adding meta as a dependency to protobuf?

[rakudrama]

@rakudrama any thoughts on adding meta as a dependency to protobuf?

I don't foresee any problems.

[rakudrama]

    var copy1 = foo.deepCopy().freeze().rebuild((_) {}) as Foo;
    expectOneofNotSet(copy1);

Side comment - rebuild is generic, so why do we need the cast?

[osa1]

Side comment - rebuild is generic, so why do we need the cast?

This is because GeneratedMessage.freeze's return type is GeneratedMessage:

protobuf.dart/protobuf/lib/src/protobuf/generated_message.dart

Lines 54 to 57 in 9c73821

	GeneratedMessage freeze() {
	_fieldSet._markReadOnly();
	return this;
	}

so the concrete type is lost after freeze.

It would be great if we could use a type like This to refer to the concrete type in a superclass. If we had that freeze could have This as return type, and calls to freeze would not upcast the receiver.

[osa1]

This revealed another SDK bug when updating a protobuf user to the current master branch: dart-lang/sdk#49023