Skip to content

Commit

Permalink
types: Fix panic on reference to object with composite key
Browse files Browse the repository at this point in the history
The type checker was panicing when given a reference to an object with
a composite key because internally it infers the key type using
ast.ValueToInterface (which returns map[string]interface{} and not
map[interface{}]interface{}). This change updates the types package to
support map[string]interface{} internally.

Fixing the TypeOf function resolved the panic but it surfaced another
issue in the type checker where errors were returned if objects or
arrays were dereferenced with composite keys--in case of arrays, the
result is undefined but in the case of objects, this is valid.

Fixes #2648

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Aug 25, 2020
1 parent de0f695 commit 40bdc42
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 7 deletions.
4 changes: 0 additions & 4 deletions ast/check.go
Expand Up @@ -669,10 +669,6 @@ func (rc *refChecker) checkRefLeaf(tpe types.Type, ref Ref, idx int) *Error {
}

case *Array, Object, Set:
// Composite references operands may only be used with a set.
if !unifies(tpe, types.NewSet(types.A)) {
return newRefErrInvalid(ref[0].Location, rc.varRewriter(ref), idx, tpe, types.NewSet(types.A), nil)
}
if !unify1(rc.env, head, keys, false) {
return newRefErrInvalid(ref[0].Location, rc.varRewriter(ref), idx, rc.env.Get(head), keys, nil)
}
Expand Down
24 changes: 21 additions & 3 deletions ast/check_test.go
Expand Up @@ -110,6 +110,16 @@ func TestCheckInference(t *testing.T) {
nil,
),
}},
{"object-composite-ref-operand", `x = {{}: 1}; x[{}] = y`, map[Var]types.Type{
Var("x"): types.NewObject(
[]*types.StaticProperty{types.NewStaticProperty(
map[string]interface{}{},
types.N,
)},
nil,
),
Var("y"): types.N,
}},
{"sets", `x = {1, 2}; y = {{"foo", 1}, x}`, map[Var]types.Type{
Var("x"): types.NewSet(types.N),
Var("y"): types.NewSet(
Expand Down Expand Up @@ -795,12 +805,12 @@ func TestCheckRefErrInvalid(t *testing.T) {
oneOf: []Value{String("p"), String("q")},
},
{
note: "composite ref into non-set",
note: "composite ref operand",
query: `data.test.q[[1, 2]]`,
ref: "data.test.q[[1, 2]]",
pos: 3,
have: types.NewObject([]*types.StaticProperty{types.NewStaticProperty("bar", types.N), types.NewStaticProperty("foo", types.N)}, nil),
want: types.NewSet(types.A),
have: types.NewArray([]types.Type{types.N, types.N}, nil),
want: types.S,
},
{
note: "composite ref type error 1",
Expand All @@ -818,6 +828,14 @@ func TestCheckRefErrInvalid(t *testing.T) {
have: types.NewObject([]*types.StaticProperty{types.NewStaticProperty("a", types.S)}, nil),
want: types.NewObject([]*types.StaticProperty{types.NewStaticProperty("a", types.N)}, nil),
},
{
note: "composite ref type error 3 - array",
query: `a = [1,2,3]; a[{}] = b`,
ref: `a[{}]`,
pos: 1,
have: types.NewObject(nil, types.NewDynamicProperty(types.A, types.A)),
want: types.N,
},
}

for _, tc := range tests {
Expand Down
9 changes: 9 additions & 0 deletions types/types.go
Expand Up @@ -822,6 +822,15 @@ func TypeOf(x interface{}) Type {
return S
case json.Number:
return N
case map[string]interface{}:
// The ast.ValueToInterface() function returns ast.Object values as map[string]interface{}
// so map[string]interface{} must be handled here because the type checker uses the value
// to interface conversion when inferring object types.
static := make([]*StaticProperty, 0, len(x))
for k, v := range x {
static = append(static, NewStaticProperty(k, TypeOf(v)))
}
return NewObject(static, nil)
case map[interface{}]interface{}:
static := make([]*StaticProperty, 0, len(x))
for k, v := range x {
Expand Down
16 changes: 16 additions & 0 deletions types/types_test.go
Expand Up @@ -298,6 +298,22 @@ func TestTypeOf(t *testing.T) {
}
}

func TestTypeOfMapOfString(t *testing.T) {
tpe := TypeOf(map[string]interface{}{
"foo": "bar",
"baz": "qux",
})

exp := NewObject([]*StaticProperty{
NewStaticProperty("foo", S),
NewStaticProperty("baz", S),
}, nil)

if Compare(exp, tpe) != 0 {
t.Fatalf("Expected %v but got: %v", exp, tpe)
}
}

func TestNil(t *testing.T) {

tpe := NewObject([]*StaticProperty{
Expand Down

0 comments on commit 40bdc42

Please sign in to comment.