Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow one or both conditional operands to be unknown #53

Merged
merged 2 commits into from
May 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions check_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func (tc *typeCheckConditional) TypeCheck(v *TypeCheck) (ast.Node, error) {
}

// The types of the true and false expression must match
if trueType != falseType {
if trueType != falseType && trueType != ast.TypeUnknown && falseType != ast.TypeUnknown {

// Since passing around stringified versions of other types is
// common, we pragmatically allow the false expression to dictate
Expand Down Expand Up @@ -460,7 +460,13 @@ func (tc *typeCheckConditional) TypeCheck(v *TypeCheck) (ast.Node, error) {
}

// Result type (guaranteed to also match falseType due to the above)
v.StackPush(trueType)
if trueType == ast.TypeUnknown {
// falseType may also be unknown, but that's okay because two
// unknowns means our result is unknown anyway.
v.StackPush(falseType)
} else {
v.StackPush(trueType)
}

return tc.n, nil
}
Expand Down
64 changes: 64 additions & 0 deletions check_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,70 @@ func TestTypeCheck(t *testing.T) {
true,
},

{
// conditional with unknown value is permitted
`foo ${true ? known : unknown}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"known": ast.Variable{
Type: ast.TypeString,
Value: "bar",
},
"unknown": ast.Variable{
Type: ast.TypeUnknown,
Value: UnknownValue,
},
},
},
false,
},

{
// conditional with unknown value the other way permitted too
`foo ${true ? unknown : known}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"known": ast.Variable{
Type: ast.TypeString,
Value: "bar",
},
"unknown": ast.Variable{
Type: ast.TypeUnknown,
Value: UnknownValue,
},
},
},
false,
},

{
// conditional with two unknowns is allowed
`foo ${true ? unknown : unknown}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Type: ast.TypeUnknown,
Value: UnknownValue,
},
},
},
false,
},

{
// conditional with unknown condition is allowed
`foo ${unknown ? 1 : 2}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Type: ast.TypeUnknown,
Value: UnknownValue,
},
},
},
false,
},

{
// currently lists are not allowed at all
`foo ${true ? arr1 : arr2}`,
Expand Down
108 changes: 105 additions & 3 deletions eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,108 @@ func TestEvalInternal(t *testing.T) {
ast.TypeUnknown,
},

{
// false expression can be unknown, and is returned
`foo ${false ? "12" : unknown}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Value: UnknownValue,
Type: ast.TypeUnknown,
},
},
},
false,
UnknownValue,
ast.TypeUnknown,
},

{
// false expression can be unknown, and result is unknown even
// if it's not selected.
// (Ideally this would not be true, but we're accepting this
// for now since this assumption is built in to the core evaluator)
`foo ${true ? "12" : unknown}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Value: UnknownValue,
Type: ast.TypeUnknown,
},
},
},
false,
UnknownValue,
ast.TypeUnknown,
},

{
// true expression can be unknown, and is returned
`foo ${false ? unknown : "bar"}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Value: UnknownValue,
Type: ast.TypeUnknown,
},
},
},
false,
UnknownValue,
ast.TypeUnknown,
},

{
// true expression can be unknown, and result is unknown even
// if it's not selected.
// (Ideally this would not be true, but we're accepting this
// for now since this assumption is built in to the core evaluator)
`foo ${false ? unknown : "bar"}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Value: UnknownValue,
Type: ast.TypeUnknown,
},
},
},
false,
UnknownValue,
ast.TypeUnknown,
},

{
// both values can be unknown
`foo ${false ? unknown : unknown}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Value: UnknownValue,
Type: ast.TypeUnknown,
},
},
},
false,
UnknownValue,
ast.TypeUnknown,
},

{
// condition can be unknown, and result is unknown
`foo ${unknown ? "baz" : "bar"}`,
&ast.BasicScope{
VarMap: map[string]ast.Variable{
"unknown": ast.Variable{
Value: UnknownValue,
Type: ast.TypeUnknown,
},
},
},
false,
UnknownValue,
ast.TypeUnknown,
},

{
"foo ${-bar}",
&ast.BasicScope{
Expand Down Expand Up @@ -1911,13 +2013,13 @@ func TestEvalInternal(t *testing.T) {

out, outType, err := internalEval(node, &EvalConfig{GlobalScope: tc.Scope})
if err != nil != tc.Error {
t.Fatalf("Error: %s\n\nInput: %s", err, tc.Input)
t.Fatalf("Error: %s\nInput: %s", err, tc.Input)
}
if tc.ResultType != ast.TypeInvalid && outType != tc.ResultType {
t.Fatalf("Bad: %s\n\nInput: %s", outType, tc.Input)
t.Fatalf("Wrong result type\nInput: %s\nGot: %#s\nWant: %s", tc.Input, outType, tc.ResultType)
}
if !reflect.DeepEqual(out, tc.Result) {
t.Fatalf("\n Got: %#v\n Want: %#v\n\nInput: %s\n", out, tc.Result, tc.Input)
t.Fatalf("Wrong result value\nInput: %s\nGot: %#s\nWant: %s", tc.Input, out, tc.Result)
}
})
}
Expand Down