From cdf859ab29a06de8ed5b0bd1a8889e7df1bbf1ea Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 4 Nov 2024 15:47:21 -0600 Subject: [PATCH 1/5] add functions to replicate old behavior --- decoder.go | 37 ++++++++++++++++++---- decoder_test.go | 50 ++++++++++++++++++++++++++++++ hcl/parser/parser.go | 30 ++++++++++++------ hcl/parser/parser_test.go | 41 +++++++++++++++--------- parse.go | 11 ++++--- test-fixtures/basic_duplicates.hcl | 2 ++ 6 files changed, 136 insertions(+), 35 deletions(-) create mode 100644 test-fixtures/basic_duplicates.hcl diff --git a/decoder.go b/decoder.go index 1e4fb972b..5d3cbe27b 100644 --- a/decoder.go +++ b/decoder.go @@ -24,7 +24,18 @@ var ( // Unmarshal accepts a byte slice as input and writes the // data to the value pointed to by v. func Unmarshal(bs []byte, v interface{}) error { - root, err := parse(bs) + root, err := parse(bs, true) + if err != nil { + return err + } + + return DecodeObject(v, root) +} + +// Unmarshal accepts a byte slice as input and writes the +// data to the value pointed to by v. +func UnmarshalDontErrorOnDuplicates(bs []byte, v interface{}) error { + root, err := parse(bs, false) if err != nil { return err } @@ -35,7 +46,20 @@ func Unmarshal(bs []byte, v interface{}) error { // Decode reads the given input and decodes it into the structure // given by `out`. func Decode(out interface{}, in string) error { - obj, err := Parse(in) + panic("what") + return decode(out, in, true) +} + +// Decode reads the given input and decodes it into the structure +// given by `out`. +func DecodeDontErrorOnDuplicates(out interface{}, in string) error { + return decode(out, in, false) +} + +// Decode reads the given input and decodes it into the structure +// given by `out`. +func decode(out interface{}, in string, errorOnDuplicateAtributes bool) error { + obj, err := parse([]byte(in), errorOnDuplicateAtributes) if err != nil { return err } @@ -393,14 +417,15 @@ func (d *decoder) decodeMap(name string, node ast.Node, result reflect.Value) er // Set the final map if we can set.Set(resultMap) + return nil } func (d *decoder) decodePtr(name string, node ast.Node, result reflect.Value) error { - // if pointer is not nil, decode into existing value - if !result.IsNil() { - return d.decode(name, node, result.Elem()) - } + // if pointer is not nil, decode into existing value + if !result.IsNil() { + return d.decode(name, node, result.Elem()) + } // Create an element of the concrete (non pointer) type and decode // into that. Then set the value of the pointer to this type. diff --git a/decoder_test.go b/decoder_test.go index e5dc1fa52..ea7a1dc9d 100644 --- a/decoder_test.go +++ b/decoder_test.go @@ -35,6 +35,11 @@ func TestDecode_interface(t *testing.T) { "foo-bar": "baz", }, }, + { + "basic_duplicates.hcl", + true, + nil, + }, { "empty.hcl", false, @@ -450,6 +455,51 @@ func TestDecode_interface(t *testing.T) { } } +func TestDecodeDontErrorOnDuplicates_interface(t *testing.T) { + cases := []struct { + File string + Err bool + Out interface{} + }{ + { + "basic_duplicates.hcl", + false, + map[string]interface{}{ + "foo": "${file(\"bing/bong.txt\")}", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.File, func(t *testing.T) { + d, err := ioutil.ReadFile(filepath.Join(fixtureDir, tc.File)) + if err != nil { + t.Fatalf("err: %s", err) + } + + var out interface{} + err = DecodeDontErrorOnDuplicates(&out, string(d)) + if (err != nil) != tc.Err { + t.Fatalf("Input: %s\n\nError: %s", tc.File, err) + } + + if !reflect.DeepEqual(out, tc.Out) { + t.Fatalf("Input: %s. Actual, Expected.\n\n%#v\n\n%#v", tc.File, out, tc.Out) + } + + var v interface{} + err = UnmarshalDontErrorOnDuplicates(d, &v) + if (err != nil) != tc.Err { + t.Fatalf("Input: %s\n\nError: %s", tc.File, err) + } + + if !reflect.DeepEqual(v, tc.Out) { + t.Fatalf("Input: %s. Actual, Expected.\n\n%#v\n\n%#v", tc.File, out, tc.Out) + } + }) + } +} + func TestDecode_interfaceInline(t *testing.T) { cases := []struct { Value string diff --git a/hcl/parser/parser.go b/hcl/parser/parser.go index 985ecef02..0f5d929c6 100644 --- a/hcl/parser/parser.go +++ b/hcl/parser/parser.go @@ -27,22 +27,35 @@ type Parser struct { enableTrace bool indent int n int // buffer size (max = 1) + + errorOnDuplicateKeys bool } -func newParser(src []byte) *Parser { +func newParser(src []byte, errorOnDuplicateKeys bool) *Parser { return &Parser{ - sc: scanner.New(src), + sc: scanner.New(src), + errorOnDuplicateKeys: errorOnDuplicateKeys, } } // Parse returns the fully parsed source and returns the abstract syntax tree. func Parse(src []byte) (*ast.File, error) { + return parse(src, true) +} + +// Parse returns the fully parsed source and returns the abstract syntax tree. +func ParseDontErrorOnDuplicateKeys(src []byte) (*ast.File, error) { + return parse(src, false) +} + +// Parse returns the fully parsed source and returns the abstract syntax tree. +func parse(src []byte, errorOnDuplicateKeys bool) (*ast.File, error) { // normalize all line endings // since the scanner and output only work with "\n" line endings, we may // end up with dangling "\r" characters in the parsed data. src = bytes.Replace(src, []byte("\r\n"), []byte("\n"), -1) - p := newParser(src) + p := newParser(src, errorOnDuplicateKeys) return p.Parse() } @@ -65,6 +78,7 @@ func (p *Parser) Parse() (*ast.File, error) { } f.Comments = p.comments + return f, nil } @@ -97,6 +111,9 @@ func (p *Parser) objectList(obj bool) (*ast.ObjectList, error) { if n.Assign.String() != "-" { for _, key := range n.Keys { + if !p.errorOnDuplicateKeys { + break + } _, ok := seenKeys[key.Token.Text] if ok { return nil, errors.New(fmt.Sprintf("The argument %q at %s was already set. Each argument can only be defined once", key.Token.Text, key.Token.Pos.String())) @@ -241,7 +258,6 @@ func (p *Parser) objectItem() (*ast.ObjectItem, error) { func (p *Parser) objectKey() ([]*ast.ObjectKey, error) { keyCount := 0 keys := make([]*ast.ObjectKey, 0) - seenKeys := map[string]struct{}{} for { tok := p.scan() @@ -285,12 +301,6 @@ func (p *Parser) objectKey() ([]*ast.ObjectKey, error) { // object return keys, err case token.IDENT, token.STRING: - _, ok := seenKeys[p.tok.Text] - if ok { - return nil, errors.New(fmt.Sprintf("The argument %q at %s was already set. Each argument can only be defined once", p.tok.Text, p.tok.Pos.String())) - } - seenKeys[p.tok.Text] = struct{}{} - keyCount++ keys = append(keys, &ast.ObjectKey{Token: p.tok}) case token.ILLEGAL: diff --git a/hcl/parser/parser_test.go b/hcl/parser/parser_test.go index 3e3d1f5c3..6638ed8f2 100644 --- a/hcl/parser/parser_test.go +++ b/hcl/parser/parser_test.go @@ -28,7 +28,7 @@ func TestType(t *testing.T) { } for _, l := range literals { - p := newParser([]byte(l.src)) + p := newParser([]byte(l.src), true) item, err := p.objectItem() if err != nil { t.Error(err) @@ -78,7 +78,7 @@ EOF } for _, l := range literals { - p := newParser([]byte(l.src)) + p := newParser([]byte(l.src), true) item, err := p.objectItem() if err != nil { t.Error(err) @@ -105,7 +105,7 @@ func TestListOfMaps(t *testing.T) { {key = "bar"}, {key = "baz", key2 = "qux"}, ]` - p := newParser([]byte(src)) + p := newParser([]byte(src), true) file, err := p.Parse() if err != nil { @@ -138,7 +138,7 @@ func TestDuplicateKeys_NotAllowed(t *testing.T) { src := `key = "value1" key = "value2" ` - p := newParser([]byte(src)) + p := newParser([]byte(src), true) _, err := p.Parse() if err == nil { @@ -151,11 +151,23 @@ func TestDuplicateKeys_NotAllowed(t *testing.T) { } } +func TestDuplicateKeys_DontErrorWhenErrorOnDuplicateKeysIsFalse(t *testing.T) { + src := `key = "value1" + key = "value2" + ` + p := newParser([]byte(src), false) + + _, err := p.Parse() + if err != nil { + t.Fatalf("Unexpected error " + err.Error()) + } +} + func TestDuplicateKeys_NotAllowedInBlock(t *testing.T) { src := `key = "value1" structkey "structname" { name = "test" name = "test2"} ` - p := newParser([]byte(src)) + p := newParser([]byte(src), true) _, err := p.Parse() if err == nil { @@ -170,9 +182,8 @@ func TestDuplicateKeys_NotAllowedInBlock(t *testing.T) { func TestDuplicateBlocks_allowed(t *testing.T) { src := `structkey { name = "test" } - structkey { name = "test" } - ` - p := newParser([]byte(src)) + structkey { name = "test" }` + p := newParser([]byte(src), true) _, err := p.Parse() if err != nil { @@ -186,7 +197,7 @@ func TestListOfMaps_requiresComma(t *testing.T) { {key = "bar"} {key = "baz"} ]` - p := newParser([]byte(src)) + p := newParser([]byte(src), true) _, err := p.Parse() if err == nil { @@ -216,7 +227,7 @@ func TestListType_leadComment(t *testing.T) { } for _, l := range literals { - p := newParser([]byte(l.src)) + p := newParser([]byte(l.src), true) item, err := p.objectItem() if err != nil { t.Fatal(err) @@ -267,7 +278,7 @@ func TestListType_lineComment(t *testing.T) { } for _, l := range literals { - p := newParser([]byte(l.src)) + p := newParser([]byte(l.src), true) item, err := p.objectItem() if err != nil { t.Fatal(err) @@ -356,7 +367,7 @@ func TestObjectType(t *testing.T) { for _, l := range literals { t.Logf("Source: %s", l.src) - p := newParser([]byte(l.src)) + p := newParser([]byte(l.src), true) // p.enableTrace = true item, err := p.objectItem() if err != nil { @@ -402,7 +413,7 @@ func TestObjectKey(t *testing.T) { } for _, k := range keys { - p := newParser([]byte(k.src)) + p := newParser([]byte(k.src), true) keys, err := p.objectKey() if err != nil { t.Fatal(err) @@ -426,7 +437,7 @@ func TestObjectKey(t *testing.T) { } for _, k := range errKeys { - p := newParser([]byte(k.src)) + p := newParser([]byte(k.src), true) _, err := p.objectKey() if err == nil { t.Errorf("case '%s' should give an error", k.src) @@ -445,7 +456,7 @@ func TestCommentGroup(t *testing.T) { for _, tc := range cases { t.Run(tc.src, func(t *testing.T) { - p := newParser([]byte(tc.src)) + p := newParser([]byte(tc.src), true) file, err := p.Parse() if err != nil { t.Fatalf("parse error: %s", err) diff --git a/parse.go b/parse.go index 1fca53c4c..f4cc1255e 100644 --- a/parse.go +++ b/parse.go @@ -12,17 +12,20 @@ import ( // // Input can be either JSON or HCL func ParseBytes(in []byte) (*ast.File, error) { - return parse(in) + return parse(in, true) } // ParseString accepts input as a string and returns ast tree. func ParseString(input string) (*ast.File, error) { - return parse([]byte(input)) + return parse([]byte(input), true) } -func parse(in []byte) (*ast.File, error) { +func parse(in []byte, errorOnDuplicateKeys bool) (*ast.File, error) { switch lexMode(in) { case lexModeHcl: + if !errorOnDuplicateKeys { + return hclParser.ParseDontErrorOnDuplicateKeys(in) + } return hclParser.Parse(in) case lexModeJson: return jsonParser.Parse(in) @@ -35,5 +38,5 @@ func parse(in []byte) (*ast.File, error) { // // The input format can be either HCL or JSON. func Parse(input string) (*ast.File, error) { - return parse([]byte(input)) + return parse([]byte(input), true) } diff --git a/test-fixtures/basic_duplicates.hcl b/test-fixtures/basic_duplicates.hcl new file mode 100644 index 000000000..fd70ed9c8 --- /dev/null +++ b/test-fixtures/basic_duplicates.hcl @@ -0,0 +1,2 @@ +foo = "bar" +foo = "${file("bing/bong.txt")}" From 782b78106edb860a0a4d9877671faf18d2da6712 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Tue, 5 Nov 2024 13:40:30 -0600 Subject: [PATCH 2/5] Update decoder.go --- decoder.go | 1 - 1 file changed, 1 deletion(-) diff --git a/decoder.go b/decoder.go index 5d3cbe27b..cd91a7e40 100644 --- a/decoder.go +++ b/decoder.go @@ -46,7 +46,6 @@ func UnmarshalDontErrorOnDuplicates(bs []byte, v interface{}) error { // Decode reads the given input and decodes it into the structure // given by `out`. func Decode(out interface{}, in string) error { - panic("what") return decode(out, in, true) } From fccb5644a3ed9dc1792670515c7155316aecde27 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Tue, 5 Nov 2024 13:45:33 -0600 Subject: [PATCH 3/5] reverse booleans --- decoder.go | 12 ++++++------ decoder_test.go | 18 +++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/decoder.go b/decoder.go index cd91a7e40..26b236442 100644 --- a/decoder.go +++ b/decoder.go @@ -24,7 +24,7 @@ var ( // Unmarshal accepts a byte slice as input and writes the // data to the value pointed to by v. func Unmarshal(bs []byte, v interface{}) error { - root, err := parse(bs, true) + root, err := parse(bs, false) if err != nil { return err } @@ -34,8 +34,8 @@ func Unmarshal(bs []byte, v interface{}) error { // Unmarshal accepts a byte slice as input and writes the // data to the value pointed to by v. -func UnmarshalDontErrorOnDuplicates(bs []byte, v interface{}) error { - root, err := parse(bs, false) +func UnmarshalErrorOnDuplicates(bs []byte, v interface{}) error { + root, err := parse(bs, true) if err != nil { return err } @@ -46,13 +46,13 @@ func UnmarshalDontErrorOnDuplicates(bs []byte, v interface{}) error { // Decode reads the given input and decodes it into the structure // given by `out`. func Decode(out interface{}, in string) error { - return decode(out, in, true) + return decode(out, in, false) } // Decode reads the given input and decodes it into the structure // given by `out`. -func DecodeDontErrorOnDuplicates(out interface{}, in string) error { - return decode(out, in, false) +func DecodeErrorOnDuplicates(out interface{}, in string) error { + return decode(out, in, true) } // Decode reads the given input and decodes it into the structure diff --git a/decoder_test.go b/decoder_test.go index ea7a1dc9d..806be0b4a 100644 --- a/decoder_test.go +++ b/decoder_test.go @@ -37,8 +37,10 @@ func TestDecode_interface(t *testing.T) { }, { "basic_duplicates.hcl", - true, - nil, + false, + map[string]interface{}{ + "foo": "${file(\"bing/bong.txt\")}", + }, }, { "empty.hcl", @@ -455,7 +457,7 @@ func TestDecode_interface(t *testing.T) { } } -func TestDecodeDontErrorOnDuplicates_interface(t *testing.T) { +func TestDecodeErrorOnDuplicates_interface(t *testing.T) { cases := []struct { File string Err bool @@ -463,10 +465,8 @@ func TestDecodeDontErrorOnDuplicates_interface(t *testing.T) { }{ { "basic_duplicates.hcl", - false, - map[string]interface{}{ - "foo": "${file(\"bing/bong.txt\")}", - }, + true, + nil, }, } @@ -478,7 +478,7 @@ func TestDecodeDontErrorOnDuplicates_interface(t *testing.T) { } var out interface{} - err = DecodeDontErrorOnDuplicates(&out, string(d)) + err = DecodeErrorOnDuplicates(&out, string(d)) if (err != nil) != tc.Err { t.Fatalf("Input: %s\n\nError: %s", tc.File, err) } @@ -488,7 +488,7 @@ func TestDecodeDontErrorOnDuplicates_interface(t *testing.T) { } var v interface{} - err = UnmarshalDontErrorOnDuplicates(d, &v) + err = UnmarshalErrorOnDuplicates(d, &v) if (err != nil) != tc.Err { t.Fatalf("Input: %s\n\nError: %s", tc.File, err) } From a0f00633f6ecaea0b2a31189c358f62cdd5f5576 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:18:50 -0600 Subject: [PATCH 4/5] Apply suggestions from code review --- decoder.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/decoder.go b/decoder.go index 26b236442..820db500e 100644 --- a/decoder.go +++ b/decoder.go @@ -32,8 +32,8 @@ func Unmarshal(bs []byte, v interface{}) error { return DecodeObject(v, root) } -// Unmarshal accepts a byte slice as input and writes the -// data to the value pointed to by v. +// UnmarshalErrorOnDuplicates accepts a byte slice as input and writes the +// data to the value pointed to by v but errors on duplicate attribute key. func UnmarshalErrorOnDuplicates(bs []byte, v interface{}) error { root, err := parse(bs, true) if err != nil { @@ -49,13 +49,13 @@ func Decode(out interface{}, in string) error { return decode(out, in, false) } -// Decode reads the given input and decodes it into the structure +// DecodeErrorOnDuplicates reads the given input and decodes it into the structure but errrors on duplicate attribute key // given by `out`. func DecodeErrorOnDuplicates(out interface{}, in string) error { return decode(out, in, true) } -// Decode reads the given input and decodes it into the structure +// decode reads the given input and decodes it into the structure, takes in a boolean to determine if it should error on duplicate attribute // given by `out`. func decode(out interface{}, in string, errorOnDuplicateAtributes bool) error { obj, err := parse([]byte(in), errorOnDuplicateAtributes) From 7130d9c51d052924483ece9dab24f2642a3e323d Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:34:34 -0600 Subject: [PATCH 5/5] Update decoder.go --- decoder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/decoder.go b/decoder.go index 820db500e..39e56f222 100644 --- a/decoder.go +++ b/decoder.go @@ -55,8 +55,8 @@ func DecodeErrorOnDuplicates(out interface{}, in string) error { return decode(out, in, true) } -// decode reads the given input and decodes it into the structure, takes in a boolean to determine if it should error on duplicate attribute -// given by `out`. +// decode reads the given input and decodes it into the structure given by `out`. +// takes in a boolean to determine if it should error on duplicate attribute func decode(out interface{}, in string, errorOnDuplicateAtributes bool) error { obj, err := parse([]byte(in), errorOnDuplicateAtributes) if err != nil {