cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.06k stars 288 forks source link

Ability to add helpful error messages for humans #291

Open cueckoo opened 3 years ago

cueckoo commented 3 years ago

Originally opened by @mvdan in https://github.com/cuelang/cue/issues/291

Issues like https://github.com/cuelang/cue/issues/52 and https://github.com/cuelang/cue/issues/171 would be a step in the right direction, so that automatic errors are more helpful.

However, there's generally quite a bit of context that cue will never be able to infer on its own. For example:

$ cat test.cue
import "list"

needs_foo: bool
foo: [...string]

if needs_foo {
    foo: list.MinItems(1)
}
$ cat test.yaml
needs_foo: true
foo: []
$ cue vet -c test.yaml test.cue
foo: invalid operation [] &! list.MinItems (1) (mismatched types list and list):
    ./test.cue:4:6
    ./test.cue:7:7
    ./test.yaml:2:7

It would be far better if cue vet gave a useful error message, like foo can't be an empty list when needs_foo is true. This is particularly important when the consumer of the errors isn't familiar with cue, or when they don't have easy access to the cue files which are vetting their yaml or json.

One can imagine other scenarios where a human-friendly error message would be far better than a generated one. For example, in the context of Kubernetes, high-level errors can be explained simply, such as Deployment "foo" refers to unknown ConfigMap "bar".

Such human-friendly errors would be optional, and ideally replace generated errors. My initial thought was along the lines of:

foo: list.MinItems(1) // foo can't be an empty list when needs_foo is true

Here's a variant from @rogpeppe:

foo: list.MinItems(1) | error("foo can't be an empty list when needs_foo is true")

@mpvl also had some thoughts on Slack, but I'll let him reply here as that seems better than copy-pasting all of his messages.

cueckoo commented 3 years ago

Original reply by @mvdan in https://github.com/cuelang/cue/issues/291#issuecomment-587554949

I'd add the errors label, but I assume only people with write access to the repo can do that.

cueckoo commented 3 years ago

Original reply by @seh in https://github.com/cuelang/cue/issues/291#issuecomment-628879796

I was also wondering if we could use a marker like the current @tag for something like this, such as @doc or @invalid to offer a more readable message when unification fails.

We'd need to consider whether one could attach it to nonscalar values like structs, where fields within the struct could also have values with these markers.

cueckoo commented 3 years ago

Original reply by @narzach in https://github.com/cuelang/cue/issues/291#issuecomment-804265527

Is this still under active investigation for the roadmap? This is my main concern in considering CUE as a framework, and the fact that this hasn't gotten any traction in over a year is a concerning indicator as to the future of this project.

cueckoo commented 3 years ago

Original reply by @mpvl in https://github.com/cuelang/cue/issues/291#issuecomment-804353636

We are writing a proposal for a set of builtins that will cover this.

The new evaluator was rather a big task to complete, which pushed this to the back, unfortunately. It is not as trivial as it seems, and it requires the new evaluator.

cueckoo commented 3 years ago

Original reply by @myitcv in https://github.com/cuelang/cue/issues/291#issuecomment-804766038

@narzach - just to add to @mpvl's comments in https://github.com/cuelang/cue/issues/291#issuecomment-804353636.

Whilst not reflected in this issue there has been a significant improvement in error messages since v0.2.2. The new evaluator (which represents the bulk of the changes in the soon-to-be-released v0.3.0) was a pre-requisite for many of these changes.

But work on error messages is far from "done", hence the proposal for a set of builtins that @mpvl references.