Support explicitly setting proto field tags.

Credits to @jasonewang for posting the first implementation at #339. I've
modified this a bit to read the extension values from the new
openconfig-codegen-extensions.yang model that's now in
openconfig/public, and also to address some of the corner cases that
were discussed in the PR.

Spec:
https://github.com/openconfig/ygot/blob/master/docs/yang-to-protobuf-transformations-spec.md#field-numbering

Corner cases not mentioned in the spec:
1. multiple `field-number` extensions.
  - Error out.
1. multiple `field-number-offset` extensions.
  - Add them up.
1. `field-number` or `field-number-offset` extensions on non-leaves.
  - Silently ignore.
1. `field-number-offset` on non-uses statements, including leafs.
  - Allow.
1. Conflicts.
  - Allow. The proto compilation will of course fail. Automated checking can be added here if we'd prefer.

I made some simple choices for the above items. Erroring out for 3 and 4 would
likely require changing `goyang` to specifically support checking these cases,
which I'm not sure is worth it here.

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/google/go-cmp v0.5.5
 	github.com/kylelemons/godebug v1.1.0
 	github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be
-	github.com/openconfig/goyang v0.2.5
+	github.com/openconfig/goyang v0.2.6
 	github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f
 	github.com/pmezard/go-difflib v1.0.0
 	google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d

go.sum

@@ -44,8 +44,8 @@ github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be h1:VEK8utxoyZu/hkp
 github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be/go.mod h1:M/EcuapNQgvzxo1DDXHK4tx3QpYM/uG4l591v33jG2A=
 github.com/openconfig/goyang v0.0.0-20200115183954-d0a48929f0ea/go.mod h1:dhXaV0JgHJzdrHi2l+w0fZrwArtXL7jEFoiqLEdmkvU=
 github.com/openconfig/goyang v0.2.2/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8=
-github.com/openconfig/goyang v0.2.5 h1:ZvV+5cF5thPFun1H6/Itt/Jbwb/bKmjS0o8imN+7ddw=
-github.com/openconfig/goyang v0.2.5/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8=
+github.com/openconfig/goyang v0.2.6 h1:uJlr2bOUe/9mTtnIXgWG9VBqZd/BeCKur4+wAU2jJPc=
+github.com/openconfig/goyang v0.2.6/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8=
 github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f h1:8vRtC+y0xh9BYPrEGf/jG/paYXiDUJ6P8iYt5rCVols=
 github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f/go.mod h1:OoH46A2kV42cIXGyviYmAlGmn6cHjGduyC2+I9d/iVs=
 github.com/openconfig/ygot v0.6.0/go.mod h1:o30svNf7O0xK+R35tlx95odkDmZWS9JyWWQSmIhqwAs=

ygen/codegen_test.go

@@ -1967,8 +1967,11 @@ func TestGenerateProto3(t *testing.T) {
 		wantOutputFiles map[string]string
 		wantErr         bool
 	}{{
-		name:    "simple protobuf test with compression",
-		inFiles: []string{filepath.Join(TestRoot, "testdata", "proto", "proto-test-a.yang")},
+		name: "simple protobuf test with compression",
+		inFiles: []string{
+			filepath.Join(TestRoot, "testdata", "proto", "proto-test-a.yang"),
+			filepath.Join(TestRoot, "testdata", "proto", "openconfig-codegen-extensions.yang"),
+		},
 		inConfig: GeneratorConfig{
 			TransformationOptions: TransformationOpts{
 				CompressBehaviour: genutil.PreferIntendedConfig,

ygen/protogen.go

@@ -93,6 +93,13 @@ const (
 	protoMatchingListNameKeySuffix = "key"
 )
 
+// Names from https://github.com/openconfig/public/blob/master/release/models/extensions/openconfig-codegen-extensions.yang
+const (
+	codegenExtModuleName = "openconfig-codegen-extensions"
+	fieldNumExtName      = "field-number"
+	offsetExtName        = "field-number-offset"
+)
+
 // protoMsgField describes a field of a protobuf message.
 // Note, throughout this package private structs that have public fields are used
 // in text/template which cannot refer to unexported fields.
@@ -1048,40 +1055,44 @@ func safeProtoIdentifierName(name string) string {
 }
 
 // protoTagForEntry returns a protobuf tag value for the entry e.
+// If the entry has user-specified field numberings, then use that instead:
+// https://github.com/openconfig/ygot/blob/master/docs/yang-to-protobuf-transformations-spec.md#field-numbering
 func protoTagForEntry(e *yang.Entry) (uint32, error) {
-	var offset uint32
-	for _, ext := range e.Exts {
-		if ext.Keyword == "occodegenext:field-number-offset" {
-			off, err := strconv.ParseUint(ext.Argument, 10, 32)
-			if err != nil {
-				return 0, fmt.Errorf("could not parse %s %s: %s", ext.Keyword, ext.Argument, err)
-			}
-			offset = uint32(off)
-			if offset == 0 {
-				return 0, fmt.Errorf("%s cannot be 0", ext.Keyword)
-			}
-			break
+	var fieldNum uint32
+	exts, err := yang.MatchingEntryExtensions(e, codegenExtModuleName, fieldNumExtName)
+	if err != nil {
+		return 0, err
+	}
+	switch len(exts) {
+	case 0:
+		return fieldTag(e.Path())
+	case 1:
+		num, err := strconv.ParseUint(exts[0].Argument, 10, 32)
+		if err != nil || num == 0 {
+			return 0, fmt.Errorf("could not parse %s:%s %q as a non-zero integer: %s", codegenExtModuleName, fieldNumExtName, exts[0].Argument, err)
 		}
+		fieldNum = uint32(num)
+	default:
+		return 0, fmt.Errorf("more than one %s:%s defined", codegenExtModuleName, fieldNumExtName)
 	}
-	for _, ext := range e.Exts {
-		if ext.Keyword == "occodegenext:field-number" {
-			fn, err := strconv.ParseUint(ext.Argument, 10, 32)
-			if err != nil {
-				return 0, fmt.Errorf("could not parse %s %s: %s", ext.Keyword, ext.Argument, err)
-			}
-			if fn == 0 {
-				return 0, fmt.Errorf("%s cannot be 0", ext.Keyword)
-			}
-			annotatedFieldNumber := uint32(fn) + offset
-			if annotatedFieldNumber > 1000 || annotatedFieldNumber < 1 {
-				return 0, fmt.Errorf("%s field number %d not in annotation reserved range of 1-1000",
-					e.Name, annotatedFieldNumber)
-			}
-			return annotatedFieldNumber, nil
+
+	exts, err = yang.MatchingEntryExtensions(e, codegenExtModuleName, offsetExtName)
+	if err != nil {
+		return 0, err
+	}
+	for _, ext := range exts {
+		num, err := strconv.ParseUint(ext.Argument, 10, 32)
+		if err != nil {
+			return 0, fmt.Errorf("could not parse %s:%s %q as an uint: %s", codegenExtModuleName, offsetExtName, ext.Argument, err)
 		}
+		// Allow for multiple offsets to accumulate.
+		fieldNum += uint32(num)
 	}
 
-	return fieldTag(e.Path())
+	if fieldNum > 1000 || fieldNum < 1 {
+		return 0, fmt.Errorf("%s: %d not in reserved range of 1-1000 for %s:%s", e.Path(), fieldNum, codegenExtModuleName, offsetExtName)
+	}
+	return fieldNum, nil
 }
 
 // fieldTag takes an input string and calculates a FNV hash for the value. If the

ygen/protogen_test.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/kylelemons/godebug/pretty"
+	"github.com/openconfig/gnmi/errdiff"
 	"github.com/openconfig/goyang/pkg/yang"
 	"github.com/openconfig/ygot/testutil"
 )
@@ -63,7 +64,7 @@ func TestGenProto3Msg(t *testing.T) {
 		inParentPackage        string
 		inChildMsgs            []*generatedProto3Message
 		wantMsgs               map[string]*protoMsg
-		wantErr                bool
+		wantErrSubstring       string
 	}{{
 		name: "simple message with only scalar fields",
 		inMsg: &Directory{
@@ -539,7 +540,7 @@ func TestGenProto3Msg(t *testing.T) {
 				},
 			},
 		},
-		wantErr: true,
+		wantErrSubstring: "could not resolve",
 	}, {
 		name: "message with an unimplemented mapping",
 		inMsg: &Directory{
@@ -565,7 +566,7 @@ func TestGenProto3Msg(t *testing.T) {
 			},
 			Path: []string{"", "messasge-with-invalid-contents", "unimplemented"},
 		},
-		wantErr: true,
+		wantErrSubstring: "unimplemented type",
 	}, {
 		name: "message with any anydata field",
 		inMsg: &Directory{
@@ -673,6 +674,26 @@ func TestGenProto3Msg(t *testing.T) {
 							Argument: "100",
 						},
 					},
+					Node: &yang.Leaf{
+						Name: "field-one",
+						Parent: &yang.Container{
+							Name: "message-name",
+							Parent: &yang.Module{
+								Name: "module",
+								Prefix: &yang.Value{
+									Name: "foo",
+								},
+								Import: []*yang.Import{{
+									Prefix: &yang.Value{
+										Name: "occodegenext",
+									},
+									Module: &yang.Module{
+										Name: codegenExtModuleName,
+									},
+								}},
+							},
+						},
+					},
 				},
 				"field-two": {
 					Name: "field-two",
@@ -683,6 +704,26 @@ func TestGenProto3Msg(t *testing.T) {
 							Argument: "1",
 						},
 					},
+					Node: &yang.Leaf{
+						Name: "field-two",
+						Parent: &yang.Container{
+							Name: "message-name",
+							Parent: &yang.Module{
+								Name: "module",
+								Prefix: &yang.Value{
+									Name: "foo",
+								},
+								Import: []*yang.Import{{
+									Prefix: &yang.Value{
+										Name: "occodegenext",
+									},
+									Module: &yang.Module{
+										Name: codegenExtModuleName,
+									},
+								}},
+							},
+						},
+					},
 				},
 				"field-three": {
 					Name: "field-three",
@@ -697,6 +738,64 @@ func TestGenProto3Msg(t *testing.T) {
 							Argument: "100",
 						},
 					},
+					Node: &yang.Leaf{
+						Name: "field-three",
+						Parent: &yang.Container{
+							Name: "message-name",
+							Parent: &yang.Module{
+								Name: "module",
+								Prefix: &yang.Value{
+									Name: "foo",
+								},
+								Import: []*yang.Import{{
+									Prefix: &yang.Value{
+										Name: "occodegenext",
+									},
+									Module: &yang.Module{
+										Name: codegenExtModuleName,
+									},
+								}},
+							},
+						},
+					},
+				},
+				"field-four": {
+					Name: "field-four",
+					Type: &yang.YangType{Kind: yang.Yint8},
+					Exts: []*yang.Statement{
+						{
+							Keyword:  "occodegenext:field-number",
+							Argument: "1",
+						},
+						{
+							Keyword:  "occodegenext:field-number-offset",
+							Argument: "100",
+						},
+						{
+							Keyword:  "occodegenext:field-number-offset",
+							Argument: "200",
+						},
+					},
+					Node: &yang.Leaf{
+						Name: "field-four",
+						Parent: &yang.Container{
+							Name: "message-name",
+							Parent: &yang.Module{
+								Name: "module",
+								Prefix: &yang.Value{
+									Name: "foo",
+								},
+								Import: []*yang.Import{{
+									Prefix: &yang.Value{
+										Name: "occodegenext",
+									},
+									Module: &yang.Module{
+										Name: codegenExtModuleName,
+									},
+								}},
+							},
+						},
+					},
 				},
 			},
 			Path: []string{"", "root", "message-name"},
@@ -719,6 +818,10 @@ func TestGenProto3Msg(t *testing.T) {
 					Tag:  101,
 					Name: "field_three",
 					Type: "ywrapper.IntValue",
+				}, {
+					Tag:  301,
+					Name: "field_four",
+					Type: "ywrapper.IntValue",
 				}},
 			},
 		},
@@ -745,11 +848,79 @@ func TestGenProto3Msg(t *testing.T) {
 							Argument: "1000",
 						},
 					},
+					Node: &yang.Leaf{
+						Name: "field-one",
+						Parent: &yang.Container{
+							Name: "message-name",
+							Parent: &yang.Module{
+								Name: "module",
+								Prefix: &yang.Value{
+									Name: "foo",
+								},
+								Import: []*yang.Import{{
+									Prefix: &yang.Value{
+										Name: "occodegenext",
+									},
+									Module: &yang.Module{
+										Name: codegenExtModuleName,
+									},
+								}},
+							},
+						},
+					},
 				},
 			},
 			Path: []string{"", "root", "message-name"},
 		},
-		wantErr: true,
+		wantErrSubstring: "not in reserved range",
+	}, {
+		name: "message with duplicate field numbers",
+		inMsg: &Directory{
+			Name: "MessageName",
+			Entry: &yang.Entry{
+				Name: "message-name",
+				Dir:  map[string]*yang.Entry{},
+				Kind: yang.DirectoryEntry,
+			},
+			Fields: map[string]*yang.Entry{
+				"field-one": {
+					Name: "field-one",
+					Type: &yang.YangType{Kind: yang.Ystring},
+					Exts: []*yang.Statement{
+						{
+							Keyword:  "occodegenext:field-number",
+							Argument: "1",
+						},
+						{
+							Keyword:  "occodegenext:field-number",
+							Argument: "1000",
+						},
+					},
+					Node: &yang.Leaf{
+						Name: "field-one",
+						Parent: &yang.Container{
+							Name: "message-name",
+							Parent: &yang.Module{
+								Name: "module",
+								Prefix: &yang.Value{
+									Name: "foo",
+								},
+								Import: []*yang.Import{{
+									Prefix: &yang.Value{
+										Name: "occodegenext",
+									},
+									Module: &yang.Module{
+										Name: codegenExtModuleName,
+									},
+								}},
+							},
+						},
+					},
+				},
+			},
+			Path: []string{"", "root", "message-name"},
+		},
+		wantErrSubstring: "more than one",
 	}}
 
 	for _, tt := range tests {
@@ -771,11 +942,20 @@ func TestGenProto3Msg(t *testing.T) {
 				annotateSchemaPaths: tt.inAnnotateSchemaPaths,
 			}, tt.inParentPackage, tt.inChildMsgs, true, true)
 
-			if (errs != nil) != tt.wantErr {
-				t.Errorf("s: genProtoMsg(%#v, %#v, *genState, %v, %v, %s, %s): did not get expected error status, got: %v, wanted err: %v", tt.name, tt.inMsg, tt.inMsgs, tt.inCompressPaths, tt.inBasePackage, tt.inEnumPackage, errs, tt.wantErr)
+			var err error
+			switch len(errs) {
+			case 1:
+				err = errs[0]
+				fallthrough
+			case 0:
+				if diff := errdiff.Substring(err, tt.wantErrSubstring); diff != "" {
+					t.Fatalf("s: genProtoMsg(%#v, %#v, *genState, %v, %v, %s, %s): did not get expected error: %s", tt.name, tt.inMsg, tt.inMsgs, tt.inCompressPaths, tt.inBasePackage, tt.inEnumPackage, diff)
+				}
+			default:
+				t.Fatalf("got more than 1 error: %v", errs)
 			}
 
-			if tt.wantErr {
+			if tt.wantErrSubstring != "" {
 				return
 			}