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.03k stars 287 forks source link

cmd/cue: syntax errors in potential inputs silently dropped #1703

Open myitcv opened 2 years ago

myitcv commented 2 years ago

What version of CUE are you using (cue version)?

$ cue version
cue version +bd9ae537 linux/arm64

Does this issue reproduce with the latest release?

Yes

What did you do?

! exec cue eval
cmp stderr stderr.golden

-- x.cue --
package x

x: 5
-- y.cue --
pacakge x

y: 4
-- stderr.golden --
expected label or ':', found 'IDENT' x:
    ./y.cue:1:9

What did you expect to see?

Subject to the suggestion below, a passing test.

What did you see instead?

> ! exec cue eval
[stdout]
x: 5
FAIL: /tmp/testscript1597855587/repro.txt/script.txt:1: unexpected command success

This conversation started off the back of a problem someone raised in Slack.

The issue today is that cue eval (not specific to eval) succeeds because cmd/cue sees the syntax error in y.cue and so ignores the file. However, it's hopefully obvious that the user intended the file to belong to package x.

Whilst an LSP implementation (#142) will likely help to prevent such syntax errors in the first place, we can't rule them out and it doesn't seem ideal that files with syntax errors silently get dropped.

Following a discussion with @rogpeppe, we believe that a better heuristic here would be to raise syntax errors unless it is known that a file does not belong to the set of packages specified as input to cmd/cue (hence the expectation in the testscript test above).

Under this suggestion, given a cmd/cue input package p, syntax errors would therefore be raised for:

But syntax errors would not be raised for:

rogpeppe commented 2 years ago

Probably another issue, but I have thought for a while that files without a package clause that co-exist with files with a package clause should result in an error, because it's a very common mistake to make.

mpvl commented 2 years ago

@rogpeppe: if one looks at the original intent, then one could argue it should be allowed.

However, we could indeed require an explicit package _ or something in case such files need to coexist with package files.