Skip to content

Commit

Permalink
cmd/cue: fix spurious errors
Browse files Browse the repository at this point in the history
Schema validation is sometimes too strict resulting in the
reporting of spurious errors. Disable schema checking for
now.
Note that any errors in the schema will anyway surface when
unified with the data instance.

Fixes #1479

Signed-off-by: Marcel van Lohuizen <[email protected]>

Change-Id: I2336f8453ee448c80c482e4eb19e0a99fd84d1a0
Signed-off-by: Marcel van Lohuizen <[email protected]>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/531263
Reviewed-by: Marcel van Lohuizen <[email protected]>
  • Loading branch information
mpvl committed Jan 31, 2022
1 parent 2fe5251 commit 04812bf
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 9 deletions.
25 changes: 19 additions & 6 deletions cmd/cue/cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (b *buildPlan) instances() iterator {
case len(b.insts) > 0:
i = &instanceIterator{
inst: b.instance,
a: buildInstances(b.cmd, b.insts),
a: buildInstances(b.cmd, b.insts, false),
i: -1,
}
default:
Expand Down Expand Up @@ -304,6 +304,12 @@ func (i *streamingIterator) scan() bool {
i.v = i.v.Unify(schema) // TODO(required fields): don't merge in schema
i.e = i.v.Err()
}
if i.e != nil {
if err = i.v.Validate(); err != nil {
// Validate should always be non-nil, but just in case.
i.e = err
}
}
i.f = nil
}
return i.e == nil
Expand Down Expand Up @@ -620,7 +626,13 @@ func parseArgs(cmd *Command, args []string, cfg *config) (p *buildPlan, err erro
}

if schema != nil && len(schema.Files) > 0 {
inst := buildInstances(p.cmd, []*build.Instance{schema})[0]
// TODO: ignore errors here for now until reporting of concreteness
// of errors is correct.
// See https://github.com/cue-lang/cue/issues/1483.
inst := buildInstances(
p.cmd,
[]*build.Instance{schema},
true)[0]

if inst.Err != nil {
return nil, err
Expand All @@ -630,7 +642,7 @@ func parseArgs(cmd *Command, args []string, cfg *config) (p *buildPlan, err erro
if p.schema != nil {
v := inst.Eval(p.schema)
if err := v.Err(); err != nil {
return nil, err
return nil, v.Validate()
}
p.encConfig.Schema = v
}
Expand Down Expand Up @@ -715,7 +727,7 @@ func (b *buildPlan) parseFlags() (err error) {
return nil
}

func buildInstances(cmd *Command, binst []*build.Instance) []*cue.Instance {
func buildInstances(cmd *Command, binst []*build.Instance, ignoreErrors bool) []*cue.Instance {
// TODO:
// If there are no files and User is true, then use those?
// Always use all files in user mode?
Expand All @@ -726,11 +738,12 @@ func buildInstances(cmd *Command, binst []*build.Instance) []*cue.Instance {
exitIfErr(cmd, inst, inst.Err, true)
}

if flagIgnore.Bool(cmd) {
// TODO: remove ignoreErrors flag and always return here, leaving it up to
// clients to check for errors down the road.
if ignoreErrors || flagIgnore.Bool(cmd) {
return instances
}

// TODO check errors after the fact in case of ignore.
for _, inst := range instances {
// TODO: consider merging errors of multiple files, but ensure
// duplicates are removed.
Expand Down
1 change: 1 addition & 0 deletions cmd/cue/cmd/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ func newTaskFunc(cmd *Command) flow.TaskFunc {
// Verify entry against template.
v = value.UnifyBuiltin(v, kind)
if err := v.Err(); err != nil {
err = v.Validate()
return nil, errors.Promote(err, "newTask")
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/cue/cmd/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func runEval(cmd *Command, args []string) error {
}
if err := v.Err(); err != nil {
errHeader()
return err
return v.Validate(syn...)
}

// TODO(#553): this can be removed once v.Syntax() below retains line
Expand Down
7 changes: 7 additions & 0 deletions cmd/cue/cmd/testdata/script/eval_flags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ use of -n/--name flag without a directory
reference "#D1" not found:
--schema:1:1
-- expect-stderr5 --
field not allowed: Z:
./test.json:4:3
./vector.cue:3:1
./vector.cue:3:6
X: conflicting values 1 and float (mismatched types int and float):
./test.json:2:8
./vector.cue:4:8
Y: conflicting values 2 and float (mismatched types int and float):
./test.json:3:8
./vector.cue:5:8
62 changes: 62 additions & 0 deletions cmd/cue/cmd/testdata/script/eval_mixedfiletypes.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#Issue 1479

exec cue eval x.cue data.json y.cue


# Demonstrate checks are ok
exec cue eval x.cue data.json
cmp stdout stdout.golden

# Import JSON and verify that we can assert checks are ok
exec cue import data.json
exec cue eval x.cue data.cue y.cue

# Assert checks ok using CUE + JSON
exec cue eval x.cue data.json y.cue

-- data.json --
{
"team": {
"alice": [
"EM"
],
"bob": [
"TL"
]
}
}
-- x.cue --
import (
"list"
)

#Team: [string]: [...("EM" | "IC" | "TL")]

team: #Team

checks: {
enoughMembers: {
ok: len(team) >= 1
}

hasManager: {
ok: len([ for m in team if list.Contains(m, "EM") {m}]) >= 1
}
}
-- y.cue --
checks: [string]: ok: true

-- stdout.golden --
#Team: {}
team: {
alice: ["EM"]
bob: ["TL"]
}
checks: {
enoughMembers: {
ok: true
}
hasManager: {
ok: true
}
}
4 changes: 2 additions & 2 deletions cmd/cue/cmd/trim.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func runTrim(cmd *Command, args []string) error {
if binst == nil {
return nil
}
instances := buildInstances(cmd, binst)
instances := buildInstances(cmd, binst, false)

dst := flagOutFile.String(cmd)
if dst != "" && dst != "-" && !flagForce.Bool(cmd) {
Expand Down Expand Up @@ -127,7 +127,7 @@ func runTrim(cmd *Command, args []string) error {

cfg := *defaultConfig.loadCfg
cfg.Overlay = overlay
tinsts := buildInstances(cmd, load.Instances(args, &cfg))
tinsts := buildInstances(cmd, load.Instances(args, &cfg), false)
if len(tinsts) != len(binst) {
return errors.New("unexpected number of new instances")
}
Expand Down

0 comments on commit 04812bf

Please sign in to comment.