From 42872f4d8faac131980be5f3bc9505851d863fff Mon Sep 17 00:00:00 2001
From: Wojciech Ptak <wojtek.ptak@gmail.com>
Date: Fri, 2 Dec 2022 15:30:44 +0100
Subject: [PATCH] lib/proto: Allow replacing repeated fields, instead of
 appending. (#440)

* lib/proto: Allow replacing repeated fields, instead of appending.

Consider the following Stralark line:

        msg.repeated = [1,2,3]

Without this commit, that line appends three elements to the previous existing
repeated values. With this commit, that line replaces the previous repeated
values with new three items.

I believe the old semantics is simply a bug, and that most users would
expect an assignment to overwrite the previous value.

* Add a small test for lib/proto

* Use list.Truncate(0) instead of msg.Clear

Co-authored-by: Wojciech Ptak <wojciechp@google.com>
---
 lib/proto/proto.go           |  1 +
 starlark/eval_test.go        |  9 +++++++++
 starlark/testdata/proto.star | 14 ++++++++++++++
 3 files changed, 24 insertions(+)
 create mode 100644 starlark/testdata/proto.star

diff --git a/lib/proto/proto.go b/lib/proto/proto.go
index 149162db..3ad37916 100644
--- a/lib/proto/proto.go
+++ b/lib/proto/proto.go
@@ -397,6 +397,7 @@ func setField(msg protoreflect.Message, fdesc protoreflect.FieldDescriptor, valu
 
 		// TODO(adonovan): handle maps
 		list := msg.Mutable(fdesc).List()
+		list.Truncate(0)
 		var x starlark.Value
 		for i := 0; iter.Next(&x); i++ {
 			v, err := toProto(fdesc, x)
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index 53786158..9ffd1794 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -18,12 +18,16 @@ import (
 	"go.starlark.net/internal/chunkedfile"
 	"go.starlark.net/lib/json"
 	starlarkmath "go.starlark.net/lib/math"
+	"go.starlark.net/lib/proto"
 	"go.starlark.net/lib/time"
 	"go.starlark.net/resolve"
 	"go.starlark.net/starlark"
 	"go.starlark.net/starlarkstruct"
 	"go.starlark.net/starlarktest"
 	"go.starlark.net/syntax"
+	"google.golang.org/protobuf/reflect/protoregistry"
+
+	_ "google.golang.org/protobuf/types/descriptorpb" // example descriptor needed for lib/proto tests
 )
 
 // A test may enable non-standard options by containing (e.g.) "option:recursion".
@@ -113,6 +117,7 @@ func TestExecFile(t *testing.T) {
 	testdata := starlarktest.DataFile("starlark", ".")
 	thread := &starlark.Thread{Load: load}
 	starlarktest.SetReporter(thread, t)
+	proto.SetPool(thread, protoregistry.GlobalFiles)
 	for _, file := range []string{
 		"testdata/assign.star",
 		"testdata/bool.star",
@@ -127,6 +132,7 @@ func TestExecFile(t *testing.T) {
 		"testdata/list.star",
 		"testdata/math.star",
 		"testdata/misc.star",
+		"testdata/proto.star",
 		"testdata/set.star",
 		"testdata/string.star",
 		"testdata/time.star",
@@ -202,6 +208,9 @@ func load(thread *starlark.Thread, module string) (starlark.StringDict, error) {
 	if module == "math.star" {
 		return starlark.StringDict{"math": starlarkmath.Module}, nil
 	}
+	if module == "proto.star" {
+		return starlark.StringDict{"proto": proto.Module}, nil
+	}
 
 	// TODO(adonovan): test load() using this execution path.
 	filename := filepath.Join(filepath.Dir(thread.CallFrame(0).Pos.Filename()), module)
diff --git a/starlark/testdata/proto.star b/starlark/testdata/proto.star
new file mode 100644
index 00000000..f1400820
--- /dev/null
+++ b/starlark/testdata/proto.star
@@ -0,0 +1,14 @@
+# Tests of the experimental 'lib/proto' module.
+
+load("assert.star", "assert")
+load("proto.star", "proto")
+
+schema = proto.file("google/protobuf/descriptor.proto")
+
+m = schema.FileDescriptorProto(name = "somename.proto", dependency = ["a", "b", "c"])
+assert.eq(type(m), "proto.Message")
+assert.eq(m.name, "somename.proto")
+assert.eq(list(m.dependency), ["a", "b", "c"])
+m.dependency = ["d", "e"]
+assert.eq(list(m.dependency), ["d", "e"])
+