Merge 897e47e into 85c2a61

docs/design.md

@@ -58,7 +58,7 @@ generation).
 The logic to extract which entities are valid to have code
 generation performed for them (skipping `config`/`state` containers, and
 surrounding containers for lists) is found in
-`go_elements.go`:`FindAllChildren`.
+`FindAllChildren` in `genutil` package.
 
 ## YANG Entities Mapped to Go Entities
 

genutil/common.go

@@ -104,16 +104,25 @@ type CompressBehaviour int64
 // No dimension spans across all others' options, so can't extract any out
 // without having to error out for some combinations.
 const (
-	// Uncompressed means to not compress the schema.
+	// Uncompressed does not compress the generated code. This means list
+	// containers and config/state containers are retained in the generated
+	// code.
 	Uncompressed CompressBehaviour = iota
-	// UncompressedExcludeDerivedState excludes config false subtrees.
+	// UncompressedExcludeDerivedState excludes config false subtrees in
+	// the generated code.
 	UncompressedExcludeDerivedState
-	// PreferIntendedConfig indicates to use the "config" version of a
-	// schema entry instead of its "state" counterpart when both exist.
+	// PreferIntendedConfig generates only the "config" version of a field
+	// when it exists under both "config" and "state" containers of its
+	// parent YANG model. This is done to resolve field conflicts that
+	// arise in the compressed code due to "config" and "state" containers
+	// being removed and their fields collapsed together. If no conflict
+	// exists between these containers, then the field is always generated.
 	PreferIntendedConfig
-	// PreferOperationalState indicates to use the "state" version of a
-	// schema entry instead of its "config" counterpart when both exist.
-	PreferOperationalState // prefer applied config
+	// PreferOperationalState generates only the "state" version of a field
+	// when it exists under both "config" and "state" containers of its
+	// parent YANG model. If no conflict exists between these containers,
+	// then the field is always generated.
+	PreferOperationalState
 	// ExcludeDerivedState excludes all values that are not writeable
 	// (i.e. config false), including their children, from the generated
 	// code output.

ygen/gogen.go

@@ -17,12 +17,12 @@ package ygen
 import (
 	"bytes"
 	"fmt"
-	"reflect"
 	"sort"
 	"strings"
 	"text/template"
 
 	log "github.com/golang/glog"
+	"github.com/google/go-cmp/cmp"
 
 	"github.com/openconfig/gnmi/errlist"
 	gpb "github.com/openconfig/gnmi/proto/gnmi"
@@ -1934,8 +1934,8 @@ func findMapPaths(parent *Directory, fieldName string, compressPaths, absolutePa
 	}
 	fieldSlicePath := util.SchemaPathNoChoiceCase(field)
 
-	// Handle specific issue of compression of the schema, where the key
-	// of the parent list is a leafref to this leaf.
+	// Handle specific issue of compressed path schemas, where a key of the
+	// parent list is a leafref to this leaf.
 	for _, k := range parent.ListAttr.KeyElems {
 		// If the key element has the same path as this element, and the
 		// corresponding element that is within the parent's container is of
@@ -1948,7 +1948,12 @@ func findMapPaths(parent *Directory, fieldName string, compressPaths, absolutePa
 			return nil, fmt.Errorf("invalid compressed schema, could not find the key %s or the grandparent of %s", k.Name, k.Path())
 		}
 
-		if reflect.DeepEqual(util.SchemaPathNoChoiceCase(k), fieldSlicePath) && k.Parent.Parent.Dir[k.Name].Type.Kind == yang.Yleafref {
+		// If a key of the list is a leafref that points to the field,
+		// then add this as an alternative path.
+		// Note: if k is a leafref, buildListKey() would have already
+		// resolved it the field that the leafref points to. So, we
+		// compare their absolute paths for equality.
+		if k.Parent.Parent.Dir[k.Name].Type.Kind == yang.Yleafref && cmp.Equal(util.SchemaPathNoChoiceCase(k), fieldSlicePath) {
 			// The path of the key element is simply the name of the leaf under the
 			// list, since the YANG specification enforces that keys are direct
 			// children of the list.