Merge 20dc8ef into d841da9

generator/generator.go

@@ -73,6 +73,7 @@ var (
 	generateDelete         = flag.Bool("generate_delete", false, "If set to true, delete methods are generated for YANG lists (Go maps) within the Go code.")
 	generateLeafGetters    = flag.Bool("generate_leaf_getters", false, "If set to true, getters for YANG leaves are generated within the Go code. Caution should be exercised when using leaf getters, since values that are explicitly set to the Go default/zero value are not distinguishable from those that are unset when retrieved via the GetXXX method.")
 	includeModelData       = flag.Bool("include_model_data", false, "If set to true, a slice of gNMI ModelData messages are included in the generated Go code containing the details of the input schemas from which the code was generated.")
+	skipEnumDedup          = flag.Bool("skip_enum_deduplication", false, "If set to true, all leaves of type enumeration will have a unique enum output for them, rather than sharing a common type (default behaviour).")
 )
 
 // writeGoCodeSingleFile takes a ygen.GeneratedGoCode struct and writes the Go code
@@ -238,7 +239,8 @@ func main() {
 	// Perform the code generation.
 	cg := ygen.NewYANGCodeGenerator(&ygen.GeneratorConfig{
 		ParseOptions: ygen.ParseOpts{
-			ExcludeModules: modsExcluded,
+			ExcludeModules:        modsExcluded,
+			SkipEnumDeduplication: *skipEnumDedup,
 			YANGParseOptions: yang.Options{
 				IgnoreSubmoduleCircularDependencies: *ignoreCircDeps,
 			},

proto_generator/protogenerator.go

@@ -50,6 +50,7 @@ var (
 	callerName             = flag.String("caller_name", "proto_generator", "The name of the generator binary that should be recorded in output files.")
 	excludeState           = flag.Bool("exclude_state", false, "If set to true, state (config false) fields in the YANG schema are not included in the generated Protobuf messages.")
 	preferOperationalState = flag.Bool("prefer_operational_state", false, "If set to true, state (config false) fields in the YANG schema are preferred over intended config leaves in the generated messages with compressed schema paths. This flag is only valid for compress_paths=true and exclude_state=false.")
+	skipEnumDedup          = flag.Bool("skip_enum_deduplication", false, "If set to true, all leaves of type enumeration will have a unique enum output for them, rather than sharing a common type (default behaviour).")
 )
 
 // main parses command-line flags to determine the set of YANG modules for
@@ -100,7 +101,8 @@ func main() {
 	// Perform the code generation.
 	cg := ygen.NewYANGCodeGenerator(&ygen.GeneratorConfig{
 		ParseOptions: ygen.ParseOpts{
-			ExcludeModules: modsExcluded,
+			ExcludeModules:        modsExcluded,
+			SkipEnumDeduplication: *skipEnumDedup,
 			YANGParseOptions: yang.Options{
 				IgnoreSubmoduleCircularDependencies: *ignoreCircDeps,
 			},

testdata/modules/enum-duplication.yang

@@ -0,0 +1,19 @@
+module enum-duplication {
+    prefix "e";
+    namespace "urn:e";
+
+    grouping duplicated-use {
+        leaf enumerated {
+            type enumeration {
+                enum A;
+                enum B;
+                enum C;
+            }
+        }
+    }
+
+    container base {
+        container config { uses duplicated-use; }
+        container state { config false; uses duplicated-use; }
+    }
+}

ygen/codegen.go

@@ -93,6 +93,34 @@ type ParseOpts struct {
 	// github.com/openconfig/goyang/pkg/yang library. These specify how the
 	// input YANG files should be parsed.
 	YANGParseOptions yang.Options
+	// SkipEnumDeduplication specifies whether leaves of type 'enumeration' that
+	// are used in multiple places in the schema should share a common type within
+	// the generated code that is output by ygen. By default (false), a common type
+	// is used.
+	//
+	// This behaviour is useful in scenarios where there is no difference between
+	// two types, and the leaf is mirrored in a logical way (e.g., the OpenConfig
+	// config/state split). For example:
+	//
+	// grouping foo-config {
+	//	leaf enabled {
+	//		type enumeration {
+	//			enum A;
+	//			enum B;
+	//			enum C;
+	//		}
+	//	 }
+	// }
+	//
+	//  container config { uses foo-config; }
+	//  container state { uses foo-config; }
+	//
+	// will result in a single enumeration type (ModuleName_Config_Enabled) being
+	// output when de-duplication is enabled.
+	//
+	// When it is disabled, two different enumerations (ModuleName_(State|Config)_Enabled)
+	// will be output in the generated code.
+	SkipEnumDeduplication bool
 }
 
 // TransformationOpts specifies transformations to the generated code with
@@ -109,6 +137,11 @@ type TransformationOpts struct {
 	// FakeRootName specifies the name of the struct that should be generated
 	// representing the root.
 	FakeRootName string
+	// ExcludeState specifies whether config false values should be
+	// included in the generated code output. When set, all values that are
+	// not writeable (i.e., config false) within the YANG schema and their
+	// children are excluded from the generated code.
+	ExcludeState bool
 }
 
 // GoOpts stores Go specific options for the code generation library.
@@ -321,7 +354,7 @@ func (cg *YANGCodeGenerator) GenerateGoCode(yangFiles, includePaths []string) (*
 	// Store the returned schematree within the state for this code generation.
 	gogen := newGoGenState(mdef.schematree)
 
-	directoryMap, errs := gogen.buildDirectoryDefinitions(mdef.directoryEntries, cg.Config.TransformationOptions.CompressBehaviour, cg.Config.TransformationOptions.GenerateFakeRoot)
+	directoryMap, errs := gogen.buildDirectoryDefinitions(mdef.directoryEntries, cg.Config.TransformationOptions.CompressBehaviour, cg.Config.TransformationOptions.GenerateFakeRoot, cg.Config.ParseOptions.SkipEnumDeduplication)
 	if errs != nil {
 		return nil, errs
 	}
@@ -353,7 +386,7 @@ func (cg *YANGCodeGenerator) GenerateGoCode(yangFiles, includePaths []string) (*
 	var structSnippets []GoStructCodeSnippet
 	for _, directoryName := range orderedDirNames {
 		structOut, errs := writeGoStruct(dirNameMap[directoryName], directoryMap, gogen,
-			cg.Config.TransformationOptions.CompressBehaviour.CompressEnabled(), cg.Config.GenerateJSONSchema, cg.Config.GoOptions)
+			cg.Config.TransformationOptions.CompressBehaviour.CompressEnabled(), cg.Config.GenerateJSONSchema, cg.Config.ParseOptions.SkipEnumDeduplication, cg.Config.GoOptions)
 		if errs != nil {
 			codegenErr = util.AppendErrs(codegenErr, errs)
 			continue
@@ -367,7 +400,7 @@ func (cg *YANGCodeGenerator) GenerateGoCode(yangFiles, includePaths []string) (*
 		}
 	}
 
-	goEnums, errs := gogen.enumGen.findEnumSet(mdef.enumEntries, cg.Config.TransformationOptions.CompressBehaviour.CompressEnabled(), false)
+	goEnums, errs := gogen.enumGen.findEnumSet(mdef.enumEntries, cg.Config.TransformationOptions.CompressBehaviour.CompressEnabled(), false, cg.Config.ParseOptions.SkipEnumDeduplication)
 	if errs != nil {
 		codegenErr = util.AppendErrs(codegenErr, errs)
 		return nil, codegenErr
@@ -447,7 +480,7 @@ func (dcg *DirectoryGenConfig) GetDirectoriesAndLeafTypes(yangFiles, includePath
 	// Store the returned schematree within the state for this code generation.
 	gogen := newGoGenState(mdef.schematree)
 
-	directoryMap, errs := gogen.buildDirectoryDefinitions(dirsToProcess, cg.TransformationOptions.CompressBehaviour, cg.TransformationOptions.GenerateFakeRoot)
+	directoryMap, errs := gogen.buildDirectoryDefinitions(dirsToProcess, cg.TransformationOptions.CompressBehaviour, cg.TransformationOptions.GenerateFakeRoot, cg.ParseOptions.SkipEnumDeduplication)
 	if errs != nil {
 		return nil, nil, errs
 	}
@@ -468,7 +501,7 @@ func (dcg *DirectoryGenConfig) GetDirectoriesAndLeafTypes(yangFiles, includePath
 		for _, fieldName := range GetOrderedFieldNames(dir) {
 			field := dir.Fields[fieldName]
 			if isLeaf := field.IsLeaf() || field.IsLeafList(); isLeaf {
-				mtype, err := gogen.yangTypeToGoType(resolveTypeArgs{yangType: field.Type, contextEntry: field}, dcg.TransformationOptions.CompressBehaviour.CompressEnabled())
+				mtype, err := gogen.yangTypeToGoType(resolveTypeArgs{yangType: field.Type, contextEntry: field}, dcg.TransformationOptions.CompressBehaviour.CompressEnabled(), cg.ParseOptions.SkipEnumDeduplication)
 				if err != nil {
 					errs = util.AppendErr(errs, err)
 					continue
@@ -545,7 +578,7 @@ func (cg *YANGCodeGenerator) GenerateProto3(yangFiles, includePaths []string) (*
 
 	protogen := newProtoGenState(mdef.schematree)
 
-	penums, errs := protogen.enumGen.findEnumSet(mdef.enumEntries, cg.Config.TransformationOptions.CompressBehaviour.CompressEnabled(), true)
+	penums, errs := protogen.enumGen.findEnumSet(mdef.enumEntries, cg.Config.TransformationOptions.CompressBehaviour.CompressEnabled(), true, cg.Config.ParseOptions.SkipEnumDeduplication)
 	if errs != nil {
 		return nil, errs
 	}

ygen/codegen_test.go

@@ -703,6 +703,27 @@ func TestSimpleStructs(t *testing.T) {
 			},
 		},
 		wantStructsCodeFile: filepath.Join(TestRoot, "testdata", "structs", "openconfig-versioned-mod.formatted-txt"),
+	}, {
+		name:    "model with deduplicated enums",
+		inFiles: []string{filepath.Join(datapath, "enum-duplication.yang")},
+		inConfig: GeneratorConfig{
+			TransformationOptions: TransformationOpts{
+				GenerateFakeRoot: true,
+			},
+		},
+		wantStructsCodeFile: filepath.Join(TestRoot, "testdata", "structs", "enum-duplication-dedup.formatted-txt"),
+	}, {
+		name:    "model with enums that are in the same grouping duplicated",
+		inFiles: []string{filepath.Join(datapath, "enum-duplication.yang")},
+		inConfig: GeneratorConfig{
+			TransformationOptions: TransformationOpts{
+				GenerateFakeRoot: true,
+			},
+			ParseOptions: ParseOpts{
+				SkipEnumDeduplication: true,
+			},
+		},
+		wantStructsCodeFile: filepath.Join(TestRoot, "testdata", "structs", "enum-duplication-dup.formatted-txt"),
 	}}
 
 	for _, tt := range tests {

ygen/genstate.go

@@ -81,7 +81,7 @@ func newEnumGenState() *enumGenState {
 // value is calculated based on the original context, whether path compression is enabled based
 // on the compressPaths boolean, and whether the name should not include underscores, as per the
 // noUnderscores boolean.
-func (s *enumGenState) enumeratedUnionEntry(e *yang.Entry, compressPaths, noUnderscores bool) ([]*yangEnum, error) {
+func (s *enumGenState) enumeratedUnionEntry(e *yang.Entry, compressPaths, noUnderscores, skipEnumDedup bool) ([]*yangEnum, error) {
 	var es []*yangEnum
 
 	for _, t := range util.EnumeratedUnionTypes(e.Type.Type) {
@@ -102,7 +102,7 @@ func (s *enumGenState) enumeratedUnionEntry(e *yang.Entry, compressPaths, noUnde
 		case t.Enum != nil:
 			var enumName string
 			if _, chBuiltin := yang.TypeKindFromName[t.Name]; chBuiltin {
-				enumName = s.resolveEnumName(e, compressPaths, noUnderscores)
+				enumName = s.resolveEnumName(e, compressPaths, noUnderscores, skipEnumDedup)
 			} else {
 				var err error
 				enumName, err = s.resolveTypedefEnumeratedName(e, noUnderscores)
@@ -208,8 +208,11 @@ func buildDirectoryDefinitions(entries map[string]*yang.Entry, compBehaviour gen
 // duplication between config and state containers when compressPaths is true.
 // It also de-dups references to the same identity base, and type definitions.
 // If noUnderscores is set to true, then underscores are omitted from the enum
+// names to reflect to the preferred style of some generated languages. If skipEnumDedup
+// is set to true, we do not attempt to deduplicate enumerated leaves that are used more
+// than once in the schema into a common type.
 // names to reflect to the preferred style of some generated languages.
-func (s *enumGenState) findEnumSet(entries map[string]*yang.Entry, compressPaths, noUnderscores bool) (map[string]*yangEnum, []error) {
+func (s *enumGenState) findEnumSet(entries map[string]*yang.Entry, compressPaths, noUnderscores, skipEnumDedup bool) (map[string]*yangEnum, []error) {
 	validEnums := make(map[string]*yang.Entry)
 	var enumNames []string
 	var errs []error
@@ -272,7 +275,7 @@ func (s *enumGenState) findEnumSet(entries map[string]*yang.Entry, compressPaths
 		case e.Type.Name == "union", len(e.Type.Type) > 0 && !builtin:
 			// Calculate any enumerated types that exist within a union, whether it
 			// is a directly defined union, or a non-builtin typedef.
-			es, err := s.enumeratedUnionEntry(e, compressPaths, noUnderscores)
+			es, err := s.enumeratedUnionEntry(e, compressPaths, noUnderscores, skipEnumDedup)
 			if err != nil {
 				errs = append(errs, err)
 				continue
@@ -305,7 +308,7 @@ func (s *enumGenState) findEnumSet(entries map[string]*yang.Entry, compressPaths
 			// in two places, then we do not want to have multiple enumerated types
 			// that represent this leaf), then we do not have errors if duplicates
 			// occur, we simply perform de-duplication at this stage.
-			enumName := s.resolveEnumName(e, compressPaths, noUnderscores)
+			enumName := s.resolveEnumName(e, compressPaths, noUnderscores, skipEnumDedup)
 			if _, ok := genEnums[enumName]; !ok {
 				genEnums[enumName] = &yangEnum{
 					name:  enumName,
@@ -373,13 +376,11 @@ func (s *enumGenState) identityrefBaseTypeFromIdentity(i *yang.Identity, noUnder
 	return uniqueName
 }
 
-// resolveEnumName takes a yang.Entry and resolves its name into the type name
-// that will be used in the generated code. Whilst a leaf may only be used
-// in a single context (i.e., at its own path), resolveEnumName may be called
-// multiple times, and hence de-duplication of unique name generation is required.
-// If noUnderscores is set to true, then underscores are omitted from the
-// output name.
-func (s *enumGenState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscores bool) string {
+// enumIdentifier takes in an enum entry and returns a unique identifier for
+// that enum constructed using its path. This identifier would be the same for
+// an enum that's used in two different places in the schema.
+func enumIdentifier(e *yang.Entry, compressPaths bool) string {
+	definingModName := genutil.ParentModulePrettyName(e.Node)
 	// It is possible, given a particular enumerated leaf, for it to appear
 	// multiple times in the schema. For example, through being defined in
 	// a grouping which is instantiated in two places. In these cases, the
@@ -395,17 +396,16 @@ func (s *enumGenState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscor
 	// "Node" hierarchy - we walk back up the tree until such time as we find
 	// a node that is not within the same module (ParentModulePrettyName(parent) !=
 	// ParentModulePrettyName(currentNode)), and use this as the unique path.
-	definingModName := genutil.ParentModulePrettyName(e.Node)
 	var identifierPathElem []string
 	for elem := e.Node; elem.ParentNode() != nil && genutil.ParentModulePrettyName(elem) == definingModName; elem = elem.ParentNode() {
 		identifierPathElem = append(identifierPathElem, elem.NName())
 	}
 
 	// Since the path elements are compiled from leaf back to root, then reverse them to
 	// form the path, this is not strictly required, but aids debugging of the elements.
-	var identifierPath string
+	var identifier string
 	for i := len(identifierPathElem) - 1; i >= 0; i-- {
-		identifierPath = fmt.Sprintf("%s/%s", identifierPath, identifierPathElem[i])
+		identifier = fmt.Sprintf("%s/%s", identifier, identifierPathElem[i])
 	}
 
 	// For leaves that have an enumeration within a typedef that is within a union,
@@ -419,31 +419,81 @@ func (s *enumGenState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscor
 		if compressPaths && e.Parent != nil && e.Parent.Parent != nil {
 			idPfx = e.Parent.Parent.Name
 		}
-		identifierPath = fmt.Sprintf("%s%s", idPfx, identifierPath)
+		identifier = fmt.Sprintf("%s%s", idPfx, identifier)
 	}
+	return identifier
+}
 
-	// If the leaf had already been encountered, then return the previously generated
-	// name, rather than generating a new name.
-	if definedName, ok := s.uniqueEnumeratedLeafNames[identifierPath]; ok {
-		return definedName
+// resolveEnumName takes a yang.Entry and resolves its name into the type name
+// that will be used in the generated code. Whilst a leaf may only be used
+// in a single context (i.e., at its own path), resolveEnumName may be called
+// multiple times, and hence de-duplication of unique name generation is required.
+// If noUnderscores is set to true, then underscores are omitted from the
+// output name.
+// If the skipDedup argument is set to true, where a single enumeration is defined
+// once in the input YANG, but instantiated multiple times (e.g., a grouping is
+// used multiple times that contains an enumeration), then we do not attempt to
+// use a single output type in the generated code for such enumerations. This allows
+// the user to control whether this behaviour is useful to them -- for OpenConfig,
+// it tends to be due to the state/config split - which would otherwise result in
+// multiple enumerated types being produced. For other schemas, it can result in
+// somewhat difficult to understand enumerated types being produced - since the first
+// leaf that is processed will define the name of the enumeration.
+func (s *enumGenState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscores, skipDedup bool) string {
+	// uniqueIdentifer is the unique identifier used to determine whether to
+	// define a new enum type for the input enum.
+	var uniqueIdentifer string
+	if skipDedup && !compressPaths {
+		// If not using compression and duplicating, then we use the
+		// entire path as the enum name, meaning every enum instance
+		// has its own definition. By using the entry's path as its
+		// identifier, we ensure this.
+		uniqueIdentifer = e.Path()
+	} else {
+		// In the other cases, de-duplication may happen, either
+		// through de-duping multiple usages or possibly through
+		// compression
+		uniqueIdentifer = enumIdentifier(e, compressPaths)
 	}
 
+	var compressName string
 	if compressPaths {
+		definingModName := genutil.ParentModulePrettyName(e.Node)
 		// If we compress paths then the name of this enum is of the form
 		// ModuleName_GrandParent_Leaf - we use GrandParent since Parent is
 		// State or Config so would not be unique. The proposed name is
 		// handed to genutil.MakeNameUnique to ensure that it does not clash with
 		// other defined names.
-		name := fmt.Sprintf("%s_%s_%s", yang.CamelCase(definingModName), yang.CamelCase(e.Parent.Parent.Name), yang.CamelCase(e.Name))
+		compressName = fmt.Sprintf("%s_%s_%s", yang.CamelCase(definingModName), yang.CamelCase(e.Parent.Parent.Name), yang.CamelCase(e.Name))
 		if noUnderscores {
-			name = strings.Replace(name, "_", "", -1)
+			compressName = strings.Replace(compressName, "_", "", -1)
 		}
-		uniqueName := genutil.MakeNameUnique(name, s.definedEnums)
-		s.uniqueEnumeratedLeafNames[identifierPath] = uniqueName
+
+		if skipDedup {
+			// If using compression and duplicating, then we add
+			// compressName to the uniqueIdentifier, meaning every
+			// enum instance in the compressed view of the schema
+			// has its own definition.  The base enum identity is
+			// still required to deal with collisions between
+			// compressed enum names when they describe different
+			// enums.
+			uniqueIdentifer += compressName
+		}
+	}
+
+	// If the leaf had already been encountered, then return the previously generated
+	// name, rather than generating a new name.
+	if definedName, ok := s.uniqueEnumeratedLeafNames[uniqueIdentifer]; ok {
+		return definedName
+	}
+
+	if compressPaths {
+		uniqueName := genutil.MakeNameUnique(compressName, s.definedEnums)
+		s.uniqueEnumeratedLeafNames[uniqueIdentifer] = uniqueName
 		return uniqueName
 	}
 
-	// If this was we don't compress the paths, then we write out the entire path.
+	// If we are not compressing the paths, then we write out the entire path.
 	var nbuf bytes.Buffer
 	for i, p := range util.SchemaPathNoChoiceCase(e) {
 		if i != 0 && !noUnderscores {
@@ -452,7 +502,7 @@ func (s *enumGenState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscor
 		nbuf.WriteString(yang.CamelCase(p))
 	}
 	uniqueName := genutil.MakeNameUnique(nbuf.String(), s.definedEnums)
-	s.uniqueEnumeratedLeafNames[identifierPath] = uniqueName
+	s.uniqueEnumeratedLeafNames[uniqueIdentifer] = uniqueName
 	return uniqueName
 }