Skip to content

Commit

Permalink
lib/proto: Allow replacing repeated fields, instead of appending. (go…
Browse files Browse the repository at this point in the history
…ogle#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 <[email protected]>
  • Loading branch information
WojciechP and Wojciech Ptak authored Dec 2, 2022
1 parent 3d7c6cd commit 42872f4
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/proto/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions starlark/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions starlark/testdata/proto.star
Original file line number Diff line number Diff line change
@@ -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"])

0 comments on commit 42872f4

Please sign in to comment.