Improve call result handling

Previously, if callers omitted output terms from call expressions, the
result would be ignored. This was fine for most calls which would only
return true if they were defined, however, for functions could return
false this became confusing because an expression like "f(1)" where f(1)
= false would be succeed and yield a result.

With these changes, built-in functions that used to only return true
always return true or false and the eval engine takes care to check if
the result is false when the the caller omits the output term.

This provides consistent behaviour across cases like...

f(1)            => undefined (previously {})
f(1,x)          => {x:false} (previously {x:false})
neq(1,1,x)      => {x:false} (previously undefined)
neq(1,1,false)  => {}        (previously undefined)
neq(1,1,true)   => undefined (previously undefined)

In the next set of changes, the Rego package will be updated to capture
values for expressions like the first one above so that function calls
behave like refs (i.e., their values are returned).

ast/builtins.go

@@ -141,7 +141,7 @@ var Equality = &Builtin{
 	Infix: "=",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -155,7 +155,7 @@ var Assign = &Builtin{
 	Infix: ":=",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -169,7 +169,7 @@ var GreaterThan = &Builtin{
 	Infix: ">",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -179,7 +179,7 @@ var GreaterThanEq = &Builtin{
 	Infix: ">=",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -189,7 +189,7 @@ var LessThan = &Builtin{
 	Infix: "<",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -199,7 +199,7 @@ var LessThanEq = &Builtin{
 	Infix: "<=",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -209,7 +209,7 @@ var NotEqual = &Builtin{
 	Infix: "!=",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -219,7 +219,7 @@ var Equal = &Builtin{
 	Infix: "==",
 	Decl: types.NewFunction(
 		types.Args(types.A, types.A),
-		types.T,
+		types.B,
 	),
 }
 
@@ -432,7 +432,7 @@ var RegexMatch = &Builtin{
 			types.S,
 			types.S,
 		),
-		types.T,
+		types.B,
 	),
 }
 
@@ -448,7 +448,7 @@ var GlobsMatch = &Builtin{
 			types.S,
 			types.S,
 		),
-		types.T,
+		types.B,
 	),
 }
 
@@ -517,7 +517,7 @@ var Contains = &Builtin{
 			types.S,
 			types.S,
 		),
-		types.T,
+		types.B,
 	),
 }
 
@@ -529,7 +529,7 @@ var StartsWith = &Builtin{
 			types.S,
 			types.S,
 		),
-		types.T,
+		types.B,
 	),
 }
 
@@ -541,7 +541,7 @@ var EndsWith = &Builtin{
 			types.S,
 			types.S,
 		),
-		types.T,
+		types.B,
 	),
 }
 

ast/check_test.go

@@ -70,7 +70,7 @@ func TestCheckInference(t *testing.T) {
 			Var("z"): types.N,
 		}},
 		{"array-nested", "[x, 1] = [true, y]", map[Var]types.Type{
-			Var("x"): types.T,
+			Var("x"): types.B,
 			Var("y"): types.N,
 		}},
 		{"array-transitive", "y = [[2], 1]; [[x], 1] = y", map[Var]types.Type{
@@ -159,7 +159,7 @@ func TestCheckInference(t *testing.T) {
 				3];
 
 			x = a[1].foo[_].bar`, map[Var]types.Type{
-			Var("x"): types.NewAny(types.NewNull(), types.T),
+			Var("x"): types.NewAny(types.NewNull(), types.B),
 		}},
 		{"local-reference-var", `
 
@@ -191,8 +191,8 @@ func TestCheckInference(t *testing.T) {
 			`, map[Var]types.Type{
 			Var("i"): types.N,
 			Var("j"): types.S,
-			Var("k"): types.NewAny(types.S, types.N, types.T),
-			Var("x"): types.NewAny(types.S, types.N, types.T),
+			Var("k"): types.NewAny(types.S, types.N, types.B),
+			Var("x"): types.NewAny(types.S, types.N, types.B),
 		}},
 		{"local-reference-var-any", `
 			a = [[], {}];
@@ -331,7 +331,7 @@ func TestCheckInferenceRules(t *testing.T) {
 		ref      string
 		expected types.Type
 	}{
-		{"trivial", ruleset1, `data.a.trivial`, types.T},
+		{"trivial", ruleset1, `data.a.trivial`, types.B},
 
 		{"complete-doc", ruleset1, `data.a.complete`, types.NewArray(
 			[]types.Type{types.NewObject(
@@ -374,7 +374,7 @@ func TestCheckInferenceRules(t *testing.T) {
 			types.NewAny(
 				types.S,
 				types.N,
-				types.T),
+				types.B),
 		)},
 
 		{"iteration-keys", ruleset1, "data.iteration.keys", types.NewSet(
@@ -400,19 +400,19 @@ func TestCheckInferenceRules(t *testing.T) {
 			types.NewDynamicProperty(types.S, types.NewAny(types.S, types.N)),
 		)},
 
-		{"ref", ruleset1, "data.b.trivial_ref", types.T},
+		{"ref", ruleset1, "data.b.trivial_ref", types.B},
 
 		{"ref-transitive", ruleset1, "data.b.transitive_ref", types.NewArray(
 			[]types.Type{
-				types.T,
+				types.B,
 			},
 			nil,
 		)},
 
 		{"prefix", ruleset1, `data.prefix.a.b`, types.NewObject(
 			[]*types.StaticProperty{{
 				"c", types.NewObject(
-					[]*types.StaticProperty{{"d", types.T}},
+					[]*types.StaticProperty{{"d", types.B}},
 					types.NewDynamicProperty(types.S, types.A),
 				),
 			}},

ast/env.go

@@ -35,10 +35,7 @@ func (env *TypeEnv) Get(x interface{}) types.Type {
 	case Null:
 		return types.NewNull()
 	case Boolean:
-		if x.Compare(Boolean(true)) == 0 {
-			return types.T
-		}
-		return types.F
+		return types.NewBoolean()
 	case Number:
 		return types.NewNumber()
 	case String:

topdown/comparison.go

@@ -34,10 +34,7 @@ func compareEq(a, b ast.Value) bool {
 
 func builtinCompare(cmp compareFunc) FunctionalBuiltin2 {
 	return func(a, b ast.Value) (ast.Value, error) {
-		if !cmp(a, b) {
-			return nil, BuiltinEmpty{}
-		}
-		return ast.Boolean(true), nil
+		return ast.Boolean(cmp(a, b)), nil
 	}
 }
 

topdown/eval.go

@@ -775,7 +775,10 @@ func (e evalBuiltin) eval(iter unifyIterator) error {
 		defer e.e.instr.startTimer(evalOpBuiltinCall)
 
 		if len(operands) == numDeclArgs {
-			return iter()
+			if output.Value.Compare(ast.Boolean(false)) != 0 {
+				return iter()
+			}
+			return nil
 		}
 		return e.e.unify(e.terms[len(e.terms)-1], output, iter)
 	})
@@ -828,24 +831,31 @@ func (e evalFunc) evalOneRule(iter unifyIterator, rule *ast.Rule, prev *ast.Term
 
 	child := e.e.child(rule.Body)
 
-	b := make(ast.Array, len(e.terms))
+	args := make(ast.Array, len(e.terms))
+
 	for i := range rule.Head.Args {
-		b[i] = rule.Head.Args[i]
+		args[i] = rule.Head.Args[i]
 	}
 
-	if len(b) == len(rule.Head.Args)+1 {
-		b[len(b)-1] = rule.Head.Value
+	if len(args) == len(rule.Head.Args)+1 {
+		args[len(args)-1] = rule.Head.Value
 	}
 
 	var result *ast.Term
 
 	child.traceEnter(rule)
 
-	err := child.biunifyArrays(e.terms, b, e.e.bindings, child.bindings, func() error {
+	err := child.biunifyArrays(e.terms, args, e.e.bindings, child.bindings, func() error {
 		return child.eval(func(child *eval) error {
 			child.traceExit(rule)
 			result = child.bindings.Plug(rule.Head.Value)
 
+			if len(e.terms) == len(rule.Head.Args) {
+				if result.Value.Compare(ast.Boolean(false)) == 0 {
+					return nil
+				}
+			}
+
 			// Partial evaluation should explore all rules and may not produce
 			// a ground result so we do not perform conflict detection or
 			// deduplication. See "ignore conflicts: functions" test case for

topdown/regex.go

@@ -30,10 +30,7 @@ func builtinRegexMatch(a, b ast.Value) (ast.Value, error) {
 	if err != nil {
 		return nil, err
 	}
-	if re.Match([]byte(s2)) {
-		return ast.Boolean(true), nil
-	}
-	return nil, BuiltinEmpty{}
+	return ast.Boolean(re.Match([]byte(s2))), nil
 }
 
 func getRegexp(pat string) (*regexp.Regexp, error) {
@@ -64,12 +61,7 @@ func builtinGlobsMatch(a, b ast.Value) (ast.Value, error) {
 	if err != nil {
 		return nil, err
 	}
-
-	if ne {
-		return ast.Boolean(true), nil
-	}
-
-	return nil, BuiltinEmpty{}
+	return ast.Boolean(ne), nil
 }
 
 func init() {

topdown/strings.go

@@ -134,11 +134,7 @@ func builtinContains(a, b ast.Value) (ast.Value, error) {
 		return nil, err
 	}
 
-	if !strings.Contains(string(s), string(substr)) {
-		return nil, BuiltinEmpty{}
-	}
-
-	return ast.Boolean(true), nil
+	return ast.Boolean(strings.Contains(string(s), string(substr))), nil
 }
 
 func builtinStartsWith(a, b ast.Value) (ast.Value, error) {
@@ -152,11 +148,7 @@ func builtinStartsWith(a, b ast.Value) (ast.Value, error) {
 		return nil, err
 	}
 
-	if !strings.HasPrefix(string(s), string(prefix)) {
-		return nil, BuiltinEmpty{}
-	}
-
-	return ast.Boolean(true), nil
+	return ast.Boolean(strings.HasPrefix(string(s), string(prefix))), nil
 }
 
 func builtinEndsWith(a, b ast.Value) (ast.Value, error) {
@@ -170,11 +162,7 @@ func builtinEndsWith(a, b ast.Value) (ast.Value, error) {
 		return nil, err
 	}
 
-	if !strings.HasSuffix(string(s), string(suffix)) {
-		return nil, BuiltinEmpty{}
-	}
-
-	return ast.Boolean(true), nil
+	return ast.Boolean(strings.HasSuffix(string(s), string(suffix))), nil
 }
 
 func builtinLower(a ast.Value) (ast.Value, error) {

topdown/topdown_test.go

@@ -1787,6 +1787,28 @@ func TestTopDownFunctions(t *testing.T) {
 			data.test.foo(y[2], a)
 		}
 
+		falsy_func(x) = false
+
+		falsy_func_else(x) = true { x = 1 } else = false { true }
+
+		falsy_undefined {
+			falsy_func(1)
+		}
+
+		falsy_negation {
+			not falsy_func(1)
+		}
+
+		falsy_else_value = falsy_func_else(2)
+
+		falsy_else_undefined {
+			falsy_func_else(2)
+		}
+
+		falsy_else_negation {
+			not falsy_func_else(2)
+		}
+
 		arrays([x, y]) = [a, b] {
 			foo(x, a)
 			foo(y, b)
@@ -1918,6 +1940,11 @@ func TestTopDownFunctions(t *testing.T) {
 	defer store.Abort(ctx, txn)
 
 	assertTopDownWithPath(t, compiler, store, "basic call", []string{"ex", "bar", "alice"}, "", `["al", "ce"]`)
+	assertTopDownWithPath(t, compiler, store, "false result", []string{"ex", "falsy_undefined"}, "", ``)
+	assertTopDownWithPath(t, compiler, store, "false result negation", []string{"ex", "falsy_negation"}, "", `true`)
+	assertTopDownWithPath(t, compiler, store, "false else value", []string{"ex", "falsy_else_value"}, "", `false`)
+	assertTopDownWithPath(t, compiler, store, "false else undefined", []string{"ex", "falsy_else_undefined"}, "", ``)
+	assertTopDownWithPath(t, compiler, store, "false else negation", []string{"ex", "falsy_else_negation"}, "", `true`)
 	assertTopDownWithPath(t, compiler, store, "chained", []string{"ex", "chain2"}, "", `["foo", "bar"]`)
 	assertTopDownWithPath(t, compiler, store, "cross package", []string{"test", "cross"}, "", `["s f", [", my name "]]`)
 	assertTopDownWithPath(t, compiler, store, "array params", []string{"ex", "arraysrule"}, "", `[["h", "h"], ["foo"]]`)