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

Lift argument type checking out of the function type definition #6148

Closed
JeanMertz opened this issue Jan 19, 2021 · 0 comments · Fixed by #6353
Closed

Lift argument type checking out of the function type definition #6148

JeanMertz opened this issue Jan 19, 2021 · 0 comments · Fixed by #6353
Assignees
Labels
domain: vrl Anything related to the Vector Remap Language type: enhancement A value-adding code change that enhances its existing functionality.

Comments

@JeanMertz
Copy link
Contributor

Functions currently can be fallible because of two reasons:

  1. They perform an action that can fail depending on the value of their input (e.g. parse_json).
  2. They can fail depending on the type of their input (e.g. sha1).

The former means its fallibility depends on the runtime characteristics of the incoming events. There's nothing we can do about this at compile-time other than enforcing people to handle the error case of a function call.

The latter is something we have more control over at compile-time, through our progressive type system.

This issue proposes moving type checking out of functions and into the compiler itself.

This means two things:

  1. The function definition has to relay its expected parameter types to the compiler.
  2. The compiler has to use that information to error if you pass a non-matching type as a function argument.

For the first point, we already have something like this in place, but it predates the new TypeDef and value::Kind system, since this check is done at runtime. We'll have to swap this out with the new value::Kind set-up.

For the latter, this is essentially another check we add to our FunctionCall expression. We already do a ton of compile-time checks there (function arity, parameter names, missing required arguments, abort-call on infallible functions), so it's just a matter of adding another check to this list:

Get the expected value kind for each parameter, and compare that against the progressively collected type information for the provided argument. If they match, we're good, if not, we add an error diagnostic related to the incompatible argument type, similar to this:

error: unexpected argument type
  ┌─ :2:42contains(.message, "foo")
  │          ^^^^^^^^
  │          │
  │          got: string or null
  │          expected: string
  │
  = hint: coerce the value using one of the coercion functions
  = see language documentation at: https://vector.dev/docs/reference/vrl/

ref #6146

@JeanMertz JeanMertz added type: enhancement A value-adding code change that enhances its existing functionality. domain: vrl Anything related to the Vector Remap Language labels Jan 19, 2021
@JeanMertz JeanMertz self-assigned this Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: vrl Anything related to the Vector Remap Language type: enhancement A value-adding code change that enhances its existing functionality.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant