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

cmd/cue: def sometimes inserts CUE comments for errors #2646

Open
gotwarlost opened this issue Oct 15, 2023 · 4 comments
Open

cmd/cue: def sometimes inserts CUE comments for errors #2646

gotwarlost opened this issue Oct 15, 2023 · 4 comments

Comments

@gotwarlost
Copy link

gotwarlost commented Oct 15, 2023

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

$ cue version
cue version v0.6.0

go version go1.21.1
      -buildmode exe
       -compiler gc
       -trimpath true
  DefaultGODEBUG panicnil=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin

Does this issue reproduce with the latest stable release?

Yes

What did you do?

cue eval ./repro # good
cue def ./repro | cue eval - # also good
cue def ./repro | cue fmt - | cue eval - # fails

-- repro/one.cue --
package repro

request: [
		if _input.real != _|_ {_input.real},
		{_input.test},
][0]

_input: { test: "pass" }

What did you expect to see?

Formatted code evaluates with the same semantics as the original code

What did you see instead?

Evaluation after formatting fails. Part of the list expression is gobbled up because it is rendered on the same line as the comment.

@gotwarlost gotwarlost added NeedsInvestigation Triage Requires triage/attention labels Oct 15, 2023
@myitcv myitcv added fmt Related to formatting functionality. and removed Triage Requires triage/attention labels Oct 18, 2023
@myitcv myitcv changed the title cue fmt generates wrong code under certain circumstances cmd/cue: fmt generates wrong code under certain circumstances Oct 18, 2023
@myitcv myitcv added this to the fmt-redesign milestone Oct 18, 2023
@mvdan mvdan added the good first issue Good for newcomers label Oct 23, 2023
@rudifa
Copy link
Contributor

rudifa commented Nov 2, 2023

Try to unpack above test case, working with

cue version v0.6.0

go version go1.20.7
      -buildmode exe
       -compiler gc
       -trimpath true
     CGO_ENABLED 1
          GOARCH amd64
            GOOS darwin
         GOAMD64 v1
  1. create file one.cue:
package repro

request: [
		if _input.real != _|_ {_input.real},
		{_input.test},
][0]

_input: {test: "pass"}
  1. run cue def one.cue:
package repro

request: [
		if _input.real != _|_ // explicit error (_|_ literal) in source
		{
		_input.real
	}, _input.test][0]
_input: {
	test: "pass"
}

TODO CLARIFY: why does cue def emit the comment above? why and how did the error message end up in a comment?

  1. run cue def one.cue | cue fmt -:
package repro

request: [
		if _input.real != _|_ {
		_input.real
	} // explicit error (_|_ literal) in source, _input.test][0]
_input: {
	test: "pass"
}

cue fmt misplaces the comment and breaks the syntax, similar to cases reported in #2423 and
#2567

@rudifa
Copy link
Contributor

rudifa commented Nov 4, 2023

Here is a minimal test case

  1. cat two.cue:
_a: {}

[
    if _a.x != _|_ {},
]
  1. cue eval two.cue:
[]
  1. cue def two.cue:
[
	if _a.x != _|_ // explicit error (_|_ literal) in source
	{}]
_a: {}
  1. cue def two.cue | cue fmt -:
[
	if _a.x != _|_ {} // explicit error (_|_ literal) in source]
_a: {}
  1. cue def two.cue | cue fmt - | cue eval -:
missing ',' in list literal:
    -:4:3

@mvdan
Copy link
Member

mvdan commented Nov 8, 2023

I agree with @rudifa that there are two potential issues here:

  1. Why is cue def adding the comment // explicit error (_|_ literal) in source to the CUE? If it is an error, I assume it should be getting printed to stderr, and we should be able to control it via flags like cue def --ignore or cue def --strict --all-errors - however, none of those flags seem to change the behavior.

  2. cue fmt places the closing square bracket inside the inline comment, resulting in invalid syntax. This is very similar to cmd/fmt: fmt emits invalid CUE when commenting struct references in lists #2274, and even more closely related to Cue fmt destroys working code #2567. I have added a reproducer to Cue fmt destroys working code #2567, and I think we should use that issue for the fmt bug.

Given that we already have an issue for the fmt bug at #2567, we can reuse this issue for the potential issue 1 above - cue def adding an error to the output CUE in the form of a comment. It's not clear to me whether this is intended behavior.

@mvdan mvdan removed fmt Related to formatting functionality. good first issue Good for newcomers labels Nov 8, 2023
@mvdan mvdan removed this from the fmt-redesign milestone Nov 8, 2023
@mvdan mvdan changed the title cmd/cue: fmt generates wrong code under certain circumstances cmd/cue: def sometimes inserts CUE comments for errors Nov 8, 2023
@mvdan
Copy link
Member

mvdan commented Nov 8, 2023

And here is a reproducer for cue def inserting a comment for an error:

exec cue eval in.cue
cmp stdout eval-stdout.golden

exec cue def in.cue
cmp stdout def-stdout.golden

-- in.cue --
_a: {}

[
    if _a.x != _|_ {},
]
-- eval-stdout.golden --
[]
-- def-stdout.golden --

[
	if _a.x != _|_ // explicit error (_|_ literal) in source
	{}]
_a: {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants