Fix field names that collide with generated names #46 (#541)

This is a partial fix of #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 #46. This removes
one of them (std/capnp/compat/json.capnp) and updates the unit tests
for code coverage testing purposes.

capnpc-go/capnpc-go_test.go

@@ -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)
@@ -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
@@ -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
@@ -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

capnpc-go/nodes.go

@@ -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
@@ -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
 }

capnpc-go/testdata/persistent-simple.capnp

@@ -31,6 +31,6 @@ interface Persistent {
 
 annotation persistent(interface, field) :Void;
 
-struct ThisStructOnlyNeededToGetImportsToWork {
-  value @0 :Int64;
+struct FieldNameThatIsInvalidInGolang {
+  string @0 :Text;
 }

std/capnp/compat/json.capnp

@@ -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.

std/fixups.patch

@@ -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 @@