Support leaf-list defaults using latest version of goyang. (#618)

* Support leaf-list defaults using latest version of goyang.

Note: I haven't released goyang yet since I want to release both repos
at the same time. This is to avoid a downstream issue:
openconfig/models-ci#59

- Update ygot to use the updated `Entry.Default []string` and its
  corresponding helpers for storing defaults for leaf-lists.
- Change some helper function such as `yangDefaultValueToGo` to return
  `string` instead of `*string`. There is no good reason for returning a
  pointer and makes the code less convenient.
- Factor out `generateGoDefaultValue` from `writeGoStruct`, and have it
  handle multiple default values.
- In protogen's logic to generate default enums, check whether an enum
  is not a leaf-list before setting a particular one as the default enum
  value.
  Note: I'm not sure whether this is the correct handling -- though it
  appears to me that having a default value doesn't make sense for
  leaf-lists as repeated fields.

Testing:
- Added integration tests for Go and proto generation.

* Update ygot to use hermetic module parsing change from goyang. (#619)

* Update to use goyang@v1.0.0

go.mod

@@ -8,9 +8,11 @@ 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.4.0
+	github.com/openconfig/goyang v1.0.0
 	github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f
 	github.com/pmezard/go-difflib v1.0.0
 	google.golang.org/grpc v1.37.0
 	google.golang.org/protobuf v1.26.0
 )
+
+replace github.com/openconfig/goyang => github.com/openconfig/goyang v0.4.1-0.20220121162048-032583e1d882

go.sum

@@ -42,10 +42,8 @@ github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+
 github.com/openconfig/gnmi v0.0.0-20200414194230-1597cc0f2600/go.mod h1:M/EcuapNQgvzxo1DDXHK4tx3QpYM/uG4l591v33jG2A=
 github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be h1:VEK8utxoyZu/hkpjLxvuBmK5yW3NmBo/v/Wu5VQAJVs=
 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.4.0 h1:e6oGwpXXirSzQa0tmvwgp4L8jc3RvfzqroJFyC5uZXE=
-github.com/openconfig/goyang v0.4.0/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8=
+github.com/openconfig/goyang v0.4.1-0.20220121162048-032583e1d882 h1:3sNGfOdpcvcQgDGprSRHaub2d3Qj8UWt31EGMi1H5Y4=
+github.com/openconfig/goyang v0.4.1-0.20220121162048-032583e1d882/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=

testdata/modules/openconfig-leaflist-default.yang

@@ -0,0 +1,51 @@
+module openconfig-leaflist-default {
+  yang-version "1.1";
+  prefix "ocs";
+  namespace "urn:ocs";
+  description
+    "A simple OpenConfig test module with leaflists.";
+
+  typedef string-with-default {
+    type string;
+  }
+
+  grouping parent-config {
+    leaf-list one { type string; }
+    leaf-list three {
+      type enumeration {
+        enum ONE;
+        enum TWO;
+      }
+      default ONE;
+      default TWO;
+    }
+    leaf-list four {
+      type binary;
+      default "abc0";
+    }
+  }
+
+  container parent {
+    description
+      "I am a parent container
+       that has 4 children.";
+    container child {
+      container config {
+        uses parent-config;
+      }
+      container state {
+        config false;
+        uses parent-config;
+        leaf-list two {
+          type string-with-default;
+          default "foo";
+          default "foo";
+          default "bar";
+          default "bar";
+          default "baz";
+          default "baz";
+        }
+      }
+    }
+  }
+}

ygen/codegen.go

@@ -826,21 +826,19 @@ func (cg *YANGCodeGenerator) GenerateProto3(yangFiles, includePaths []string) (*
 // generated code for the modules. If errors are returned during the Goyang
 // processing of the modules, these errors are returned.
 func processModules(yangFiles, includePaths []string, options yang.Options) ([]*yang.Entry, util.Errors) {
+	// Initialise the set of YANG modules within the Goyang parsing package.
+	moduleSet := yang.NewModules()
+	// Propagate the options for the YANG library through to the parsing
+	// code - this allows the calling binary to specify characteristics
+	// of the YANG in a manner that we are transparent to.
+	moduleSet.ParseOptions = options
 	// Append the includePaths to the Goyang path variable, this ensures
 	// that where a YANG module uses an 'include' statement to reference
 	// another module, then Goyang can find this module to process.
 	for _, path := range includePaths {
-		yang.AddPath(path)
+		moduleSet.AddPath(path)
 	}
 
-	// Propagate the options for the YANG library through to the parsing
-	// code - this allows the calling binary to specify characteristics
-	// of the YANG in a manner that we are transparent to.
-	yang.ParseOptions = options
-
-	// Initialise the set of YANG modules within the Goyang parsing package.
-	moduleSet := yang.NewModules()
-
 	var errs util.Errors
 	for _, name := range yangFiles {
 		errs = util.AppendErr(errs, moduleSet.Read(name))

ygen/codegen_test.go

@@ -503,6 +503,22 @@ func TestSimpleStructs(t *testing.T) {
 			},
 		},
 		wantStructsCodeFile: filepath.Join(TestRoot, "testdata/structs/openconfig-simple-no-compress.trimmed-enum.formatted-txt"),
+	}, {
+		name:    "OpenConfig leaf-list defaults test, with compression",
+		inFiles: []string{filepath.Join(datapath, "openconfig-leaflist-default.yang")},
+		inConfig: GeneratorConfig{
+			GoOptions: GoOpts{
+				GenerateSimpleUnions:    true,
+				GenerateLeafGetters:     true,
+				GeneratePopulateDefault: true,
+			},
+			TransformationOptions: TransformationOpts{
+				CompressBehaviour:                    genutil.PreferIntendedConfig,
+				ShortenEnumLeafNames:                 true,
+				UseDefiningModuleForTypedefEnumNames: true,
+			},
+		},
+		wantStructsCodeFile: filepath.Join(TestRoot, "testdata/structs/openconfig-leaflist-default.formatted-txt"),
 	}, {
 		name:    "OpenConfig schema test - with annotations",
 		inFiles: []string{filepath.Join(datapath, "openconfig-simple.yang")},
@@ -2102,6 +2118,7 @@ func TestGenerateProto3(t *testing.T) {
 			"openconfig.proto_test_c.entity":       filepath.Join(TestRoot, "testdata", "proto", "proto-test-c.proto-test-c.entity.formatted-txt"),
 			"openconfig.proto_test_c.elists":       filepath.Join(TestRoot, "testdata", "proto", "proto-test-c.proto-test-c.elists.formatted-txt"),
 			"openconfig.proto_test_c.elists.elist": filepath.Join(TestRoot, "testdata", "proto", "proto-test-c.proto-test-c.elists.elist.formatted-txt"),
+			"openconfig.enums":                     filepath.Join(TestRoot, "testdata", "proto", "proto-test-c.enums.formatted-txt"),
 		},
 	}, {
 		name:    "yang schema with identityref and enumerated typedef, compression off",

ygen/goelements.go

@@ -543,13 +543,13 @@ func (s *goGenState) goUnionSubTypes(subtype *yang.YangType, ctx *yang.Entry, cu
 // The skipEnumDedup argument specifies whether leaves of type enumeration that are
 // used more than once in the schema should share a common type. By default, a single
 // type for each leaf is created.
-func (s *goGenState) yangDefaultValueToGo(value string, args resolveTypeArgs, isSingletonUnion, compressOCPaths, skipEnumDedup, shortenEnumLeafNames, useDefiningModuleForTypedefEnumNames bool, enumOrgPrefixesToTrim []string) (*string, yang.TypeKind, error) {
+func (s *goGenState) yangDefaultValueToGo(value string, args resolveTypeArgs, isSingletonUnion, compressOCPaths, skipEnumDedup, shortenEnumLeafNames, useDefiningModuleForTypedefEnumNames bool, enumOrgPrefixesToTrim []string) (string, yang.TypeKind, error) {
 	// Handle the case of a typedef which is actually an enumeration.
 	mtype, err := s.enumSet.enumeratedTypedefTypeName(args, goEnumPrefix, false, useDefiningModuleForTypedefEnumNames)
 	if err != nil {
 		// err is non nil when this was a typedef which included
 		// an invalid enumerated type.
-		return nil, yang.Ynone, err
+		return "", yang.Ynone, err
 	}
 	if mtype != nil {
 		if strings.Contains(value, ":") {
@@ -558,11 +558,11 @@ func (s *goGenState) yangDefaultValueToGo(value string, args resolveTypeArgs, is
 		switch args.yangType.Kind {
 		case yang.Yenum:
 			if !args.yangType.Enum.IsDefined(value) {
-				return nil, yang.Ynone, fmt.Errorf("default value conversion: typedef enum value %q not found in enum with type name %q", value, args.yangType.Name)
+				return "", yang.Ynone, fmt.Errorf("default value conversion: typedef enum value %q not found in enum with type name %q", value, args.yangType.Name)
 			}
 		case yang.Yidentityref:
 			if !args.yangType.IdentityBase.IsDefined(value) {
-				return nil, yang.Ynone, fmt.Errorf("default value conversion: typedef identity value %q not found in enum with type name %q", value, args.yangType.Name)
+				return "", yang.Ynone, fmt.Errorf("default value conversion: typedef identity value %q not found in enum with type name %q", value, args.yangType.Name)
 			}
 		}
 		return enumDefaultValue(mtype.NativeType, value, goEnumPrefix), args.yangType.Kind, nil
@@ -577,122 +577,121 @@ func (s *goGenState) yangDefaultValueToGo(value string, args resolveTypeArgs, is
 	case yang.Yuint64, yang.Yuint32, yang.Yuint16, yang.Yuint8:
 		bits, err := util.YangIntTypeBits(ykind)
 		if err != nil {
-			return nil, yang.Ynone, err
+			return "", yang.Ynone, err
 		}
 		if signed {
 			val, err := strconv.ParseInt(value, 10, bits)
 			if err != nil {
-				return nil, yang.Ynone, fmt.Errorf("default value conversion: unable to convert default value %q to %v: %v", value, ykind, err)
+				return "", yang.Ynone, fmt.Errorf("default value conversion: unable to convert default value %q to %v: %v", value, ykind, err)
 			}
 			if err := ytypes.ValidateIntRestrictions(args.yangType, val); err != nil {
-				return nil, yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match int restrictions: %v", value, err)
+				return "", yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match int restrictions: %v", value, err)
 			}
 		} else {
 			val, err := strconv.ParseUint(value, 10, bits)
 			if err != nil {
-				return nil, yang.Ynone, fmt.Errorf("default value conversion: unable to convert default value %q to %v: %v", value, ykind, err)
+				return "", yang.Ynone, fmt.Errorf("default value conversion: unable to convert default value %q to %v: %v", value, ykind, err)
 			}
 			if err := ytypes.ValidateUintRestrictions(args.yangType, val); err != nil {
-				return nil, yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match int restrictions: %v", value, err)
+				return "", yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match int restrictions: %v", value, err)
 			}
 		}
-		return &value, ykind, nil
+		return value, ykind, nil
 	case yang.Ydecimal64:
 		val, err := strconv.ParseFloat(value, 64)
 		if err != nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: unable to convert default value %q to %v: %v", value, ykind, err)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: unable to convert default value %q to %v: %v", value, ykind, err)
 		}
 		if err := ytypes.ValidateDecimalRestrictions(args.yangType, val); err != nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match int restrictions: %v", value, err)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match int restrictions: %v", value, err)
 		}
-		return &value, ykind, nil
+		return value, ykind, nil
 	case yang.Ybinary:
 		bytes, err := base64.StdEncoding.DecodeString(value)
 		if err != nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: error in DecodeString for \n%v\n for type name %q: %q", value, args.yangType.Name, err)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: error in DecodeString for \n%v\n for type name %q: %q", value, args.yangType.Name, err)
 		}
 		if err := ytypes.ValidateBinaryRestrictions(args.yangType, bytes); err != nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match binary restrictions: %v", value, err)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match binary restrictions: %v", value, err)
 		}
 		value := fmt.Sprintf(ygot.BinaryTypeName+"(%q)", value)
-		return &value, ykind, nil
+		return value, ykind, nil
 	case yang.Ystring:
 		if err := ytypes.ValidateStringRestrictions(args.yangType, value); err != nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match string restrictions: %v", value, err)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: %q doesn't match string restrictions: %v", value, err)
 		}
 		value := fmt.Sprintf("%q", value)
-		return &value, ykind, nil
+		return value, ykind, nil
 	case yang.Ybool:
 		switch value {
 		case "true", "false":
-			return &value, ykind, nil
+			return value, ykind, nil
 		}
-		return nil, yang.Ynone, fmt.Errorf("default value conversion: cannot convert default value %q to bool, type name: %q", value, args.yangType.Name)
+		return "", yang.Ynone, fmt.Errorf("default value conversion: cannot convert default value %q to bool, type name: %q", value, args.yangType.Name)
 	case yang.Yempty:
-		return nil, yang.Ynone, fmt.Errorf("default value conversion: received default value %q, but an empty type cannot have a default value", value)
+		return "", yang.Ynone, fmt.Errorf("default value conversion: received default value %q, but an empty type cannot have a default value", value)
 	case yang.Yenum:
 		// Enumeration types need to be resolved to a particular data path such
 		// that a created enumerated Go type can be used to set their value. Hand
 		// the leaf to the enumName function to determine the name.
 		if args.contextEntry == nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: cannot map enum without context")
+			return "", yang.Ynone, fmt.Errorf("default value conversion: cannot map enum without context")
 		}
 		if strings.Contains(value, ":") {
 			value = strings.Split(value, ":")[1]
 		}
 		if !args.yangType.Enum.IsDefined(value) {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: enum value %q not found in enum with type name %q", value, args.yangType.Name)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: enum value %q not found in enum with type name %q", value, args.yangType.Name)
 		}
 		n, err := s.enumSet.enumName(args.contextEntry, compressOCPaths, false, skipEnumDedup, shortenEnumLeafNames, false, enumOrgPrefixesToTrim)
 		if err != nil {
-			return nil, yang.Ynone, err
+			return "", yang.Ynone, err
 		}
 		return enumDefaultValue(n, value, ""), ykind, nil
 	case yang.Yidentityref:
 		// Identityref leaves are mapped according to the base identity that they
 		// refer to - this is stored in the IdentityBase field of the context leaf
 		// which is determined by the identityrefBaseTypeFromLeaf.
 		if args.contextEntry == nil {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: cannot map identityref without context")
+			return "", yang.Ynone, fmt.Errorf("default value conversion: cannot map identityref without context")
 		}
 		if strings.Contains(value, ":") {
 			value = strings.Split(value, ":")[1]
 		}
 		if !args.yangType.IdentityBase.IsDefined(value) {
-			return nil, yang.Ynone, fmt.Errorf("default value conversion: identity value %q not found in enum with type name %q", value, args.yangType.Name)
+			return "", yang.Ynone, fmt.Errorf("default value conversion: identity value %q not found in enum with type name %q", value, args.yangType.Name)
 		}
 		n, err := s.enumSet.identityrefBaseTypeFromIdentity(args.yangType.IdentityBase)
 		if err != nil {
-			return nil, yang.Ynone, err
+			return "", yang.Ynone, err
 		}
 		return enumDefaultValue(n, value, ""), ykind, nil
 	case yang.Yleafref:
 		// This is a leafref, so we check what the type of the leaf that it
 		// references is by looking it up in the schematree.
 		target, err := s.schematree.resolveLeafrefTarget(args.yangType.Path, args.contextEntry)
 		if err != nil {
-			return nil, yang.Ynone, err
+			return "", yang.Ynone, err
 		}
 		return s.yangDefaultValueToGo(value, resolveTypeArgs{yangType: target.Type, contextEntry: target}, isSingletonUnion, compressOCPaths, skipEnumDedup, shortenEnumLeafNames, useDefiningModuleForTypedefEnumNames, enumOrgPrefixesToTrim)
 	case yang.Yunion:
 		// Try to convert to each type in order, but try the enumerated types first.
 		for _, t := range util.FlattenedTypes(args.yangType.Type) {
-			snippetRef, convertedKind, err := s.yangDefaultValueToGo(value, resolveTypeArgs{yangType: t, contextEntry: args.contextEntry}, isSingletonUnion, compressOCPaths, skipEnumDedup, shortenEnumLeafNames, useDefiningModuleForTypedefEnumNames, enumOrgPrefixesToTrim)
+			snippet, convertedKind, err := s.yangDefaultValueToGo(value, resolveTypeArgs{yangType: t, contextEntry: args.contextEntry}, isSingletonUnion, compressOCPaths, skipEnumDedup, shortenEnumLeafNames, useDefiningModuleForTypedefEnumNames, enumOrgPrefixesToTrim)
 			if err == nil {
 				if !isSingletonUnion {
 					if simpleName, ok := simpleUnionConversionsFromKind[convertedKind]; ok {
-						convertedSnippet := fmt.Sprintf("%s(%s)", simpleName, *snippetRef)
-						snippetRef = &convertedSnippet
+						snippet = fmt.Sprintf("%s(%s)", simpleName, snippet)
 					}
 				}
-				return snippetRef, convertedKind, nil
+				return snippet, convertedKind, nil
 			}
 		}
-		return nil, yang.Ynone, fmt.Errorf("default value conversion: cannot convert default value %q to any union subtype, type name %q", value, args.yangType.Name)
+		return "", yang.Ynone, fmt.Errorf("default value conversion: cannot convert default value %q to any union subtype, type name %q", value, args.yangType.Name)
 	default:
 		// Default values are not supported for unsupported types, so
 		// just generate the zero value instead.
 		// TODO(wenbli): support bit type.
-		return nil, yang.Ynone, fmt.Errorf("default value conversion: cannot create default value for unsupported type %v, type name: %q", ykind, args.yangType.Name)
+		return "", yang.Ynone, fmt.Errorf("default value conversion: cannot create default value for unsupported type %v, type name: %q", ykind, args.yangType.Name)
 	}
 }