Inconsistent runtime type check of "initial" argument in graph.reachable and graph.reachable_paths

[anderseknert]

The graph.reachable and graph.reachable_paths built-in functions seem to behave differently from other built-in functions at runtime, as providing an unsupported type as the second argument (the "initial" value) does not cause an undefined result, and does not render a runtime error with --strict-builtin-errors provided. The declared type of the initial argument is either a set or an array, but this is never checked in the code.

❯ echo '{"initial": 1}' |  opa eval -f pretty -I 'graph.reachable({}, input.initial)'
[]

❯ echo '{"initial": 1}' |  opa eval -f pretty --strict-builtin-errors -I 'graph.reachable({}, input.initial)'
[]

Other built-ins return either undefined on runtime type check failures, or an error if strict built-in errors is requested, e.g:

❯ echo '{"initial": 1}' |  opa eval -f pretty -I 'lower(input.initial)'
undefined

❯ echo '{"initial": 1}' |  opa eval -f pretty --strict-builtin-errors -I 'lower(input.initial)'
1 error occurred: lower(input.initial): eval_type_error: lower: operand 1 must be string but got number

Discovered when porting these built-ins for Jarl, and while only a minor inconsistency/inconvenience in OPA, the runtime type checker there currently verifies the type of the arguments against the types declared for any given built-in in the plan definition. Fixing this here seems like the better option compared to hard coding a workaround for these two functions in any IR implementation.

[philipaconrad]

@anderseknert This behavior seems to be caused by the implementation of the reachable and reachable_paths functions in topdown/reachable only checking if supported types are available, and then skipping action on things that don't match up type-wise, instead of giving a useful runtime type error.

[philipaconrad]

Do we want to have errors for wrong types on both arguments? I'll have a PR up shortly with that behavior, and if we want to switch it back to returning an empty set sometimes for backwards compatibility, it'll be a simple commit or two to revert back.