Repository: openconfig/goyang
Pull request #125 (2020-06-19 00:26:35)

Parse and semantic-check min-elements and max-elements.
Currently, the statements don't undergo semantic checking, forcing
backends like ygot to have to handle it (which incidentally does it
wrong -- to be handled separately).

These fields are the most useful as uint64. This is because whether or
not they're explicitly set in the `list` or `leaf-list`, there is always
a valid uint64 value for them. As a result, uint64 is proposed as the
type for these fields.

By not using a pointer, however, the implementation of deviations
necessitates the new `deviationPresence` field to distiguish, for a
DeviationEntry, between no deviation applied vs. the default uint64
value. So, it has been added to the `Entry` type.

Lastly, deviation for deletes was actually unimplemented due to a test
bug that misinterpreted `nil` as "don't test it", rather than "test that
it's nil". This has been implemented and the test fixed.


Patch:
diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go
index 3c373dd..41da2b0 100644
--- a/pkg/yang/entry.go
+++ b/pkg/yang/entry.go
@@ -25,6 +25,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"reflect"
 	"sort"
 	"strconv"
@@ -62,6 +63,14 @@ func (t TriState) String() string {
 	}
 }
 
+// deviationPresence stores whether certain attributes for a DeviateEntry-type
+// Entry have been given deviation values. This is useful when the attribute
+// doesn't have a presence indicator (e.g. non-pointers).
+type deviationPresence struct {
+	hasMinElements bool
+	hasMaxElements bool
+}
+
 // An Entry represents a single node (directory or leaf) created from the
 // AST.  Directory entries have a non-nil Dir entry.  Leaf nodes have a nil
 // Dir entry.  If Errors is not nil then the only other valid field is Node.
@@ -99,7 +108,10 @@ type Entry struct {
 	Augmented  []*Entry                   `json:",omitempty"` // Augments merged into this entry.
 	Deviations []*DeviatedEntry           `json:"-"`          // Deviations associated with this entry.
 	Deviate    map[deviationType][]*Entry `json:"-"`
-	Uses       []*UsesStmt                `json:",omitempty"` // Uses merged into this entry.
+	// deviationPresence tracks whether certain attributes for a DeviateEntry-type
+	// Entry have been given deviation values.
+	deviatePresence deviationPresence
+	Uses            []*UsesStmt `json:",omitempty"` // Uses merged into this entry.
 
 	// Extra maps all the unsupported fields to their values
 	Extra map[string][]interface{} `json:"-"`
@@ -125,8 +137,8 @@ type RPCEntry struct {
 
 // A ListAttr is associated with an Entry that represents a List node
 type ListAttr struct {
-	MinElements *Value // leaf-list or list MUST have at least min-elements
-	MaxElements *Value // leaf-list or list has at most max-elements
+	MinElements uint64 // leaf-list or list MUST have at least min-elements
+	MaxElements uint64 // leaf-list or list has at most max-elements
 	OrderedBy   *Value // order of entries determined by "system" or "user"
 }
 
@@ -473,6 +485,30 @@ type DeviatedEntry struct {
 	*Entry
 }
 
+// semCheckMaxElements checks whether the max-element argument is valid, and returns the specified value.
+func semCheckMaxElements(v *Value) (uint64, error) {
+	if v == nil || v.Name == "unbounded" {
+		return math.MaxUint64, nil
+	}
+	val, err := strconv.ParseUint(v.Name, 10, 64)
+	if err != nil {
+		return val, fmt.Errorf(`%s: invalid max-elements value %q (expect "unbounded" or a non-negative integer): %v`, Source(v), v.Name, err)
+	}
+	return val, nil
+}
+
+// semCheckMinElements checks whether the min-element argument is valid, and returns the specified value.
+func semCheckMinElements(v *Value) (uint64, error) {
+	if v == nil {
+		return 0, nil
+	}
+	val, err := strconv.ParseUint(v.Name, 10, 64)
+	if err != nil {
+		return val, fmt.Errorf(`%s: invalid min-elements value %q (expect a non-negative integer): %v`, Source(v), v.Name, err)
+	}
+	return val, nil
+}
+
 // ToEntry expands node n into a directory Entry.  Expansion is based on the
 // YANG tags in the structure behind n.  ToEntry must only be used
 // with nodes that are directories, such as top level modules and sub-modules.
@@ -540,7 +576,6 @@ func ToEntry(n Node) (e *Entry) {
 		e.Config, err = tristateValue(s.Config)
 		e.addError(err)
 		e.Prefix = getRootPrefix(e)
-		addExtraKeywordsToLeafEntry(n, e)
 		return e
 	case *LeafList:
 		// Create the equivalent leaf element that we are a list of.
@@ -563,12 +598,16 @@ func ToEntry(n Node) (e *Entry) {
 
 		e := ToEntry(leaf)
 		e.ListAttr = &ListAttr{
-			MinElements: s.MinElements,
-			MaxElements: s.MaxElements,
-			OrderedBy:   s.OrderedBy,
+			OrderedBy: s.OrderedBy,
+		}
+		var err error
+		if e.ListAttr.MaxElements, err = semCheckMaxElements(s.MaxElements); err != nil {
+			e.addError(err)
+		}
+		if e.ListAttr.MinElements, err = semCheckMinElements(s.MinElements); err != nil {
+			e.addError(err)
 		}
 		e.Prefix = getRootPrefix(e)
-		addExtraKeywordsToLeafEntry(n, e)
 		return e
 	case *Uses:
 		g := FindGrouping(s, s.Name, map[string]bool{})
@@ -578,9 +617,7 @@ func ToEntry(n Node) (e *Entry) {
 		// We need to return a duplicate so we resolve properly
 		// when the group is used in multiple locations and the
 		// grouping has a leafref that references outside the group.
-		e := ToEntry(g).dup()
-		addExtraKeywordsToLeafEntry(n, e)
-		return e
+		return ToEntry(g).dup()
 	}
 
 	e = newDirectory(n)
@@ -592,9 +629,14 @@ func ToEntry(n Node) (e *Entry) {
 	switch s := n.(type) {
 	case *List:
 		e.ListAttr = &ListAttr{
-			MinElements: s.MinElements,
-			MaxElements: s.MaxElements,
-			OrderedBy:   s.OrderedBy,
+			OrderedBy: s.OrderedBy,
+		}
+		var err error
+		if e.ListAttr.MaxElements, err = semCheckMaxElements(s.MaxElements); err != nil {
+			e.addError(err)
+		}
+		if e.ListAttr.MinElements, err = semCheckMinElements(s.MinElements); err != nil {
+			e.addError(err)
 		}
 	case *Choice:
 		e.Kind = ChoiceEntry
@@ -884,10 +926,20 @@ func ToEntry(n Node) (e *Entry) {
 				e.ListAttr = &ListAttr{}
 			}
 
-			if name == "max-elements" {
-				e.ListAttr.MaxElements = v
-			} else {
-				e.ListAttr.MinElements = v
+			// Only record the deviation if the statement exists.
+			if v != nil {
+				var err error
+				if name == "max-elements" {
+					e.deviatePresence.hasMaxElements = true
+					if e.ListAttr.MaxElements, err = semCheckMaxElements(v); err != nil {
+						e.addError(err)
+					}
+				} else {
+					e.deviatePresence.hasMinElements = true
+					if e.ListAttr.MinElements, err = semCheckMinElements(v); err != nil {
+						e.addError(err)
+					}
+				}
 			}
 		case "units":
 			v, ok := fv.Interface().(*Value)
@@ -939,30 +991,6 @@ func ToEntry(n Node) (e *Entry) {
 	return e
 }
 
-// addExtraKeywordsToLeafEntry stores the values for unimplemented keywords in leaf entries.
-func addExtraKeywordsToLeafEntry(n Node, e *Entry) {
-	v := reflect.ValueOf(n).Elem()
-	t := v.Type()
-
-	for i := t.NumField() - 1; i > 0; i-- {
-		f := t.Field(i)
-		yang := f.Tag.Get("yang")
-		if yang == "" {
-			continue
-		}
-		fv := v.Field(i)
-		name := strings.Split(yang, ",")[0]
-		switch name {
-		case "if-feature",
-			"must",
-			"reference",
-			"status",
-			"when":
-			e.Extra[name] = append(e.Extra[name], fv.Interface())
-		}
-	}
-}
-
 // getRootPrefix returns the prefix of e's root node (module)
 func getRootPrefix(e *Entry) *Value {
 	if m := RootNode(e.Node); m != nil {
@@ -1034,7 +1062,7 @@ func (e *Entry) ApplyDeviate() []error {
 						deviatedNode.Mandatory = devSpec.Mandatory
 					}
 
-					if devSpec.ListAttr != nil && devSpec.ListAttr.MinElements != nil {
+					if devSpec.deviatePresence.hasMinElements {
 						if !deviatedNode.IsList() && !deviatedNode.IsLeafList() {
 							appendErr(fmt.Errorf("tried to deviate min-elements on a non-list type %s", deviatedNode.Kind))
 							continue
@@ -1042,7 +1070,7 @@ func (e *Entry) ApplyDeviate() []error {
 						deviatedNode.ListAttr.MinElements = devSpec.ListAttr.MinElements
 					}
 
-					if devSpec.ListAttr != nil && devSpec.ListAttr.MaxElements != nil {
+					if devSpec.deviatePresence.hasMaxElements {
 						if !deviatedNode.IsList() && !deviatedNode.IsLeafList() {
 							appendErr(fmt.Errorf("tried to deviate max-elements on a non-list type %s", deviatedNode.Kind))
 							continue
@@ -1077,6 +1105,31 @@ func (e *Entry) ApplyDeviate() []error {
 					if devSpec.Mandatory != TSUnset {
 						devSpec.Mandatory = TSUnset
 					}
+
+					if devSpec.deviatePresence.hasMinElements {
+						if !deviatedNode.IsList() && !deviatedNode.IsLeafList() {
+							appendErr(fmt.Errorf("tried to deviate min-elements on a non-list type %s", deviatedNode.Kind))
+							continue
+						}
+						if deviatedNode.ListAttr.MinElements != devSpec.ListAttr.MinElements {
+							// Argument value must match:
+							// https://tools.ietf.org/html/rfc7950#section-7.20.3.2
+							appendErr(fmt.Errorf("min-element value %d differs from deviation's min-element value %d for entry %v", devSpec.ListAttr.MinElements, deviatedNode.ListAttr.MinElements, d.DeviatedPath))
+						}
+						deviatedNode.ListAttr.MinElements = 0
+					}
+
+					if devSpec.deviatePresence.hasMaxElements {
+						if !deviatedNode.IsList() && !deviatedNode.IsLeafList() {
+							appendErr(fmt.Errorf("tried to deviate max-elements on a non-list type %s", deviatedNode.Kind))
+							continue
+						}
+						if deviatedNode.ListAttr.MaxElements != devSpec.ListAttr.MaxElements {
+							appendErr(fmt.Errorf("max-element value %d differs from deviation's max-element value %d for entry %v", devSpec.ListAttr.MaxElements, deviatedNode.ListAttr.MaxElements, d.DeviatedPath))
+						}
+						deviatedNode.ListAttr.MaxElements = math.MaxUint64
+					}
+
 				default:
 					appendErr(fmt.Errorf("invalid deviation type %s", dt))
 				}
diff --git a/pkg/yang/yang.go b/pkg/yang/yang.go
index 7b466a7..da7638a 100644
--- a/pkg/yang/yang.go
+++ b/pkg/yang/yang.go
@@ -339,26 +339,25 @@ type Container struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Anydata      []*AnyData      `yang:"anydata"`
-	Action       []*Action       `yang:"action"`
-	Anyxml       []*AnyXML       `yang:"anyxml"`
-	Choice       []*Choice       `yang:"choice"`
-	Config       *Value          `yang:"config"`
-	Container    []*Container    `yang:"container"`
-	Description  *Value          `yang:"description"`
-	Grouping     []*Grouping     `yang:"grouping"`
-	IfFeature    []*Value        `yang:"if-feature"`
-	Leaf         []*Leaf         `yang:"leaf"`
-	LeafList     []*LeafList     `yang:"leaf-list"`
-	List         []*List         `yang:"list"`
-	Must         []*Must         `yang:"must"`
-	Notification []*Notification `yang:"notification"`
-	Presence     *Value          `yang:"presence"`
-	Reference    *Value          `yang:"reference"`
-	Status       *Value          `yang:"status"`
-	Typedef      []*Typedef      `yang:"typedef"`
-	Uses         []*Uses         `yang:"uses"`
-	When         *Value          `yang:"when"`
+	Anydata     []*AnyData   `yang:"anydata"`
+	Action      []*Action    `yang:"action"`
+	Anyxml      []*AnyXML    `yang:"anyxml"`
+	Choice      []*Choice    `yang:"choice"`
+	Config      *Value       `yang:"config"`
+	Container   []*Container `yang:"container"`
+	Description *Value       `yang:"description"`
+	Grouping    []*Grouping  `yang:"grouping"`
+	IfFeature   []*Value     `yang:"if-feature"`
+	Leaf        []*Leaf      `yang:"leaf"`
+	LeafList    []*LeafList  `yang:"leaf-list"`
+	List        []*List      `yang:"list"`
+	Must        []*Must      `yang:"must"`
+	Presence    *Value       `yang:"presence"`
+	Reference   *Value       `yang:"reference"`
+	Status      *Value       `yang:"status"`
+	Typedef     []*Typedef   `yang:"typedef"`
+	Uses        []*Uses      `yang:"uses"`
+	When        *Value       `yang:"when"`
 }
 
 func (Container) Kind() string              { return "container" }
@@ -450,30 +449,29 @@ type List struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Anydata      []*AnyData      `yang:"anydata"`
-	Action       []*Action       `yang:"action"`
-	Anyxml       []*AnyXML       `yang:"anyxml"`
-	Choice       []*Choice       `yang:"choice"`
-	Config       *Value          `yang:"config"`
-	Container    []*Container    `yang:"container"`
-	Description  *Value          `yang:"description"`
-	Grouping     []*Grouping     `yang:"grouping"`
-	IfFeature    []*Value        `yang:"if-feature"`
-	Key          *Value          `yang:"key"`
-	Leaf         []*Leaf         `yang:"leaf"`
-	LeafList     []*LeafList     `yang:"leaf-list"`
-	List         []*List         `yang:"list"`
-	MaxElements  *Value          `yang:"max-elements"`
-	MinElements  *Value          `yang:"min-elements"`
-	Must         []*Must         `yang:"must"`
-	Notification []*Notification `yang:"notification"`
-	OrderedBy    *Value          `yang:"ordered-by"`
-	Reference    *Value          `yang:"reference"`
-	Status       *Value          `yang:"status"`
-	Typedef      []*Typedef      `yang:"typedef"`
-	Unique       []*Value        `yang:"unique"`
-	Uses         []*Uses         `yang:"uses"`
-	When         *Value          `yang:"when"`
+	Anydata     []*AnyData   `yang:"anydata"`
+	Action      []*Action    `yang:"action"`
+	Anyxml      []*AnyXML    `yang:"anyxml"`
+	Choice      []*Choice    `yang:"choice"`
+	Config      *Value       `yang:"config"`
+	Container   []*Container `yang:"container"`
+	Description *Value       `yang:"description"`
+	Grouping    []*Grouping  `yang:"grouping"`
+	IfFeature   []*Value     `yang:"if-feature"`
+	Key         *Value       `yang:"key"`
+	Leaf        []*Leaf      `yang:"leaf"`
+	LeafList    []*LeafList  `yang:"leaf-list"`
+	List        []*List      `yang:"list"`
+	MaxElements *Value       `yang:"max-elements"`
+	MinElements *Value       `yang:"min-elements"`
+	Must        []*Must      `yang:"must"`
+	OrderedBy   *Value       `yang:"ordered-by"`
+	Reference   *Value       `yang:"reference"`
+	Status      *Value       `yang:"status"`
+	Typedef     []*Typedef   `yang:"typedef"`
+	Unique      []*Value     `yang:"unique"`
+	Uses        []*Uses      `yang:"uses"`
+	When        *Value       `yang:"when"`
 }
 
 func (List) Kind() string              { return "list" }
@@ -599,21 +597,20 @@ type Grouping struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Anydata      []*AnyData      `yang:"anydata"`
-	Action       []*Action       `yang:"action"`
-	Anyxml       []*AnyXML       `yang:"anyxml"`
-	Choice       []*Choice       `yang:"choice"`
-	Container    []*Container    `yang:"container"`
-	Description  *Value          `yang:"description"`
-	Grouping     []*Grouping     `yang:"grouping"`
-	Leaf         []*Leaf         `yang:"leaf"`
-	LeafList     []*LeafList     `yang:"leaf-list"`
-	List         []*List         `yang:"list"`
-	Notification []*Notification `yang:"notification"`
-	Reference    *Value          `yang:"reference"`
-	Status       *Value          `yang:"status"`
-	Typedef      []*Typedef      `yang:"typedef"`
-	Uses         []*Uses         `yang:"uses"`
+	Anydata     []*AnyData   `yang:"anydata"`
+	Action      []*Action    `yang:"action"`
+	Anyxml      []*AnyXML    `yang:"anyxml"`
+	Choice      []*Choice    `yang:"choice"`
+	Container   []*Container `yang:"container"`
+	Description *Value       `yang:"description"`
+	Grouping    []*Grouping  `yang:"grouping"`
+	Leaf        []*Leaf      `yang:"leaf"`
+	LeafList    []*LeafList  `yang:"leaf-list"`
+	List        []*List      `yang:"list"`
+	Reference   *Value       `yang:"reference"`
+	Status      *Value       `yang:"status"`
+	Typedef     []*Typedef   `yang:"typedef"`
+	Uses        []*Uses      `yang:"uses"`
 }
 
 func (Grouping) Kind() string              { return "grouping" }
@@ -653,16 +650,15 @@ type Refine struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Default     *Value   `yang:"default"`
-	Description *Value   `yang:"description"`
-	IfFeature   []*Value `yang:"if-feature"`
-	Reference   *Value   `yang:"reference"`
-	Config      *Value   `yang:"config"`
-	Mandatory   *Value   `yang:"mandatory"`
-	Presence    *Value   `yang:"presence"`
-	Must        []*Must  `yang:"must"`
-	MaxElements *Value   `yang:"max-elements"`
-	MinElements *Value   `yang:"min-elements"`
+	Default     *Value  `yang:"default"`
+	Description *Value  `yang:"description"`
+	Reference   *Value  `yang:"reference"`
+	Config      *Value  `yang:"config"`
+	Mandatory   *Value  `yang:"mandatory"`
+	Presence    *Value  `yang:"presence"`
+	Must        []*Must `yang:"must"`
+	MaxElements *Value  `yang:"max-elements"`
+	MinElements *Value  `yang:"min-elements"`
 }
 
 func (Refine) Kind() string             { return "refine" }
@@ -789,22 +785,21 @@ type Augment struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Anydata      []*AnyData      `yang:"anydata"`
-	Action       []*Action       `yang:"action"`
-	Anyxml       []*AnyXML       `yang:"anyxml"`
-	Case         []*Case         `yang:"case"`
-	Choice       []*Choice       `yang:"choice"`
-	Container    []*Container    `yang:"container"`
-	Description  *Value          `yang:"description"`
-	IfFeature    []*Value        `yang:"if-feature"`
-	Leaf         []*Leaf         `yang:"leaf"`
-	LeafList     []*LeafList     `yang:"leaf-list"`
-	List         []*List         `yang:"list"`
-	Notification []*Notification `yang:"notification"`
-	Reference    *Value          `yang:"reference"`
-	Status       *Value          `yang:"status"`
-	Uses         []*Uses         `yang:"uses"`
-	When         *Value          `yang:"when"`
+	Anydata     []*AnyData   `yang:"anydata"`
+	Action      []*Action    `yang:"action"`
+	Anyxml      []*AnyXML    `yang:"anyxml"`
+	Case        []*Case      `yang:"case"`
+	Choice      []*Choice    `yang:"choice"`
+	Container   []*Container `yang:"container"`
+	Description *Value       `yang:"description"`
+	IfFeature   []*Value     `yang:"if-feature"`
+	Leaf        []*Leaf      `yang:"leaf"`
+	LeafList    []*LeafList  `yang:"leaf-list"`
+	List        []*List      `yang:"list"`
+	Reference   *Value       `yang:"reference"`
+	Status      *Value       `yang:"status"`
+	Uses        []*Uses      `yang:"uses"`
+	When        *Value       `yang:"when"`
 }
 
 func (Augment) Kind() string             { return "augment" }
@@ -822,7 +817,6 @@ type Identity struct {
 
 	Base        *Value      `yang:"base" json:"-"`
 	Description *Value      `yang:"description" json:"-"`
-	IfFeature   []*Value    `yang:"if-feature" json:"-"`
 	Reference   *Value      `yang:"reference" json:"-"`
 	Status      *Value      `yang:"status" json:"-"`
 	Values      []*Identity `json:",omitempty"`
@@ -979,11 +973,10 @@ type Enum struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Description *Value   `yang:"description"`
-	IfFeature   []*Value `yang:"if-feature"`
-	Reference   *Value   `yang:"reference"`
-	Status      *Value   `yang:"status"`
-	Value       *Value   `yang:"value"`
+	Description *Value `yang:"description"`
+	Reference   *Value `yang:"reference"`
+	Status      *Value `yang:"status"`
+	Value       *Value `yang:"value"`
 }
 
 func (Enum) Kind() string             { return "enum" }
@@ -999,11 +992,10 @@ type Bit struct {
 	Parent     Node         `yang:"Parent,nomerge"`
 	Extensions []*Statement `yang:"Ext"`
 
-	Description *Value   `yang:"description"`
-	IfFeature   []*Value `yang:"if-feature"`
-	Reference   *Value   `yang:"reference"`
-	Status      *Value   `yang:"status"`
-	Position    *Value   `yang:"position"`
+	Description *Value `yang:"description"`
+	Reference   *Value `yang:"reference"`
+	Status      *Value `yang:"status"`
+	Position    *Value `yang:"position"`
 }
 
 func (Bit) Kind() string             { return "bit" }


Test patch:
diff --git a/pkg/yang/entry_test.go b/pkg/yang/entry_test.go
index 4743ac1..0ada64d 100644
--- a/pkg/yang/entry_test.go
+++ b/pkg/yang/entry_test.go
@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
+	"math"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -118,6 +119,37 @@ module base {
 			`bad-augment.yang:6:3: augment erewhon not found`,
 		},
 	},
+	{
+		name: "bad-min-max-elements.yang",
+		in: `
+module base {
+  namespace "urn:mod";
+  prefix "base";
+  list foo {
+    // bad arguments to min-elements and max-elements
+    min-elements bar;
+    max-elements -5;
+  }
+  leaf-list bar {
+    type string;
+    // bad arguments to min-elements and max-elements
+    min-elements unbounded;
+    max-elements 122222222222222222222222222222222222222222222222222222222222;
+  }
+  list baz {
+    // good arguments
+    min-elements 0;
+    max-elements unbounded;
+  }
+}
+`,
+		errors: []string{
+			`bad-min-max-elements.yang:7:5: invalid min-elements value`,
+			`bad-min-max-elements.yang:8:5: invalid max-elements value`,
+			`bad-min-max-elements.yang:13:5: invalid min-elements value`,
+			`bad-min-max-elements.yang:14:5: invalid max-elements value`,
+		},
+	},
 }
 
 func TestBadYang(t *testing.T) {
@@ -133,7 +165,7 @@ func TestBadYang(t *testing.T) {
 		} else {
 			ok := true
 			for x, err := range errs {
-				if err.Error() != tt.errors[x] {
+				if !strings.Contains(err.Error(), tt.errors[x]) {
 					ok = false
 					break
 				}
@@ -1801,339 +1833,6 @@ func TestActionRPC(t *testing.T) {
 	}
 }
 
-var testIfFeatureModules = []struct {
-	name string
-	in   string
-}{
-	{
-		name: "if-feature.yang",
-		in: `module if-feature {
-  namespace "urn:if-feature";
-  prefix "feat";
-
-  feature ft-container;
-  feature ft-action;
-  feature ft-anydata1;
-  feature ft-anydata2;
-  feature ft-anyxml;
-  feature ft-choice;
-  feature ft-case;
-  feature ft-feature;
-  feature ft-leaf;
-  feature ft-bit;
-  feature ft-leaf-list;
-  feature ft-enum;
-  feature ft-list;
-  feature ft-notification;
-  feature ft-rpc;
-  feature ft-augment;
-  feature ft-identity;
-  feature ft-uses;
-  feature ft-refine;
-
-  container cont {
-    if-feature ft-container;
-    action act {
-      if-feature ft-action;
-    }
-  }
-
-  anydata data {
-    if-feature ft-anydata1;
-    if-feature ft-anydata2;
-  }
-
-  anyxml xml {
-    if-feature ft-anyxml;
-  }
-
-  choice ch {
-    if-feature ft-choice;
-    case cs {
-      if-feature ft-case;
-    }
-  }
-
-  feature f {
-    if-feature ft-feature;
-  }
-
-  leaf l {
-    if-feature ft-leaf;
-    type bits {
-      bit A {
-        if-feature ft-bit;
-      }
-    }
-  }
-
-  leaf-list ll {
-    if-feature ft-leaf-list;
-    type enumeration {
-      enum zero {
-        if-feature ft-enum;
-      }
-    }
-  }
-
-  list ls {
-    if-feature ft-list;
-  }
-
-  notification n {
-    if-feature ft-notification;
-  }
-
-  rpc r {
-    if-feature ft-rpc;
-  }
-
-  augment "/cont" {
-    if-feature ft-augment;
-  }
-
-  identity id {
-    if-feature ft-identity;
-  }
-
-  uses g {
-    if-feature ft-uses;
-    refine rf {
-      if-feature ft-refine;
-    }
-  }
-  grouping g {}
-}
-`,
-	},
-}
-
-func TestIfFeature(t *testing.T) {
-	entryIfFeatures := func(e *Entry) []*Value {
-		extra := e.Extra["if-feature"]
-		if len(extra) == 0 {
-			return nil
-		}
-		return extra[0].([]*Value)
-	}
-
-	featureByName := func(e *Entry, name string) *Feature {
-		for _, f := range e.Extra["feature"][0].([]*Feature) {
-			if f.Name == name {
-				return f
-			}
-		}
-		return nil
-	}
-
-	ms := NewModules()
-	for _, tt := range testIfFeatureModules {
-		if err := ms.Parse(tt.in, tt.name); err != nil {
-			t.Fatalf("could not parse module %s: %v", tt.name, err)
-		}
-	}
-
-	if errs := ms.Process(); len(errs) > 0 {
-		t.Fatalf("could not process modules: %v", errs)
-	}
-
-	mod, _ := ms.GetModule("if-feature")
-
-	testcases := []struct {
-		name           string
-		inIfFeatures   []*Value
-		wantIfFeatures []string
-	}{
-		// Node statements
-		{
-			name:           "action",
-			inIfFeatures:   entryIfFeatures(mod.Dir["cont"].Dir["act"]),
-			wantIfFeatures: []string{"ft-action"},
-		},
-		{
-			name:           "anydata",
-			inIfFeatures:   entryIfFeatures(mod.Dir["data"]),
-			wantIfFeatures: []string{"ft-anydata1", "ft-anydata2"},
-		},
-		{
-			name:           "anyxml",
-			inIfFeatures:   entryIfFeatures(mod.Dir["xml"]),
-			wantIfFeatures: []string{"ft-anyxml"},
-		},
-		{
-			name:           "case",
-			inIfFeatures:   entryIfFeatures(mod.Dir["ch"].Dir["cs"]),
-			wantIfFeatures: []string{"ft-case"},
-		},
-		{
-			name:           "choice",
-			inIfFeatures:   entryIfFeatures(mod.Dir["ch"]),
-			wantIfFeatures: []string{"ft-choice"},
-		},
-		{
-			name:           "container",
-			inIfFeatures:   entryIfFeatures(mod.Dir["cont"]),
-			wantIfFeatures: []string{"ft-container"},
-		},
-		{
-			name:           "feature",
-			inIfFeatures:   featureByName(mod, "f").IfFeature,
-			wantIfFeatures: []string{"ft-feature"},
-		},
-		{
-			name:           "leaf",
-			inIfFeatures:   entryIfFeatures(mod.Dir["l"]),
-			wantIfFeatures: []string{"ft-leaf"},
-		},
-		{
-			name:           "leaf-list",
-			inIfFeatures:   entryIfFeatures(mod.Dir["ll"]),
-			wantIfFeatures: []string{"ft-leaf-list"},
-		},
-		{
-			name:           "list",
-			inIfFeatures:   entryIfFeatures(mod.Dir["ls"]),
-			wantIfFeatures: []string{"ft-list"},
-		},
-		{
-			name:           "notification",
-			inIfFeatures:   entryIfFeatures(mod.Dir["n"]),
-			wantIfFeatures: []string{"ft-notification"},
-		},
-		{
-			name:           "rpc",
-			inIfFeatures:   entryIfFeatures(mod.Dir["r"]),
-			wantIfFeatures: []string{"ft-rpc"},
-		},
-		// Other statements
-		{
-			name:           "augment",
-			inIfFeatures:   entryIfFeatures(mod.Dir["cont"].Augmented[0]),
-			wantIfFeatures: []string{"ft-augment"},
-		},
-		{
-			name:           "bit",
-			inIfFeatures:   mod.Dir["l"].Node.(*Leaf).Type.Bit[0].IfFeature,
-			wantIfFeatures: []string{"ft-bit"},
-		},
-		{
-			name:           "enum",
-			inIfFeatures:   mod.Dir["ll"].Node.(*Leaf).Type.Enum[0].IfFeature,
-			wantIfFeatures: []string{"ft-enum"},
-		},
-		{
-			name:           "identity",
-			inIfFeatures:   mod.Identities[0].IfFeature,
-			wantIfFeatures: []string{"ft-identity"},
-		},
-		{
-			name:           "refine",
-			inIfFeatures:   ms.Modules["if-feature"].Uses[0].Refine[0].IfFeature,
-			wantIfFeatures: []string{"ft-refine"},
-		},
-		{
-			name:           "uses",
-			inIfFeatures:   ms.Modules["if-feature"].Uses[0].IfFeature,
-			wantIfFeatures: []string{"ft-uses"},
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			var names []string
-			for _, f := range tc.inIfFeatures {
-				names = append(names, f.Name)
-			}
-
-			if !reflect.DeepEqual(names, tc.wantIfFeatures) {
-				t.Errorf("%s: did not get expected if-features, got %v, want %v", tc.name, names, tc.wantIfFeatures)
-			}
-		})
-	}
-}
-
-var testNotificationModules = []struct {
-	name string
-	in   string
-}{
-	{
-		name: "notification.yang",
-		in: `module notification {
-  namespace "urn:notification";
-  prefix "n";
-
-  notification n {}
-
-  grouping g {
-    notification g-n {}
-  }
-
-  container cont {
-    notification cont-n {}
-  }
-
-  list ls {
-    notification ls-n {}
-    uses g;
-  }
-
-  augment "/cont" {
-    notification aug-n {}
-  }
-}
-`,
-	},
-}
-
-func TestNotification(t *testing.T) {
-	ms := NewModules()
-	for _, tt := range testNotificationModules {
-		if err := ms.Parse(tt.in, tt.name); err != nil {
-			t.Fatalf("could not parse module %s: %v", tt.name, err)
-		}
-	}
-
-	if errs := ms.Process(); len(errs) > 0 {
-		t.Fatalf("could not process modules: %v", errs)
-	}
-
-	mod, _ := ms.GetModule("notification")
-
-	testcases := []struct {
-		name     string
-		wantPath []string
-	}{
-		{
-			name:     "module",
-			wantPath: []string{"n"},
-		},
-		{
-			name:     "container",
-			wantPath: []string{"cont", "cont-n"},
-		},
-		{
-			name:     "list",
-			wantPath: []string{"ls", "ls-n"},
-		},
-		{
-			name:     "grouping",
-			wantPath: []string{"ls", "g-n"},
-		},
-		{
-			name:     "augment",
-			wantPath: []string{"cont", "aug-n"},
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			if e := getEntry(mod, tc.wantPath); e == nil || e.Node.Kind() != "notification" {
-				t.Errorf("%s: want notification entry at: %v, got: %+v", tc.name, tc.wantPath, e)
-			}
-		})
-	}
-}
-
 // addTreeE takes an input Entry and appends it to a directory, keyed by path, to the Entry.
 // If the Entry has children, they are appended to the directory recursively. Used in test
 // cases where a path is to be referred to.
@@ -2327,7 +2026,7 @@ func TestEntryTypes(t *testing.T) {
 
 	leafListSchema := &Entry{
 		Kind:     LeafEntry,
-		ListAttr: &ListAttr{MinElements: &Value{Name: "0"}},
+		ListAttr: &ListAttr{MinElements: 0},
 		Type:     &YangType{Kind: Ystring},
 		Name:     "leaf-list-schema",
 	}
@@ -2335,7 +2034,7 @@ func TestEntryTypes(t *testing.T) {
 	listSchema := &Entry{
 		Name:     "list-schema",
 		Kind:     DirectoryEntry,
-		ListAttr: &ListAttr{MinElements: &Value{Name: "0"}},
+		ListAttr: &ListAttr{MinElements: 0},
 		Dir: map[string]*Entry{
 			"leaf-name": {
 				Kind: LeafEntry,
@@ -2674,22 +2373,32 @@ func TestDeviation(t *testing.T) {
 				path: "/target/add/min-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MinElements: &Value{Name: "42"},
+						MinElements: 42,
+					},
+					deviatePresence: deviationPresence{
+						hasMinElements: true,
 					},
 				},
 			}, {
 				path: "/target/add/max-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MaxElements: &Value{Name: "42"},
+						MaxElements: 42,
+					},
+					deviatePresence: deviationPresence{
+						hasMaxElements: true,
 					},
 				},
 			}, {
 				path: "/target/add/max-and-min-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MinElements: &Value{Name: "42"},
-						MaxElements: &Value{Name: "42"},
+						MinElements: 42,
+						MaxElements: 42,
+					},
+					deviatePresence: deviationPresence{
+						hasMinElements: true,
+						hasMaxElements: true,
 					},
 				},
 			}, {
@@ -2735,6 +2444,42 @@ func TestDeviation(t *testing.T) {
 				}`,
 		},
 		wantProcessErrSubstring: "tried to deviate min-elements on a non-list type",
+	}, {
+		desc: "error case - deviation delete max-element on non-list",
+		inFiles: map[string]string{
+			"deviate": `
+				module deviate {
+					prefix "d";
+					namespace "urn:d";
+
+					leaf a { type string; }
+
+					deviation /a {
+						deviate delete {
+							max-elements 42;
+						}
+					}
+				}`,
+		},
+		wantProcessErrSubstring: "tried to deviate max-elements on a non-list type",
+	}, {
+		desc: "error case - deviation delete min elements on non-list",
+		inFiles: map[string]string{
+			"deviate": `
+				module deviate {
+					prefix "d";
+					namespace "urn:d";
+
+					leaf a { type string; }
+
+					deviation /a {
+						deviate delete {
+							min-elements 42;
+						}
+					}
+				}`,
+		},
+		wantProcessErrSubstring: "tried to deviate min-elements on a non-list type",
 	}, {
 		desc:    "deviation - not supported",
 		inFiles: map[string]string{"deviate": mustReadFile(filepath.Join("testdata", "deviate-notsupported.yang"))},
@@ -2816,22 +2561,32 @@ func TestDeviation(t *testing.T) {
 				path: "/target/replace/min-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MinElements: &Value{Name: "42"},
+						MinElements: 42,
+					},
+					deviatePresence: deviationPresence{
+						hasMinElements: true,
 					},
 				},
 			}, {
 				path: "/target/replace/max-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MaxElements: &Value{Name: "42"},
+						MaxElements: 42,
+					},
+					deviatePresence: deviationPresence{
+						hasMaxElements: true,
 					},
 				},
 			}, {
 				path: "/target/replace/max-and-min-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MinElements: &Value{Name: "42"},
-						MaxElements: &Value{Name: "42"},
+						MinElements: 42,
+						MaxElements: 42,
+					},
+					deviatePresence: deviationPresence{
+						hasMinElements: true,
+						hasMaxElements: true,
 					},
 				},
 			}, {
@@ -2867,22 +2622,32 @@ func TestDeviation(t *testing.T) {
 				path: "/target/delete/min-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MinElements: nil,
+						MinElements: 0,
+					},
+					deviatePresence: deviationPresence{
+						hasMinElements: true,
 					},
 				},
 			}, {
 				path: "/target/delete/max-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MaxElements: nil,
+						MaxElements: math.MaxUint64,
+					},
+					deviatePresence: deviationPresence{
+						hasMaxElements: true,
 					},
 				},
 			}, {
 				path: "/target/delete/max-and-min-elements",
 				entry: &Entry{
 					ListAttr: &ListAttr{
-						MinElements: nil,
-						MaxElements: nil,
+						MinElements: 0,
+						MaxElements: math.MaxUint64,
+					},
+					deviatePresence: deviationPresence{
+						hasMinElements: true,
+						hasMaxElements: true,
 					},
 				},
 			}, {
@@ -2892,6 +2657,45 @@ func TestDeviation(t *testing.T) {
 				},
 			}},
 		},
+	}, {
+		desc: "error case - deviation delete of min-elements has different keyword value",
+		inFiles: map[string]string{
+			"deviate": `
+				module deviate {
+					prefix "d";
+					namespace "urn:d";
+
+					leaf-list a { type string; }
+
+					deviation /a {
+						deviate delete {
+							min-elements 42;
+						}
+					}
+				}`,
+		},
+		wantProcessErrSubstring: "differs from deviation's min-element value",
+	}, {
+		desc: "error case - deviation delete of max-elements has different keyword value",
+		inFiles: map[string]string{
+			"deviate": `
+				module deviate {
+					prefix "d";
+					namespace "urn:d";
+
+					leaf-list a {
+						type string;
+						max-elements 100;
+					}
+
+					deviation /a {
+						deviate delete {
+							max-elements 42;
+						}
+					}
+				}`,
+		},
+		wantProcessErrSubstring: "differs from deviation's max-element value",
 	}, {
 		desc: "deviation using locally defined typedef",
 		inFiles: map[string]string{
@@ -3042,9 +2846,14 @@ func TestDeviation(t *testing.T) {
 							t.Errorf("%d (%s): listattr was nil for an entry expected to be a list at %s", idx, want.path, want.path)
 							continue
 						}
-						if want.entry.ListAttr.MinElements != nil {
-							if gotn, wantn := got.ListAttr.MinElements.Name, want.entry.ListAttr.MinElements.Name; gotn != wantn {
-								t.Errorf("%d (%s): min-elements, got: %v, want: %v", idx, want.path, gotn, wantn)
+						if want.entry.deviatePresence.hasMinElements {
+							if gotMin, wantMin := got.ListAttr.MinElements, want.entry.ListAttr.MinElements; gotMin != wantMin {
+								t.Errorf("%d (%s): min-elements, got: %v, want: %v", idx, want.path, gotMin, wantMin)
+							}
+						}
+						if want.entry.deviatePresence.hasMaxElements {
+							if gotMax, wantMax := got.ListAttr.MaxElements, want.entry.ListAttr.MaxElements; gotMax != wantMax {
+								t.Errorf("%d (%s): max-elements, got: %v, want: %v", idx, want.path, gotMax, wantMax)
 							}
 						}
 					}
diff --git a/pkg/yang/marshal_test.go b/pkg/yang/marshal_test.go
index 12e8890..015f900 100644
--- a/pkg/yang/marshal_test.go
+++ b/pkg/yang/marshal_test.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/kylelemons/godebug/pretty"
 )
 
@@ -302,17 +303,16 @@ func TestMarshalJSON(t *testing.T) {
 					Kind: LeafEntry,
 				},
 				"leaf-list": {
-					Name:     "leaf-list",
-					ListAttr: &ListAttr{},
+					Name: "leaf-list",
+					ListAttr: &ListAttr{
+						MaxElements: 18446744073709551615,
+						MinElements: 0,
+					},
 				},
 			},
 			ListAttr: &ListAttr{
-				MaxElements: &Value{
-					Name: "42",
-				},
-				MinElements: &Value{
-					Name: "48",
-				},
+				MaxElements: 42,
+				MinElements: 48,
 			},
 			Identities: []*Identity{{
 				Name: "ID_ONE",
@@ -338,8 +338,8 @@ func TestMarshalJSON(t *testing.T) {
       "Kind": 0,
       "Config": 0,
       "ListAttr": {
-        "MinElements": null,
-        "MaxElements": null,
+        "MinElements": 0,
+        "MaxElements": 18446744073709551615,
         "OrderedBy": null
       }
     }
@@ -352,12 +352,8 @@ func TestMarshalJSON(t *testing.T) {
     }
   ],
   "ListAttr": {
-    "MinElements": {
-      "Name": "48"
-    },
-    "MaxElements": {
-      "Name": "42"
-    },
+    "MinElements": 48,
+    "MaxElements": 42,
     "OrderedBy": null
   },
   "Identities": [
@@ -408,6 +404,8 @@ func TestParseAndMarshal(t *testing.T) {
 											container test {
 												list a {
 													key "k";
+													min-elements 10;
+													max-elements "unbounded";
 													leaf k { type string; }
 
 													leaf bar {
@@ -425,6 +423,11 @@ func TestParseAndMarshal(t *testing.T) {
 													type string;
 												}
 
+												leaf-list zip2 {
+													max-elements 1000;
+													type string;
+												}
+
 												leaf x {
 													type union {
 														type string;
@@ -527,8 +530,8 @@ func TestParseAndMarshal(t *testing.T) {
           },
           "Key": "k",
           "ListAttr": {
-            "MinElements": null,
-            "MaxElements": null,
+            "MinElements": 10,
+            "MaxElements": 18446744073709551615,
             "OrderedBy": null
           }
         },
@@ -616,8 +619,30 @@ func TestParseAndMarshal(t *testing.T) {
             "Kind": 18
           },
           "ListAttr": {
-            "MinElements": null,
-            "MaxElements": null,
+            "MinElements": 0,
+            "MaxElements": 18446744073709551615,
+            "OrderedBy": null
+          }
+        },
+        "zip2": {
+          "Name": "zip2",
+          "Kind": 0,
+          "Config": 0,
+          "Prefix": {
+            "Name": "t",
+            "Source": {
+              "Keyword": "prefix",
+              "HasArgument": true,
+              "Argument": "t"
+            }
+          },
+          "Type": {
+            "Name": "string",
+            "Kind": 18
+          },
+          "ListAttr": {
+            "MinElements": 0,
+            "MaxElements": 1000,
             "OrderedBy": null
           }
         }
@@ -746,7 +771,7 @@ func TestParseAndMarshal(t *testing.T) {
 						continue
 					}
 
-					if diff := pretty.Compare(string(got), tt.want[m.Name]); diff != "" {
+					if diff := cmp.Diff(string(got), tt.want[m.Name]); diff != "" {
 						t.Errorf("%s: json.MarshalIndent(...): did not get expected JSON, diff(-got,+want):\n%s", tt.name, diff)
 					}
 				}
diff --git a/pkg/yang/testdata/deviate-delete.yang b/pkg/yang/testdata/deviate-delete.yang
index 7ce1caa..8e86466 100644
--- a/pkg/yang/testdata/deviate-delete.yang
+++ b/pkg/yang/testdata/deviate-delete.yang
@@ -86,4 +86,4 @@ module deviate {
             units "nanofish per millenium";
         }
     }
-}
\ No newline at end of file
+}