diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dbf93d..9e336c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # 1.16.2 (Unreleased) +* `json`: `ImpliedType` now returns an error if a JSON object contains two properties of the same name. As a compatibility concession it allows duplicates whose values have the same implied type, since it was unintentionally possible to combine `ImpliedType` and `Unmarshal` successfully in that case before, but this is not an endorsement of using duplicate property names since that makes the input ambiguous in any case. ([#199](https://github.com/zclconf/go-cty/issues/199)) * `function/stdlib`: `ElementFunc` no longer crashes when asked for a negative index into a tuple. This fixes a miss in the negative index support added back in v1.15.0. ([#200](https://github.com/zclconf/go-cty/pull/200)) # 1.16.1 (January 13, 2025) diff --git a/cty/json/type_implied.go b/cty/json/type_implied.go index 0fa13f6..1c90f34 100644 --- a/cty/json/type_implied.go +++ b/cty/json/type_implied.go @@ -127,6 +127,29 @@ func impliedObjectType(dec *json.Decoder) (cty.Type, error) { if atys == nil { atys = make(map[string]cty.Type) } + if existing, exists := atys[key]; exists { + // We didn't originally have any special treatment for multiple properties + // of the same name, having the type of the last one "win". But that caused + // some confusing error messages when the same input was subsequently used + // with [Unmarshal] using the returned object type, since [Unmarshal] would + // try to fit all of the property values of that name to whatever type + // the last one had, and would likely fail in doing so if the earlier + // properties of the same name had different types. + // + // As a compromise to avoid breaking existing successful use of _consistently-typed_ + // redundant properties, we return an error here only if the new type + // differs from the old type. The error message doesn't mention that subtlety + // because the equal type carveout is a compatibility concession rather than + // a feature folks should rely on in new code. + if !existing.Equals(aty) { + // This error message is low-quality because ImpliedType doesn't do + // path tracking while it traverses, unlike Unmarshal. However, this + // is a rare enough case that we don't want to pay the cost of allocating + // another path-tracking buffer that would in most cases be ignored, + // so we just accept a low-context error message. :( + return cty.NilType, fmt.Errorf("duplicate %q property in JSON object", key) + } + } atys[key] = aty } diff --git a/cty/json/type_implied_test.go b/cty/json/type_implied_test.go index 019760c..50cc5e4 100644 --- a/cty/json/type_implied_test.go +++ b/cty/json/type_implied_test.go @@ -91,6 +91,12 @@ func TestImpliedType(t *testing.T) { }), }), }, + { + `{"a": "hello", "a": "world"}`, + cty.Object(map[string]cty.Type{ + "a": cty.String, + }), + }, } for _, test := range tests { @@ -110,3 +116,44 @@ func TestImpliedType(t *testing.T) { }) } } + +func TestImpliedTypeErrors(t *testing.T) { + tests := []struct { + Input string + WantError string + }{ + { + `{"a": "hello", "a": true}`, + `duplicate "a" property in JSON object`, + }, + { + `{}boop`, + `extraneous data after JSON object`, + }, + { + `[!]`, + `invalid character '!' looking for beginning of value`, + }, + { + `[}`, + `invalid character '}' looking for beginning of value`, + }, + { + `{true: null}`, + `invalid character 't'`, + }, + } + + for _, test := range tests { + t.Run(test.Input, func(t *testing.T) { + _, err := ImpliedType([]byte(test.Input)) + if err == nil { + t.Fatalf("unexpected success\nwant error: %s", err) + } + + if got, want := err.Error(), test.WantError; got != want { + t.Errorf("wrong error\ngot: %s\nwant: %s", got, want) + } + }) + } +}