From b8bc9383eec60f79c82f7d09bfd22a157380539f Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 28 Oct 2024 10:45:50 -0500 Subject: [PATCH 1/2] update parser to check for duplicates --- hcl/parser/parser.go | 20 ++++++++++++++++++++ hcl/parser/parser_test.go | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/hcl/parser/parser.go b/hcl/parser/parser.go index 64c83bcfb..0d8a13257 100644 --- a/hcl/parser/parser.go +++ b/hcl/parser/parser.go @@ -76,6 +76,7 @@ func (p *Parser) objectList(obj bool) (*ast.ObjectList, error) { defer un(trace(p, "ParseObjectList")) node := &ast.ObjectList{} + seenKeys := map[string]struct{}{} for { if obj { tok := p.scan() @@ -88,8 +89,17 @@ func (p *Parser) objectList(obj bool) (*ast.ObjectList, error) { n, err := p.objectItem() if err == errEofToken { break // we are finished + } else if err != nil { + return nil, err } + for _, key := range n.Keys { + _, 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 per block.", key.Token.Text, node.Pos().String())) + } + seenKeys[key.Token.Text] = struct{}{} + } // we don't return a nil node, because might want to use already // collected items. if err != nil { @@ -225,6 +235,7 @@ 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() @@ -268,6 +279,12 @@ 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: @@ -324,6 +341,8 @@ func (p *Parser) objectType() (*ast.ObjectType, error) { // not a RBRACE, it's an syntax error and we just return it. if err != nil && p.tok.Type != token.RBRACE { return nil, err + } else if err != nil { + return nil, err } // No error, scan and expect the ending to be a brace @@ -365,6 +384,7 @@ func (p *Parser) listType() (*ast.ListType, error) { } switch tok.Type { case token.BOOL, token.NUMBER, token.FLOAT, token.STRING, token.HEREDOC: + node, err := p.literalType() if err != nil { return nil, err diff --git a/hcl/parser/parser_test.go b/hcl/parser/parser_test.go index 270212207..6801d4ad1 100644 --- a/hcl/parser/parser_test.go +++ b/hcl/parser/parser_test.go @@ -134,6 +134,40 @@ func TestListOfMaps(t *testing.T) { } } +func TestDuplicateKeys_NotAllowed(t *testing.T) { + src := `key = "value1" + key = "value2" + ` + p := newParser([]byte(src)) + + _, err := p.Parse() + if err == nil { + t.Fatalf("Expected error, got none!") + } + + expected := "Each argument can only be defined once per block." + if !strings.Contains(err.Error(), expected) { + t.Fatalf("Expected err:\n %s\nTo contain:\n %s\n", err, expected) + } +} + +func TestDuplicateKeys_NotAllowedInBlock(t *testing.T) { + src := `key = "value1" + structkey "structname" { name = "test" name = "test2"} + ` + p := newParser([]byte(src)) + + _, err := p.Parse() + if err == nil { + t.Fatalf("Expected error, got none!") + } + + expected := "Each argument can only be defined once per block." + if !strings.Contains(err.Error(), expected) { + t.Fatalf("Expected err:\n %s\nTo contain:\n %s\n", err, expected) + } +} + func TestListOfMaps_requiresComma(t *testing.T) { src := `foo = [ {key = "bar"} @@ -240,7 +274,7 @@ func TestListType_lineComment(t *testing.T) { comment := l.comment[i] if (lt.LineComment == nil) != (comment == "") { - t.Fatalf("bad: %s", lt) + t.Fatalf("bad: %v", lt) } if comment == "" { From 203eb201abf355354afa58a9f1c4ea19b4e768b3 Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Mon, 28 Oct 2024 13:26:34 -0500 Subject: [PATCH 2/2] add additional test cases and ensure they still work --- hcl/parser/parser.go | 16 +++++++++++----- hcl/parser/parser_test.go | 17 +++++++++++++++-- hcl/parser/test-fixtures/types.hcl | 8 ++++---- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/hcl/parser/parser.go b/hcl/parser/parser.go index 0d8a13257..985ecef02 100644 --- a/hcl/parser/parser.go +++ b/hcl/parser/parser.go @@ -84,22 +84,28 @@ func (p *Parser) objectList(obj bool) (*ast.ObjectList, error) { if tok.Type == token.RBRACE { break } + } n, err := p.objectItem() + if err == errEofToken { break // we are finished } else if err != nil { return nil, err } - for _, key := range n.Keys { - _, 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 per block.", key.Token.Text, node.Pos().String())) + if n.Assign.String() != "-" { + for _, key := range n.Keys { + _, 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())) + + } + seenKeys[key.Token.Text] = struct{}{} } - seenKeys[key.Token.Text] = struct{}{} } + // we don't return a nil node, because might want to use already // collected items. if err != nil { diff --git a/hcl/parser/parser_test.go b/hcl/parser/parser_test.go index 6801d4ad1..3e3d1f5c3 100644 --- a/hcl/parser/parser_test.go +++ b/hcl/parser/parser_test.go @@ -145,7 +145,7 @@ func TestDuplicateKeys_NotAllowed(t *testing.T) { t.Fatalf("Expected error, got none!") } - expected := "Each argument can only be defined once per block." + expected := "Each argument can only be defined once" if !strings.Contains(err.Error(), expected) { t.Fatalf("Expected err:\n %s\nTo contain:\n %s\n", err, expected) } @@ -162,12 +162,25 @@ func TestDuplicateKeys_NotAllowedInBlock(t *testing.T) { t.Fatalf("Expected error, got none!") } - expected := "Each argument can only be defined once per block." + expected := "Each argument can only be defined once" if !strings.Contains(err.Error(), expected) { t.Fatalf("Expected err:\n %s\nTo contain:\n %s\n", err, expected) } } +func TestDuplicateBlocks_allowed(t *testing.T) { + src := `structkey { name = "test" } + structkey { name = "test" } + ` + p := newParser([]byte(src)) + + _, err := p.Parse() + if err != nil { + t.Fatalf(err.Error()) + } + +} + func TestListOfMaps_requiresComma(t *testing.T) { src := `foo = [ {key = "bar"} diff --git a/hcl/parser/test-fixtures/types.hcl b/hcl/parser/test-fixtures/types.hcl index cf2747ea1..f403da1fb 100644 --- a/hcl/parser/test-fixtures/types.hcl +++ b/hcl/parser/test-fixtures/types.hcl @@ -1,7 +1,7 @@ foo = "bar" bar = 7 baz = [1,2,3] -foo = -12 -bar = 3.14159 -foo = true -bar = false +foo2 = -12 +bar2 = 3.14159 +foo3 = true +bar3 = false