hcl/parser: cleaner error handling for objects with empty keys #158

The way that the contents of an object (objectList()) was parsed before
was weirdly implicit in expecting RBRACE as the ending token. This makes
that expectation explicit, which fixes a parse error that could occur
with an object that ends in an empty assign to an RBRACE.

Before this, the parser would accept this as expected behavior because
the object "ended" since the unexpected token was an RBRACE.

hcl/parser/parser.go

@@ -50,7 +50,7 @@ func (p *Parser) Parse() (*ast.File, error) {
 		scerr = &PosError{Pos: pos, Err: errors.New(msg)}
 	}
 
-	f.Node, err = p.objectList()
+	f.Node, err = p.objectList(false)
 	if scerr != nil {
 		return nil, scerr
 	}
@@ -62,11 +62,23 @@ func (p *Parser) Parse() (*ast.File, error) {
 	return f, nil
 }
 
-func (p *Parser) objectList() (*ast.ObjectList, error) {
+// objectList parses a list of items within an object (generally k/v pairs).
+// The parameter" obj" tells this whether to we are within an object (braces:
+// '{', '}') or just at the top level. If we're within an object, we end
+// at an RBRACE.
+func (p *Parser) objectList(obj bool) (*ast.ObjectList, error) {
 	defer un(trace(p, "ParseObjectList"))
 	node := &ast.ObjectList{}
 
 	for {
+		if obj {
+			tok := p.scan()
+			p.unscan()
+			if tok.Type == token.RBRACE {
+				break
+			}
+		}
+
 		n, err := p.objectItem()
 		if err == errEofToken {
 			break // we are finished
@@ -288,7 +300,7 @@ func (p *Parser) objectType() (*ast.ObjectType, error) {
 		Lbrace: p.tok.Pos,
 	}
 
-	l, err := p.objectList()
+	l, err := p.objectList(true)
 
 	// if we hit RBRACE, we are good to go (means we parsed all Items), if it's
 	// not a RBRACE, it's an syntax error and we just return it.
@@ -296,9 +308,9 @@ func (p *Parser) objectType() (*ast.ObjectType, error) {
 		return nil, err
 	}
 
-	// If there is no error, we should be at a RBRACE to end the object
-	if p.tok.Type != token.RBRACE {
-		return nil, fmt.Errorf("object expected closing RBRACE got: %s", p.tok.Type)
+	// No error, scan and expect the ending to be a brace
+	if tok := p.scan(); tok.Type != token.RBRACE {
+		return nil, fmt.Errorf("object expected closing RBRACE got: %s", tok.Type)
 	}
 
 	o.List = l

hcl/parser/parser_test.go

@@ -307,6 +307,8 @@ func TestObjectType(t *testing.T) {
 	}
 
 	for _, l := range literals {
+		t.Logf("Source: %s", l.src)
+
 		p := newParser([]byte(l.src))
 		// p.enableTrace = true
 		item, err := p.objectItem()
@@ -494,20 +496,34 @@ func TestParse(t *testing.T) {
 			"object_key_without_value.hcl",
 			true,
 		},
+		{
+			"object_key_assign_without_value.hcl",
+			true,
+		},
+		{
+			"object_key_assign_without_value2.hcl",
+			true,
+		},
+		{
+			"object_key_assign_without_value3.hcl",
+			true,
+		},
 	}
 
 	const fixtureDir = "./test-fixtures"
 
 	for _, tc := range cases {
-		d, err := ioutil.ReadFile(filepath.Join(fixtureDir, tc.Name))
-		if err != nil {
-			t.Fatalf("err: %s", err)
-		}
+		t.Run(tc.Name, func(t *testing.T) {
+			d, err := ioutil.ReadFile(filepath.Join(fixtureDir, tc.Name))
+			if err != nil {
+				t.Fatalf("err: %s", err)
+			}
 
-		_, err = Parse(d)
-		if (err != nil) != tc.Err {
-			t.Fatalf("Input: %s\n\nError: %s", tc.Name, err)
-		}
+			v, err := Parse(d)
+			if (err != nil) != tc.Err {
+				t.Fatalf("Input: %s\n\nError: %s\n\nAST: %#v", tc.Name, err, v)
+			}
+		})
 	}
 }
 

hcl/parser/test-fixtures/object_key_assign_without_value.hcl

@@ -0,0 +1,3 @@
+foo {
+  bar =
+}

hcl/parser/test-fixtures/object_key_assign_without_value2.hcl

@@ -0,0 +1,4 @@
+foo {
+  baz = 7
+  bar =
+}

hcl/parser/test-fixtures/object_key_assign_without_value3.hcl

@@ -0,0 +1,4 @@
+foo {
+  bar =
+  baz = 7
+}