From 0e5b9da950fdba73fea2c577fa7f640a7fa594a7 Mon Sep 17 00:00:00 2001 From: wenovus Date: Thu, 18 Jun 2020 17:14:11 -0700 Subject: [PATCH 1/6] 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. --- pkg/yang/entry.go | 111 +++++++++++++++++--- pkg/yang/entry_test.go | 145 +++++++++++++++++++++++--- pkg/yang/marshal_test.go | 67 ++++++++---- pkg/yang/testdata/deviate-delete.yang | 2 +- 4 files changed, 271 insertions(+), 54 deletions(-) diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go index dfd4525..cd1376a 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 value (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(`invalid max-elements value %q (expect "unbounded" or a non-negative integer): %v`, v.Name, err) + } + return val, nil +} + +// semCheckMaxElements 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(`invalid min-elements value %q (expect a non-negative integer): %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. @@ -562,9 +598,14 @@ 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(fmt.Errorf("%s: %v", Source(n), err)) + } + if e.ListAttr.MinElements, err = semCheckMinElements(s.MinElements); err != nil { + e.addError(fmt.Errorf("%s: %v", Source(n), err)) } e.Prefix = getRootPrefix(e) return e @@ -588,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(fmt.Errorf("%s: %v", Source(n), err)) + } + if e.ListAttr.MinElements, err = semCheckMinElements(s.MinElements); err != nil { + e.addError(fmt.Errorf("%s: %v", Source(n), err)) } case *Choice: e.Kind = ChoiceEntry @@ -880,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(fmt.Errorf("%s: %v", Source(n), err)) + } + } else { + e.deviatePresence.hasMinElements = true + if e.ListAttr.MinElements, err = semCheckMinElements(v); err != nil { + e.addError(fmt.Errorf("%s: %v", Source(n), err)) + } + } } case "units": v, ok := fv.Interface().(*Value) @@ -1006,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 @@ -1014,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 @@ -1049,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/entry_test.go b/pkg/yang/entry_test.go index 1420d64..2125b02 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" @@ -1994,7 +1995,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", } @@ -2002,7 +2003,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, @@ -2341,22 +2342,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, }, }, }, { @@ -2402,6 +2413,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"))}, @@ -2483,22 +2530,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, }, }, }, { @@ -2534,22 +2591,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, }, }, }, { @@ -2559,6 +2626,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{ @@ -2709,9 +2815,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 +} From 89451ca92a1578c0e7c150bb08ead95b049aa29e Mon Sep 17 00:00:00 2001 From: wenovus Date: Thu, 18 Jun 2020 17:27:09 -0700 Subject: [PATCH 2/6] style fix --- pkg/yang/entry.go | 2 +- pkg/yang/testdata/deviate-delete.yang | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go index cd1376a..1c5194d 100644 --- a/pkg/yang/entry.go +++ b/pkg/yang/entry.go @@ -65,7 +65,7 @@ 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 value (e.g. non-pointers). +// doesn't have a presence indicator (e.g. non-pointers). type deviationPresence struct { hasMinElements bool hasMaxElements bool diff --git a/pkg/yang/testdata/deviate-delete.yang b/pkg/yang/testdata/deviate-delete.yang index 8e86466..1bb4cb7 100644 --- a/pkg/yang/testdata/deviate-delete.yang +++ b/pkg/yang/testdata/deviate-delete.yang @@ -87,3 +87,4 @@ module deviate { } } } + From 165a0aa9f8ae267fac4db8292d4a5411672e0e3b Mon Sep 17 00:00:00 2001 From: wenovus Date: Thu, 18 Jun 2020 17:27:48 -0700 Subject: [PATCH 3/6] style fix --- pkg/yang/testdata/deviate-delete.yang | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/yang/testdata/deviate-delete.yang b/pkg/yang/testdata/deviate-delete.yang index 1bb4cb7..8e86466 100644 --- a/pkg/yang/testdata/deviate-delete.yang +++ b/pkg/yang/testdata/deviate-delete.yang @@ -87,4 +87,3 @@ module deviate { } } } - From b10a87a2c89f68a8bec49ce589fd1b0ac011c99d Mon Sep 17 00:00:00 2001 From: wenovus Date: Thu, 18 Jun 2020 17:51:54 -0700 Subject: [PATCH 4/6] Add missing coverage --- pkg/yang/entry.go | 16 ++++++++-------- pkg/yang/entry_test.go | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go index 1c5194d..1900608 100644 --- a/pkg/yang/entry.go +++ b/pkg/yang/entry.go @@ -492,7 +492,7 @@ func semCheckMaxElements(v *Value) (uint64, error) { } val, err := strconv.ParseUint(v.Name, 10, 64) if err != nil { - return val, fmt.Errorf(`invalid max-elements value %q (expect "unbounded" or a non-negative integer): %v`, v.Name, err) + 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 } @@ -504,7 +504,7 @@ func semCheckMinElements(v *Value) (uint64, error) { } val, err := strconv.ParseUint(v.Name, 10, 64) if err != nil { - return val, fmt.Errorf(`invalid min-elements value %q (expect a non-negative integer): %v`, v.Name, err) + return val, fmt.Errorf(`%s: invalid min-elements value %q (expect a non-negative integer): %v`, Source(v), v.Name, err) } return val, nil } @@ -602,10 +602,10 @@ func ToEntry(n Node) (e *Entry) { } var err error if e.ListAttr.MaxElements, err = semCheckMaxElements(s.MaxElements); err != nil { - e.addError(fmt.Errorf("%s: %v", Source(n), err)) + e.addError(err) } if e.ListAttr.MinElements, err = semCheckMinElements(s.MinElements); err != nil { - e.addError(fmt.Errorf("%s: %v", Source(n), err)) + e.addError(err) } e.Prefix = getRootPrefix(e) return e @@ -633,10 +633,10 @@ func ToEntry(n Node) (e *Entry) { } var err error if e.ListAttr.MaxElements, err = semCheckMaxElements(s.MaxElements); err != nil { - e.addError(fmt.Errorf("%s: %v", Source(n), err)) + e.addError(err) } if e.ListAttr.MinElements, err = semCheckMinElements(s.MinElements); err != nil { - e.addError(fmt.Errorf("%s: %v", Source(n), err)) + e.addError(err) } case *Choice: e.Kind = ChoiceEntry @@ -932,12 +932,12 @@ func ToEntry(n Node) (e *Entry) { if name == "max-elements" { e.deviatePresence.hasMaxElements = true if e.ListAttr.MaxElements, err = semCheckMaxElements(v); err != nil { - e.addError(fmt.Errorf("%s: %v", Source(n), err)) + e.addError(err) } } else { e.deviatePresence.hasMinElements = true if e.ListAttr.MinElements, err = semCheckMinElements(v); err != nil { - e.addError(fmt.Errorf("%s: %v", Source(n), err)) + e.addError(err) } } } diff --git a/pkg/yang/entry_test.go b/pkg/yang/entry_test.go index 2125b02..0ada64d 100644 --- a/pkg/yang/entry_test.go +++ b/pkg/yang/entry_test.go @@ -119,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) { @@ -134,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 } From 1c23000593a71635118e262d94db9b272b4ea60f Mon Sep 17 00:00:00 2001 From: wenovus Date: Fri, 19 Jun 2020 10:56:49 -0700 Subject: [PATCH 5/6] add staticcheck --- .github/workflows/go.yml | 7 ++++++- pkg/yang/entry.go | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 7cd25bb..0ec881c 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -39,7 +39,12 @@ jobs: - name: Go vet run: go vet ./... - + + - name: Staticcheck + run: | + go get -u honnef.co/go/tools/cmd/staticcheck + staticcheck ./... + - name: Send Coverage uses: shogo82148/actions-goveralls@v1 with: diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go index 1900608..41da2b0 100644 --- a/pkg/yang/entry.go +++ b/pkg/yang/entry.go @@ -497,7 +497,7 @@ func semCheckMaxElements(v *Value) (uint64, error) { return val, nil } -// semCheckMaxElements checks whether the min-element argument is valid, and returns the specified value. +// 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 From e39889f3db428da23c0217617929cc128374a646 Mon Sep 17 00:00:00 2001 From: wenovus Date: Fri, 19 Jun 2020 11:11:19 -0700 Subject: [PATCH 6/6] Remove staticcheck --- .github/workflows/go.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 0ec881c..7cd25bb 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -39,12 +39,7 @@ jobs: - name: Go vet run: go vet ./... - - - name: Staticcheck - run: | - go get -u honnef.co/go/tools/cmd/staticcheck - staticcheck ./... - + - name: Send Coverage uses: shogo82148/actions-goveralls@v1 with: