lang: Prevent struct types with duplicate fields

Struct types with duplicate fields are invalid types and weren't caught
by the parser. This fixes the issue and adds some associated tests. It
also checks and tests for duplicate struct value field names.

As a technical side-note, this doesn't change the lang/types/ functions
to remove panics-- the signatures are simplified to make their use
simple, and we intentionally panic if they're used incorrectly. In this
case, one was being used without having previously validated the input.

Thanks to Patrick Meyer for finding this issue via fuzzing!

lang/interpret_test.go

@@ -976,6 +976,8 @@ func TestAstFunc1(t *testing.T) {
 func TestAstFunc2(t *testing.T) {
 	const magicError = "# err: "
 	const magicError1 = "err1: "
+	const magicError9 = "err9: " // TODO: rename
+	// TODO: move them all down by one
 	const magicError2 = "err2: "
 	const magicError3 = "err3: "
 	const magicError4 = "err4: "
@@ -1010,6 +1012,8 @@ func TestAstFunc2(t *testing.T) {
 
 	type errs struct {
 		fail1 bool
+		fail9 bool // TODO: rename
+		// TODO: move them all down by one
 		fail2 bool
 		fail3 bool
 		fail4 bool
@@ -1068,6 +1072,8 @@ func TestAstFunc2(t *testing.T) {
 		// if the graph file has a magic error string, it's a failure
 		errStr := ""
 		fail1 := false
+		fail9 := false // TODO: rename
+		// TODO: move them all down by one
 		fail2 := false
 		fail3 := false
 		fail4 := false
@@ -1082,6 +1088,12 @@ func TestAstFunc2(t *testing.T) {
 				str = errStr
 				fail1 = true
 			}
+			if strings.HasPrefix(str, magicError9) { // TODO: rename
+				errStr = strings.TrimPrefix(str, magicError9)
+				str = errStr
+				fail9 = true
+			}
+			// TODO: move them all down by one
 			if strings.HasPrefix(str, magicError2) {
 				errStr = strings.TrimPrefix(str, magicError2)
 				str = errStr
@@ -1117,6 +1129,8 @@ func TestAstFunc2(t *testing.T) {
 			expstr: str,
 			errs: errs{
 				fail1: fail1,
+				fail9: fail9, // TODO: rename
+				// TODO: move them all down by one
 				fail2: fail2,
 				fail3: fail3,
 				fail4: fail4,
@@ -1156,6 +1170,8 @@ func TestAstFunc2(t *testing.T) {
 			name, path, fail, expstr, errs := tc.name, tc.path, tc.fail, strings.Trim(tc.expstr, "\n"), tc.errs
 			src := dir + path // location of the test
 			fail1 := errs.fail1
+			fail9 := errs.fail9 // TODO: rename
+			// TODO: move them all down by one
 			fail2 := errs.fail2
 			fail3 := errs.fail3
 			fail4 := errs.fail4
@@ -1282,11 +1298,28 @@ func TestAstFunc2(t *testing.T) {
 				},
 			}
 			// some of this might happen *after* interpolate in SetScope or Unify...
-			if err := ast.Init(data); err != nil {
+			err = ast.Init(data)
+			if (!fail || !fail9) && err != nil {
 				t.Errorf("test #%d: FAIL", index)
 				t.Errorf("test #%d: could not init and validate AST: %+v", index, err)
 				return
 			}
+			if fail9 && err != nil {
+				// TODO: %+v instead?
+				s := fmt.Sprintf("%s", err) // convert to string
+				if s != expstr {
+					t.Errorf("test #%d: FAIL", index)
+					t.Errorf("test #%d: expected different error", index)
+					t.Logf("test #%d: err: %s", index, s)
+					t.Logf("test #%d: exp: %s", index, expstr)
+				}
+				return // fail happened during lex parse, don't run init/interpolate!
+			}
+			if fail9 && err == nil {
+				t.Errorf("test #%d: FAIL", index)
+				t.Errorf("test #%d: Init passed, expected fail", index)
+				return
+			}
 
 			iast, err := ast.Interpolate()
 			if err != nil {

lang/interpret_test/TestAstFunc2/struct-duplicate0.output

@@ -0,0 +1 @@
+# err: err1: parser: `syntax error: unexpected $end, expecting EQUALS` @1:6

lang/interpret_test/TestAstFunc2/struct-duplicate0/main.mcl

@@ -0,0 +1 @@
+$d []struct{x int;x int}

lang/interpret_test/TestAstFunc2/struct-duplicate1.output

@@ -0,0 +1 @@
+# err: err1: can't set return type in parser: `can't set return type in parser: duplicate struct field of `x int`` @1:24

lang/interpret_test/TestAstFunc2/struct-duplicate1/main.mcl

@@ -0,0 +1 @@
+$st struct{x int; x int} = struct{x => 0,}

lang/interpret_test/TestAstFunc2/struct-duplicate2.output

@@ -0,0 +1 @@
+# err: err1: can't set return type in parser: `can't set return type in parser: duplicate struct field of `x int`` @1:24

lang/interpret_test/TestAstFunc2/struct-duplicate2/main.mcl

@@ -0,0 +1 @@
+$st struct{x int; x int} = struct{x => 0, x => 0,}

lang/interpret_test/TestAstFunc2/struct-duplicate3.output

@@ -0,0 +1 @@
+# err: err9: duplicate struct field name of: `x`

lang/interpret_test/TestAstFunc2/struct-duplicate3/main.mcl

@@ -0,0 +1,2 @@
+$st struct{x str} = struct{x => "hello", x => "world",}
+test structlookup($st, "x") {}

lang/parser.y

@@ -48,8 +48,6 @@ func init() {
 	int     int64 // this is the .int as seen in lexer.nex
 	float   float64
 
-	strSlice []string
-
 	typ   *types.Type
 
 	stmts []interfaces.Stmt
@@ -1177,7 +1175,23 @@ type:
 	// struct: struct{} or struct{a bool} or struct{a bool; bb int}
 	{
 		posLast(yylex, yyDollar) // our pos
-		$$.typ = types.NewType(fmt.Sprintf("%s{%s}", $1.str, strings.Join($3.strSlice, "; ")))
+
+		names := make(map[string]struct{})
+		strs := []string{}
+		for _, arg := range $3.args {
+			s := fmt.Sprintf("%s %s", arg.Name, arg.Type.String())
+			if _, exists := names[s]; exists {
+				// duplicate field name used
+				err := fmt.Errorf("duplicate struct field of `%s`", s)
+				// this will ultimately cause a parser error to occur...
+				yylex.Error(fmt.Sprintf("%s: %+v", ErrParseSetType, err))
+				break // we must skip, because code continues!
+			}
+			names[s] = struct{}{}
+			strs = append(strs, s)
+		}
+
+		$$.typ = types.NewType(fmt.Sprintf("%s{%s}", $1.str, strings.Join(strs, "; ")))
 	}
 |	FUNC_IDENTIFIER OPEN_PAREN type_func_args CLOSE_PAREN type
 	// XXX: should we allow named args in the type signature?
@@ -1228,24 +1242,27 @@ type_struct_fields:
 	/* end of list */
 	{
 		posLast(yylex, yyDollar) // our pos
-		$$.strSlice = []string{}
+		$$.args = []*Arg{}
 	}
 |	type_struct_fields SEMICOLON type_struct_field
 	{
 		posLast(yylex, yyDollar) // our pos
-		$$.strSlice = append($1.strSlice, $3.str)
+		$$.args = append($1.args, $3.arg)
 	}
 |	type_struct_field
 	{
 		posLast(yylex, yyDollar) // our pos
-		$$.strSlice = []string{$1.str}
+		$$.args = append([]*Arg{}, $1.arg)
 	}
 ;
 type_struct_field:
 	IDENTIFIER type
 	{
 		posLast(yylex, yyDollar) // our pos
-		$$.str = fmt.Sprintf("%s %s", $1.str, $2.typ.String())
+		$$.arg = &Arg{ // re-use the Arg struct
+			Name: $1.str,
+			Type: $2.typ,
+		}
 	}
 ;
 type_func_args:

lang/structs.go

@@ -5730,6 +5730,7 @@ func (obj *ExprMap) Apply(fn func(interfaces.Node) error) error {
 // Init initializes this branch of the AST, and returns an error if it fails to
 // validate.
 func (obj *ExprMap) Init(data *interfaces.Data) error {
+	// XXX: Can we check that there aren't any duplicate keys? Can we Cmp?
 	for _, x := range obj.KVs {
 		if err := x.Key.Init(data); err != nil {
 			return err
@@ -6252,7 +6253,14 @@ func (obj *ExprStruct) Apply(fn func(interfaces.Node) error) error {
 // Init initializes this branch of the AST, and returns an error if it fails to
 // validate.
 func (obj *ExprStruct) Init(data *interfaces.Data) error {
+	fields := make(map[string]struct{})
 	for _, x := range obj.Fields {
+		// Validate field names and ensure no duplicates!
+		if _, exists := fields[x.Name]; exists {
+			return fmt.Errorf("duplicate struct field name of: `%s`", x.Name)
+		}
+		fields[x.Name] = struct{}{}
+
 		if err := x.Value.Init(data); err != nil {
 			return err
 		}