Skip to content

Commit

Permalink
Fix field names that collide with generated names capnproto#46
Browse files Browse the repository at this point in the history
This is a partial fix of capnproto#46

> zenhack commented on Sep 17, 2016:
>
> if a name would otherwise collide with a go or go-capnpc name, append
> a single underscore. e.g. `Struct` becomes `Struct_`

Also see capnproto/capnproto#323

This commit changes the generated getter and setter methods for
struct fields. If the field name would collide with code generated
for any struct such as `Method()` or `String()`, the field getter
becomes `Method_()` and the setter becomes `SetMethod_()`.

Technically only `Method_()` would be necessary, but who wants to
maintain the mental state? If the getter changes, the setter also
changing is just simpler conceptually.

The capnp files in this repo under `std` have $Go.name() annotations
due to capnproto#46. This removes
one of them (std/capnp/compat/json.capnp) and updates the unit tests
for code coverage testing purposes.
  • Loading branch information
davidhubbard committed Aug 8, 2023
1 parent 93062cf commit 25f4251
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 88 deletions.
40 changes: 24 additions & 16 deletions capnpc-go/capnpc-go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,40 +310,39 @@ func TestDefineFile(t *testing.T) {
structStrings: true,
}
tests := []struct {
fileID uint64
fname string
opts genoptions
}{
{0x832bcc6686a26d56, "aircraft.capnp.out", defaultOptions},
{0x832bcc6686a26d56, "aircraft.capnp.out", genoptions{
{"aircraft.capnp.out", defaultOptions},
{"aircraft.capnp.out", genoptions{
promises: false,
schemas: false,
structStrings: false,
}},
{0x832bcc6686a26d56, "aircraft.capnp.out", genoptions{
{"aircraft.capnp.out", genoptions{
promises: true,
schemas: false,
structStrings: false,
}},
{0x832bcc6686a26d56, "aircraft.capnp.out", genoptions{
{"aircraft.capnp.out", genoptions{
promises: false,
schemas: true,
structStrings: false,
}},
{0x832bcc6686a26d56, "aircraft.capnp.out", genoptions{
{"aircraft.capnp.out", genoptions{
promises: true,
schemas: true,
structStrings: false,
}},
{0x832bcc6686a26d56, "aircraft.capnp.out", genoptions{
{"aircraft.capnp.out", genoptions{
promises: false,
schemas: true,
structStrings: true,
}},
{0x83c2b5818e83ab19, "group.capnp.out", defaultOptions},
{0xb312981b2552a250, "rpc.capnp.out", defaultOptions},
{0xd68755941d99d05e, "scopes.capnp.out", defaultOptions},
{0xecd50d792c3d9992, "util.capnp.out", defaultOptions},
{"group.capnp.out", defaultOptions},
{"rpc.capnp.out", defaultOptions},
{"scopes.capnp.out", defaultOptions},
{"util.capnp.out", defaultOptions},
}
for _, test := range tests {
data, err := readTestFile(test.fname)
Expand All @@ -361,12 +360,21 @@ func TestDefineFile(t *testing.T) {
t.Errorf("Reading code generator request %s: %v", test.fname, err)
continue
}
reqFiles, err := req.RequestedFiles()
if err != nil {
t.Errorf("Reading code generator request %q: RequestedFiles: %v", test.fname, err)
continue
}
if reqFiles.Len() < 1 {
t.Errorf("Reading code generator request %q: %d RequestedFiles", test.fname, reqFiles.Len())
continue
}
nodes, err := buildNodeMap(req)
if err != nil {
t.Errorf("buildNodeMap %s: %v", test.fname, err)
continue
}
g := newGenerator(test.fileID, nodes, test.opts)
g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
if err := g.defineFile(); err != nil {
t.Errorf("defineFile %s %+v: %v", test.fname, test.opts, err)
continue
Expand All @@ -379,7 +387,7 @@ func TestDefineFile(t *testing.T) {

// Generation should be deterministic between runs.
for i := 0; i < iterations-1; i++ {
g := newGenerator(test.fileID, nodes, test.opts)
g := newGenerator(reqFiles.At(0).Id(), nodes, test.opts)
if err := g.defineFile(); err != nil {
t.Errorf("defineFile %s %+v [iteration %d]: %v", test.fname, test.opts, i+2, err)
continue
Expand Down Expand Up @@ -525,9 +533,9 @@ func TestPersistent(t *testing.T) {
t.Parallel()

defaultOptions := genoptions{
promises: false,
schemas: false,
structStrings: false,
promises: true,
schemas: true,
structStrings: true,
}
tests := []struct {
fname string
Expand Down
19 changes: 17 additions & 2 deletions capnpc-go/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ import (
"capnproto.org/go/capnp/v3/std/capnp/stream"
)

// These renames only apply to the codegen for struct fields.
var renameIdents = map[string]bool {
"IsValid": true, // This is not a complete list.
"Segment": true, // E.g. "ToPtr", "SetNull" are too
"String": true, // unusual to burden codegen with.
"Message": true,
"Which": true,
}

type node struct {
schema.Node
pkg string
Expand All @@ -26,8 +35,14 @@ func (n *node) codeOrderFields() []field {
f := fields.At(i)
fann, _ := f.Annotations()
fname, _ := f.Name()
fname = parseAnnotations(fann).Rename(fname)
mbrs[f.CodeOrder()] = field{Field: f, Name: fname}
var renamed = parseAnnotations(fann).Rename(fname)
if renamed == fname { // Avoid collisions if no annotation
if _, ok := renameIdents[strings.Title(fname)]; ok {
renamed = fname + "_"
}

}
mbrs[f.CodeOrder()] = field{Field: f, Name: renamed}
}
return mbrs
}
Expand Down
4 changes: 2 additions & 2 deletions capnpc-go/testdata/persistent-simple.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ interface Persistent {

annotation persistent(interface, field) :Void;

struct ThisStructOnlyNeededToGetImportsToWork {
value @0 :Int64;
struct FieldNameThatIsInvalidInGolang {
string @0 :Text;
}
Binary file modified capnpc-go/testdata/persistent-simple.capnp.out
Binary file not shown.
2 changes: 1 addition & 1 deletion std/capnp/compat/json.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ struct Value {
null @0 :Void;
boolean @1 :Bool;
number @2 :Float64;
string @3 :Text $Go.name("string_");
string @3 :Text;
array @4 :List(Value);
object @5 :List(Field);
# Standard JSON values.
Expand Down
110 changes: 54 additions & 56 deletions std/capnp/compat/json/json.capnp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 0 additions & 11 deletions std/fixups.patch
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
--- a/capnp/compat/json.capnp 2021-10-21 14:59:51.244324279 -0400
+++ b/capnp/compat/json.capnp 2021-10-21 15:01:09.543760012 -0400
@@ -28,7 +28,7 @@
null @0 :Void;
boolean @1 :Bool;
number @2 :Float64;
- string @3 :Text;
+ string @3 :Text $Go.name("string_");
array @4 :List(Value);
object @5 :List(Field);
# Standard JSON values.
--- a/capnp/schema.capnp 2021-10-21 15:03:20.221774538 -0400
+++ b/capnp/schema.capnp 2021-10-21 15:04:20.870019327 -0400
@@ -82,7 +82,7 @@
Expand Down

0 comments on commit 25f4251

Please sign in to comment.