diff --git a/cmd/cue/cmd/import.go b/cmd/cue/cmd/import.go index 80a669af4..dfd947dff 100644 --- a/cmd/cue/cmd/import.go +++ b/cmd/cue/cmd/import.go @@ -36,7 +36,7 @@ import ( "cuelang.org/go/encoding/json" "cuelang.org/go/encoding/protobuf" "cuelang.org/go/internal" - "cuelang.org/go/internal/third_party/yaml" + pkgyaml "cuelang.org/go/pkg/encoding/yaml" ) func newImportCmd(c *Command) *cobra.Command { @@ -590,7 +590,7 @@ func tryParse(str string) (s ast.Expr, pkg string) { return nil, "" } - if expr, err := yaml.Unmarshal("", b); err == nil { + if expr, err := pkgyaml.Unmarshal(b); err == nil { switch expr.(type) { case *ast.StructLit, *ast.ListLit: default: diff --git a/encoding/yaml/yaml.go b/encoding/yaml/yaml.go index d24872e76..118f086b8 100644 --- a/encoding/yaml/yaml.go +++ b/encoding/yaml/yaml.go @@ -23,7 +23,7 @@ import ( "cuelang.org/go/cue" "cuelang.org/go/cue/ast" cueyaml "cuelang.org/go/internal/encoding/yaml" - "cuelang.org/go/internal/third_party/yaml" + "cuelang.org/go/internal/source" pkgyaml "cuelang.org/go/pkg/encoding/yaml" ) @@ -33,11 +33,12 @@ import ( // src is nil, the result of reading the file specified by filename will // be used. func Extract(filename string, src interface{}) (*ast.File, error) { - a := []ast.Expr{} - d, err := yaml.NewDecoder(filename, src) + data, err := source.ReadAll(filename, src) if err != nil { return nil, err } + a := []ast.Expr{} + d := cueyaml.NewDecoder(filename, data) for { expr, err := d.Decode() if err != nil { diff --git a/internal/cueexperiment/exp.go b/internal/cueexperiment/exp.go index 302b6ac20..3ee658afe 100644 --- a/internal/cueexperiment/exp.go +++ b/internal/cueexperiment/exp.go @@ -10,6 +10,10 @@ import ( // by Init. var Flags struct { Modules bool + + // YAMLV3Decoder swaps the old internal/third_party/yaml decoder with the new + // decoder implemented in internal/encoding/yaml on top of yaml.v3. + YAMLV3Decoder bool } // Init initializes Flags. Note: this isn't named "init" because we diff --git a/internal/cueexperiment/exp_test.go b/internal/cueexperiment/exp_test.go index 760eede6a..e75a6e847 100644 --- a/internal/cueexperiment/exp_test.go +++ b/internal/cueexperiment/exp_test.go @@ -8,8 +8,9 @@ import ( func TestInit(t *testing.T) { // This is just a smoke test to make sure it's all wired up OK. - t.Setenv("CUE_EXPERIMENT", "modules") + t.Setenv("CUE_EXPERIMENT", "modules,yamlv3decoder") err := Init() qt.Assert(t, qt.IsNil(err)) qt.Assert(t, qt.IsTrue(Flags.Modules)) + qt.Assert(t, qt.IsTrue(Flags.YAMLV3Decoder)) } diff --git a/internal/encoding/encoding.go b/internal/encoding/encoding.go index 1f451e7d9..84ac9f52a 100644 --- a/internal/encoding/encoding.go +++ b/internal/encoding/encoding.go @@ -38,9 +38,9 @@ import ( "cuelang.org/go/encoding/protobuf/jsonpb" "cuelang.org/go/encoding/protobuf/textproto" "cuelang.org/go/internal" + "cuelang.org/go/internal/encoding/yaml" "cuelang.org/go/internal/filetypes" "cuelang.org/go/internal/source" - "cuelang.org/go/internal/third_party/yaml" "golang.org/x/text/encoding/unicode" "golang.org/x/text/transform" ) @@ -251,9 +251,9 @@ func NewDecoder(f *build.File, cfg *Config) *Decoder { i.next = json.NewDecoder(nil, path, r).Extract i.Next() case build.YAML: - d, err := yaml.NewDecoder(path, r) + b, err := io.ReadAll(r) i.err = err - i.next = d.Decode + i.next = yaml.NewDecoder(path, b).Decode i.Next() case build.Text: b, err := io.ReadAll(r) diff --git a/internal/encoding/yaml/decode.go b/internal/encoding/yaml/decode.go new file mode 100644 index 000000000..5c6889255 --- /dev/null +++ b/internal/encoding/yaml/decode.go @@ -0,0 +1,675 @@ +package yaml + +import ( + "bytes" + "encoding/base64" + "errors" + "fmt" + "io" + "strconv" + "strings" + + "gopkg.in/yaml.v3" + + "cuelang.org/go/cue/ast" + "cuelang.org/go/cue/literal" + "cuelang.org/go/cue/token" + "cuelang.org/go/internal" + "cuelang.org/go/internal/cueexperiment" + tpyaml "cuelang.org/go/internal/third_party/yaml" +) + +// TODO(mvdan): we should sanity check that the decoder always produces valid CUE, +// as it is possible to construct a cue/ast syntax tree with invalid literals +// or with expressions that will always error, such as `float & 123`. +// +// One option would be to do this as part of the tests; a more general approach +// may be fuzzing, which would find more bugs and work for any decoder, +// although it may be slow as we need to involve the evaluator. + +// Decoder is a temporary interface compatible with both the old and new yaml decoders. +type Decoder interface { + // Decode consumes a YAML value and returns it in CUE syntax tree node. + Decode() (ast.Expr, error) +} + +// NewDecoder is a temporary constructor compatible with both the old and new yaml decoders. +// Note that the signature matches the new yaml decoder, as the old signature can only error +// when reading a source that isn't []byte. +func NewDecoder(filename string, b []byte) Decoder { + if cueexperiment.Flags.YAMLV3Decoder { + return newDecoder(filename, b) + } + dec, err := tpyaml.NewDecoder(filename, b) + if err != nil { + panic(err) // should never happen as we give it []byte + } + return dec +} + +// decoder wraps a [yaml.Decoder] to extract CUE syntax tree nodes. +type decoder struct { + yamlDecoder yaml.Decoder + + // yamlNonEmpty is true once yamlDecoder tells us the input YAML wasn't empty. + // Useful so that we can extract "null" when the input is empty. + yamlNonEmpty bool + + // decodeErr is returned by any further calls to Decode when not nil. + decodeErr error + + tokFile *token.File + tokLines []int + + // pendingHeadComments collects the head (preceding) comments + // from the YAML nodes we are extracting. + // We can't add comments to a CUE syntax tree node until we've created it, + // but we need to extract these comments first since they have earlier positions. + pendingHeadComments []*ast.Comment + + // extractingAliases ensures we don't loop forever when expanding YAML anchors. + extractingAliases map[*yaml.Node]bool + + // lastPos is the last YAML node position that we decoded, + // used for working out relative positions such as token.NewSection. + // This position can only increase, moving forward in the file. + lastPos token.Position + + // forceNewline ensures that the next position will be on a new line. + forceNewline bool +} + +// TODO(mvdan): this can be io.Reader really, except that token.Pos is offset-based, +// so the only way to really have true Offset+Line+Col numbers is to know +// the size of the entire YAML node upfront. +// With json we can use RawMessage to know the size of the input +// before we extract into ast.Expr, but unfortunately, yaml.Node has no size. + +// newDecoder creates a decoder for YAML values to extract CUE syntax tree nodes. +// +// The filename is used for position information in CUE syntax tree nodes +// as well as any errors encountered while decoding YAML. +func newDecoder(filename string, b []byte) *decoder { + // Note that yaml.v3 can insert a null node just past the end of the input + // in some edge cases, so we pretend that there's an extra newline + // so that we don't panic when handling such a position. + tokFile := token.NewFile(filename, 0, len(b)+1) + tokFile.SetLinesForContent(b) + return &decoder{ + tokFile: tokFile, + tokLines: append(tokFile.Lines(), len(b)), + yamlDecoder: *yaml.NewDecoder(bytes.NewReader(b)), + } +} + +// Decode consumes a YAML value and returns it in CUE syntax tree node. +// +// A nil node with an io.EOF error is returned once no more YAML values +// are available for decoding. +func (d *decoder) Decode() (ast.Expr, error) { + if err := d.decodeErr; err != nil { + return nil, err + } + var yn yaml.Node + if err := d.yamlDecoder.Decode(&yn); err != nil { + if err == io.EOF { + // Any further Decode calls must return EOF to avoid an endless loop. + d.decodeErr = io.EOF + + // If the input is empty, we produce a single null literal with EOF. + // Note that when the input contains "---", we get an empty document + // with a null scalar value inside instead. + // + // TODO(mvdan): the old decoder seemingly intended to do this, + // but returned a "null" literal with io.EOF, which consumers ignored. + if false && !d.yamlNonEmpty { + return &ast.BasicLit{ + Kind: token.NULL, + Value: "null", + }, nil + } + // If the input wasn't empty, we already decoded some CUE syntax nodes, + // so here we should just return io.EOF to stop. + return nil, io.EOF + } + // Unfortunately, yaml.v3's syntax errors are opaque strings, + // and they only include line numbers in some but not all cases. + // TODO(mvdan): improve upstream's errors so they are structured + // and always contain some position information. + e := err.Error() + if s, ok := strings.CutPrefix(e, "yaml: line "); ok { + // From "yaml: line 3: some issue" to "foo.yaml:3: some issue". + e = d.tokFile.Name() + ":" + s + } else if s, ok := strings.CutPrefix(e, "yaml:"); ok { + // From "yaml: some issue" to "foo.yaml: some issue". + e = d.tokFile.Name() + ":" + s + } else { + return nil, err + } + err = errors.New(e) + // Any further Decode calls repeat this error. + d.decodeErr = err + return nil, err + } + d.yamlNonEmpty = true + return d.extract(&yn) +} + +// Unmarshal parses a single YAML value to a CUE expression. +func Unmarshal(filename string, data []byte) (ast.Expr, error) { + d := NewDecoder(filename, data) + x, err := d.Decode() + if err != nil { + if err == io.EOF { + return nil, nil // empty input + } + return nil, err + } + // TODO(mvdan): fail if there are more documents or garbage in the input + return x, nil +} + +func (d *decoder) extract(yn *yaml.Node) (ast.Expr, error) { + d.addHeadCommentsToPending(yn) + var expr ast.Expr + var err error + switch yn.Kind { + case yaml.DocumentNode: + expr, err = d.document(yn) + case yaml.SequenceNode: + expr, err = d.sequence(yn) + case yaml.MappingNode: + expr, err = d.mapping(yn) + case yaml.ScalarNode: + expr, err = d.scalar(yn) + case yaml.AliasNode: + expr, err = d.alias(yn) + default: + return nil, d.posErrorf(yn, "unknown yaml node kind: %d", yn.Kind) + } + if err != nil { + return nil, err + } + d.addCommentsToNode(expr, yn, 1) + return expr, nil +} + +// comments parses a newline-delimited list of YAML "#" comments +// and turns them into a list of cue/ast comments. +func (d *decoder) comments(src string) []*ast.Comment { + if src == "" { + return nil + } + var comments []*ast.Comment + for _, line := range strings.Split(src, "\n") { + if line == "" { + continue // yaml.v3 comments have a trailing newline at times + } + comments = append(comments, &ast.Comment{ + // Trim the leading "#". + // Note that yaml.v3 does not give us comment positions. + Text: "//" + line[1:], + }) + } + return comments +} + +// addHeadCommentsToPending parses a node's head comments and adds them to a pending list, +// to be used later by addComments once a cue/ast node is constructed. +func (d *decoder) addHeadCommentsToPending(yn *yaml.Node) { + comments := d.comments(yn.HeadComment) + // TODO(mvdan): once yaml.v3 records comment positions, + // we can better ensure that sections separated by empty lines are kept that way. + // For now, all we can do is approximate by counting lines, + // and assuming that head comments are not separated from their node. + // This will be wrong in some cases, moving empty lines, but is better than nothing. + if len(d.pendingHeadComments) == 0 && len(comments) > 0 { + c := comments[0] + if d.lastPos.IsValid() && (yn.Line-len(comments))-d.lastPos.Line >= 2 { + c.Slash = c.Slash.WithRel(token.NewSection) + } + } + d.pendingHeadComments = append(d.pendingHeadComments, comments...) +} + +// addCommentsToNode adds any pending head comments, plus a YAML node's line +// and foot comments, to a cue/ast node. +func (d *decoder) addCommentsToNode(n ast.Node, yn *yaml.Node, linePos int8) { + // cue/ast and cue/format are not able to attach a comment to a node + // when the comment immediately follows the node. + // For some nodes like fields, the best we can do is move the comments up. + // For the root-level struct, we do want to leave comments + // at the end of the document to be left at the very end. + // + // TODO(mvdan): can we do better? for example, support attaching trailing comments to a cue/ast.Node? + footComments := d.comments(yn.FootComment) + if _, ok := n.(*ast.StructLit); !ok { + d.pendingHeadComments = append(d.pendingHeadComments, footComments...) + footComments = nil + } + if comments := d.pendingHeadComments; len(comments) > 0 { + ast.AddComment(n, &ast.CommentGroup{ + Doc: true, + Position: 0, + List: comments, + }) + } + if comments := d.comments(yn.LineComment); len(comments) > 0 { + ast.AddComment(n, &ast.CommentGroup{ + Line: true, + Position: linePos, + List: comments, + }) + } + if comments := footComments; len(comments) > 0 { + ast.AddComment(n, &ast.CommentGroup{ + // After 100 tokens, so that the comment goes after the entire node. + // TODO(mvdan): this is hacky, can the cue/ast API support trailing comments better? + Position: 100, + List: comments, + }) + } + d.pendingHeadComments = nil +} + +func (d *decoder) posErrorf(yn *yaml.Node, format string, args ...any) error { + // TODO(mvdan): use columns as well; for now they are left out to avoid test churn + // return fmt.Errorf(d.pos(n).String()+" "+format, args...) + return fmt.Errorf(d.tokFile.Name()+":"+strconv.Itoa(yn.Line)+": "+format, args...) +} + +// pos converts a YAML node position to a cue/ast position. +// Note that this method uses and updates the last position in lastPos, +// so it should be called on YAML nodes in increasing position order. +func (d *decoder) pos(yn *yaml.Node) token.Pos { + // Calculate the position's offset via the line and column numbers. + offset := d.tokLines[yn.Line-1] + (yn.Column - 1) + pos := d.tokFile.Pos(offset, token.NoRelPos) + + if d.forceNewline { + d.forceNewline = false + pos = pos.WithRel(token.Newline) + } else if d.lastPos.IsValid() { + switch { + case yn.Line-d.lastPos.Line >= 2: + pos = pos.WithRel(token.NewSection) + case yn.Line-d.lastPos.Line == 1: + pos = pos.WithRel(token.Newline) + case yn.Column-d.lastPos.Column > 0: + pos = pos.WithRel(token.Blank) + default: + pos = pos.WithRel(token.NoSpace) + } + // If for any reason the node's position is before the last position, + // give up and return an empty position. Akin to: yn.Pos().Before(d.lastPos) + // + // TODO(mvdan): Brought over from the old decoder; when does this happen? + // Can we get rid of those edge cases and this bit of logic? + if yn.Line < d.lastPos.Line || (yn.Line == d.lastPos.Line && yn.Column < d.lastPos.Column) { + return token.NoPos + } + } + d.lastPos = token.Position{Line: yn.Line, Column: yn.Column} + return pos +} + +func (d *decoder) document(yn *yaml.Node) (ast.Expr, error) { + if n := len(yn.Content); n != 1 { + return nil, d.posErrorf(yn, "yaml document nodes are meant to have one content node but have %d", n) + } + return d.extract(yn.Content[0]) +} + +func (d *decoder) sequence(yn *yaml.Node) (ast.Expr, error) { + list := &ast.ListLit{ + Lbrack: d.pos(yn).WithRel(token.Blank), + } + multiline := false + if len(yn.Content) > 0 { + multiline = yn.Line < yn.Content[len(yn.Content)-1].Line + } + + // If a list is empty, or ends with a struct, the closing `]` is on the same line. + closeSameLine := true + for _, c := range yn.Content { + d.forceNewline = multiline + elem, err := d.extract(c) + if err != nil { + return nil, err + } + list.Elts = append(list.Elts, elem) + // A list of structs begins with `[{`, so let it end with `}]`. + _, closeSameLine = elem.(*ast.StructLit) + } + if multiline && !closeSameLine { + list.Rbrack = list.Rbrack.WithRel(token.Newline) + } + return list, nil +} + +func (d *decoder) mapping(yn *yaml.Node) (ast.Expr, error) { + strct := &ast.StructLit{} + multiline := false + if len(yn.Content) > 0 { + multiline = yn.Line < yn.Content[len(yn.Content)-1].Line + } + + if err := d.insertMap(yn, strct, multiline, false); err != nil { + return nil, err + } + // TODO(mvdan): moving these positions above insertMap breaks a few tests, why? + strct.Lbrace = d.pos(yn).WithRel(token.Blank) + if multiline { + strct.Rbrace = strct.Lbrace.WithRel(token.Newline) + } else { + strct.Rbrace = strct.Lbrace + } + return strct, nil +} + +func (d *decoder) insertMap(yn *yaml.Node, m *ast.StructLit, multiline, mergeValues bool) error { + l := len(yn.Content) +outer: + for i := 0; i < l; i += 2 { + if multiline { + d.forceNewline = true + } + yk, yv := yn.Content[i], yn.Content[i+1] + d.addHeadCommentsToPending(yk) + if isMerge(yk) { + mergeValues = true + if err := d.merge(yv, m, multiline); err != nil { + return err + } + continue + } + if yk.Kind != yaml.ScalarNode { + return d.posErrorf(yn, "invalid map key: %v", yk.ShortTag()) + } + + field := &ast.Field{} + label, err := d.label(yk) + if err != nil { + return err + } + d.addCommentsToNode(label, yk, 1) + field.Label = label + + if mergeValues { + key := labelStr(label) + for _, decl := range m.Elts { + f := decl.(*ast.Field) + name, _, err := ast.LabelName(f.Label) + if err == nil && name == key { + f.Value, err = d.extract(yv) + if err != nil { + return err + } + continue outer + } + } + } + + value, err := d.extract(yv) + if err != nil { + return err + } + field.Value = value + + m.Elts = append(m.Elts, field) + } + return nil +} + +func (d *decoder) merge(yn *yaml.Node, m *ast.StructLit, multiline bool) error { + switch yn.Kind { + case yaml.MappingNode: + return d.insertMap(yn, m, multiline, true) + case yaml.AliasNode: + return d.insertMap(yn.Alias, m, multiline, true) + case yaml.SequenceNode: + // Step backwards as earlier nodes take precedence. + for i := len(yn.Content) - 1; i >= 0; i-- { + if err := d.merge(yn.Content[i], m, multiline); err != nil { + return err + } + } + return nil + default: + return d.posErrorf(yn, "map merge requires map or sequence of maps as the value") + } +} + +func (d *decoder) label(yn *yaml.Node) (ast.Label, error) { + pos := d.pos(yn) + + expr, err := d.scalar(yn) + if err != nil { + return nil, err + } + switch expr := expr.(type) { + case *ast.BasicLit: + if expr.Kind == token.STRING { + if ast.IsValidIdent(yn.Value) && !internal.IsDefOrHidden(yn.Value) { + return &ast.Ident{ + NamePos: pos, + Name: yn.Value, + }, nil + } + ast.SetPos(expr, pos) + return expr, nil + } + + return &ast.BasicLit{ + ValuePos: pos, + Kind: token.STRING, + Value: literal.Label.Quote(expr.Value), + }, nil + + default: + return nil, d.posErrorf(yn, "invalid label "+yn.Value) + } +} + +const ( + // TODO(mvdan): The strings below are from yaml.v3; should we be relying on upstream somehow? + nullTag = "!!null" + boolTag = "!!bool" + strTag = "!!str" + intTag = "!!int" + floatTag = "!!float" + timestampTag = "!!timestamp" + seqTag = "!!seq" + mapTag = "!!map" + binaryTag = "!!binary" + mergeTag = "!!merge" +) + +func (d *decoder) scalar(yn *yaml.Node) (ast.Expr, error) { + switch tag := yn.ShortTag(); tag { + // TODO: use parse literal or parse expression instead. + case timestampTag: + return &ast.BasicLit{ + ValuePos: d.pos(yn), + Kind: token.STRING, + Value: literal.String.Quote(yn.Value), + }, nil + case strTag: + return &ast.BasicLit{ + ValuePos: d.pos(yn), + Kind: token.STRING, + Value: quoteString(yn.Value), + }, nil + + case binaryTag: + data, err := base64.StdEncoding.DecodeString(yn.Value) + if err != nil { + return nil, d.posErrorf(yn, "!!binary value contains invalid base64 data") + } + return &ast.BasicLit{ + ValuePos: d.pos(yn), + Kind: token.STRING, + Value: literal.Bytes.Quote(string(data)), + }, nil + + case boolTag: + t := false + switch yn.Value { + // TODO(mvdan): The strings below are from yaml.v3; should we be relying on upstream somehow? + case "true", "True", "TRUE": + t = true + } + lit := ast.NewBool(t) + lit.ValuePos = d.pos(yn) + return lit, nil + + case intTag: + // Convert YAML octal to CUE octal. If YAML accepted an invalid + // integer, just convert it as well to ensure CUE will fail. + value := yn.Value + if len(value) > 1 && value[0] == '0' && value[1] <= '9' { + value = "0o" + value[1:] + } + var info literal.NumInfo + // We make the assumption that any valid YAML integer literal will be a valid + // CUE integer literal as well, with the only exception of octal numbers above. + // Note that `!!int 123.456` is not allowed. + if err := literal.ParseNum(value, &info); err != nil || !info.IsInt() { + return nil, d.posErrorf(yn, "cannot decode %q as %s", value, yn.ShortTag()) + } + return d.makeNum(yn, value, token.INT), nil + + case floatTag: + value := yn.Value + // TODO(mvdan): The strings below are from yaml.v3; should we be relying on upstream somehow? + switch value { + case ".inf", ".Inf", ".INF", "+.inf", "+.Inf", "+.INF": + value = "+Inf" + case "-.inf", "-.Inf", "-.INF": + value = "-Inf" + case ".nan", ".NaN", ".NAN": + value = "NaN" + default: + var info literal.NumInfo + // We make the assumption that any valid YAML float literal will be a valid + // CUE float literal as well, with the only exception of Inf/NaN above. + // Note that `!!float 123` is allowed. + if err := literal.ParseNum(value, &info); err != nil { + return nil, d.posErrorf(yn, "cannot decode %q as %s", value, yn.ShortTag()) + } + // If the decoded YAML scalar was explicitly or implicitly a float, + // and the scalar literal looks like an integer, + // unify it with "number" to record the fact that it was represented as a float. + // Don't unify with float, as `float & 123` is invalid, and there's no need + // to forbid representing the number as an integer either. + if yn.Tag != "" { + if p := strings.IndexAny(value, ".eEiInN"); p == -1 { + // TODO: number(v) when we have conversions + // TODO(mvdan): don't shove the unification inside a BasicLit.Value string + // + // TODO(mvdan): would it be better to do turn `!!float 123` into `123.0` + // rather than `number & 123`? Note that `float & 123` is an error. + value = fmt.Sprintf("number & %s", value) + } + } + } + return d.makeNum(yn, value, token.FLOAT), nil + + case nullTag: + return &ast.BasicLit{ + ValuePos: d.pos(yn).WithRel(token.Blank), + Kind: token.NULL, + Value: "null", + }, nil + default: + return nil, d.posErrorf(yn, "cannot unmarshal tag %q", tag) + } +} + +func (d *decoder) makeNum(yn *yaml.Node, val string, kind token.Token) (expr ast.Expr) { + val, negative := strings.CutPrefix(val, "-") + expr = &ast.BasicLit{ + ValuePos: d.pos(yn), + Kind: kind, + Value: val, + } + if negative { + expr = &ast.UnaryExpr{ + OpPos: d.pos(yn), + Op: token.SUB, + X: expr, + } + } + return expr +} + +func (d *decoder) alias(yn *yaml.Node) (ast.Expr, error) { + if d.extractingAliases[yn] { + // TODO this could actually be allowed in some circumstances. + return nil, d.posErrorf(yn, "anchor %q value contains itself", yn.Value) + } + if d.extractingAliases == nil { + d.extractingAliases = make(map[*yaml.Node]bool) + } + d.extractingAliases[yn] = true + var node ast.Expr + node, err := d.extract(yn.Alias) + delete(d.extractingAliases, yn) + return node, err +} + +// quoteString converts a string to a CUE multiline string if needed. +// TODO(mvdan): this is brought over from the old decoder; we should consider +// polishing this API and moving it someplace better like cue/literal. +func quoteString(s string) string { + lines := []string{} + last := 0 + for i, c := range s { + if c == '\n' { + lines = append(lines, s[last:i]) + last = i + 1 + } + if c == '\r' { + goto quoted + } + } + lines = append(lines, s[last:]) + if len(lines) >= 2 { + buf := []byte{} + buf = append(buf, `"""`+"\n"...) + for _, l := range lines { + if l == "" { + // no indentation for empty lines + buf = append(buf, '\n') + continue + } + buf = append(buf, '\t') + p := len(buf) + // TODO(mvdan): do not use Go's strconv for CUE syntax. + buf = strconv.AppendQuote(buf, l) + // remove quotes + buf[p] = '\t' + buf[len(buf)-1] = '\n' + } + buf = append(buf, "\t\t"+`"""`...) + return string(buf) + } +quoted: + return literal.String.Quote(s) +} + +func labelStr(l ast.Label) string { + switch l := l.(type) { + case *ast.Ident: + return l.Name + case *ast.BasicLit: + s, _ := literal.Unquote(l.Value) + return s + } + return "" +} + +func isMerge(yn *yaml.Node) bool { + // TODO(mvdan): The boolean logic below is from yaml.v3; should we be relying on upstream somehow? + return yn.Kind == yaml.ScalarNode && yn.Value == "<<" && (yn.Tag == "" || yn.Tag == "!" || yn.ShortTag() == mergeTag) +} diff --git a/internal/encoding/yaml/decode_test.go b/internal/encoding/yaml/decode_test.go index 6e69f38fe..768f0b851 100644 --- a/internal/encoding/yaml/decode_test.go +++ b/internal/encoding/yaml/decode_test.go @@ -28,10 +28,17 @@ import ( "cuelang.org/go/cue/ast" "cuelang.org/go/cue/format" + "cuelang.org/go/internal" + "cuelang.org/go/internal/cueexperiment" "cuelang.org/go/internal/cuetest" - "cuelang.org/go/internal/third_party/yaml" + "cuelang.org/go/internal/encoding/yaml" + "github.com/google/go-cmp/cmp" ) +// These tests are only for the new YAML decoder. +// The old YAML decoder has its own tests in internal/third_party/yaml. +func init() { cueexperiment.Flags.YAMLV3Decoder = true } + var unmarshalTests = []struct { data string want string @@ -234,7 +241,7 @@ apple: "newline"`, \ttext - """`, + """ // Comment`, }, // Folded block scalar @@ -249,7 +256,7 @@ apple: "newline"`, last line - """`, + """ // Comment`, }, // Structs @@ -375,7 +382,7 @@ Null: 1 }, { "float32_maxuint64+1: 18446744073709551616", - `"float32_maxuint64+1": 18446744073709551616`, + `"float32_maxuint64+1": number & 18446744073709551616`, }, // float64 @@ -391,9 +398,13 @@ Null: 1 "float64_maxuint64: 18446744073709551615", "float64_maxuint64: 18446744073709551615", }, + // TODO(mvdan): yaml.v3 uses strconv APIs like ParseUint to try to detect + // whether a scalar should be considered a YAML integer or a float. + // Integers in CUE aren't limited to 64 bits, so we should arguably not decode + // large integers that don't fit in 64 bits as floats via `number &`. { "float64_maxuint64+1: 18446744073709551616", - `"float64_maxuint64+1": 18446744073709551616`, + `"float64_maxuint64+1": number & 18446744073709551616`, }, // Overflow cases. @@ -429,10 +440,10 @@ Null: 1 "v: 1.1", }, { "v: !!float 0", - "v: float & 0", // Should this be 0.0? + "v: number & 0", }, { "v: !!float -1", - "v: float & -1", // Should this be -1.0? + "v: number & -1", }, { "v: !!null ''", "v: null", @@ -444,8 +455,7 @@ Null: 1 // Non-specific tag (Issue #75) { `v: ! test`, - // TODO this should work and produce a string. - "", + `v: "test"`, }, // Anchors and aliases. @@ -567,12 +577,11 @@ d: [ ] e: [] `, + // TODO(mvdan): keep the separated opening/closing tokens once yaml.v3 exposes end positions. `a: {} b: {} - c: 1 -d: [ -] +d: [] e: []`, }, @@ -675,14 +684,15 @@ a: }, // Floating comments. - // TODO: avoid losing some of these. + // TODO(mvdan): all empty lines separating comments should stay in place. + // TODO(mvdan): avoid losing comments in empty lists and objects. { "# Start\n\na: 123\n\n# Middle\n\nb: 456\n\n# End", - "// Start\n\na: 123\n\n// Middle\n\nb: 456", + "// Start\na: 123\n\n// Middle\nb: 456\n// End", }, { "a: [\n\t# Comment\n]", - "a: [\n\n]", + "a: []", }, { "a: {\n\t# Comment\n}", @@ -692,7 +702,7 @@ a: // Attached comments. { "start: 100\n\n# Before\na: 123 # Inline\n# After\n\nend: 200", - "start: 100\n\n// Before\na: 123 // Inline\n// After\n\nend: 200", + "start: 100\n\n// Before\n// After\na: 123 // Inline\nend: 200", }, { "# One\none: null\n\n# Two\ntwo: [2, 2]\n\n# Three\nthree: {val: 3}", @@ -755,6 +765,8 @@ a: "Reuse anchor": "Bar"`, }, // Single document with garbage following it. + // TODO(mvdan): This should work for a single Decoder.Decode call, + // but it should be an error with Unmarshal. { "---\nhello\n...\n}not yaml", `"hello"`, @@ -764,24 +776,22 @@ a: type M map[interface{}]interface{} func cueStr(node ast.Node) string { - if s, ok := node.(*ast.StructLit); ok { - node = &ast.File{ - Decls: s.Elts, - } + if node == nil { + return "" } - b, _ := format.Node(node) + b, _ := format.Node(internal.ToFile(node)) return strings.TrimSpace(string(b)) } -func newDecoder(t *testing.T, data string) *yaml.Decoder { - dec, err := yaml.NewDecoder("test.yaml", strings.NewReader(data)) - if err != nil { - t.Fatal(err) - } - return dec +func newDecoder(t *testing.T, data string) yaml.Decoder { + t.Helper() + t.Logf("input yaml:\n%s", data) + return yaml.NewDecoder("test.yaml", []byte(data)) } func callUnmarshal(t *testing.T, data string) (ast.Expr, error) { + t.Helper() + t.Logf(" yaml:\n%s", data) return yaml.Unmarshal("test.yaml", []byte(data)) } @@ -790,11 +800,11 @@ func TestUnmarshal(t *testing.T) { t.Run(strconv.Itoa(i), func(t *testing.T) { t.Logf("test %d: %q", i, item.data) expr, err := callUnmarshal(t, item.data) - if _, ok := err.(*yaml.TypeError); !ok && err != nil { - t.Fatal("expected error to be nil") + if err != nil { + t.Fatalf("expected error to be nil: %v", err) } if got := cueStr(expr); got != item.want { - t.Errorf("\n got:\n%v\nwant:\n%v", got, item.want) + t.Errorf("\n got:\n%v\n want:\n%v", got, item.want) } }) } @@ -810,7 +820,7 @@ func TestX(t *testing.T) { } expr, err := callUnmarshal(t, y) - if _, ok := err.(*yaml.TypeError); !ok && err != nil { + if err != nil { t.Fatal(err) } t.Error(cueStr(expr)) @@ -826,11 +836,11 @@ func TestDecoderSingleDocument(t *testing.T) { return } expr, err := newDecoder(t, item.data).Decode() - if _, ok := err.(*yaml.TypeError); !ok && err != nil { + if err != nil { t.Errorf("err should be nil, was %v", err) } if got := cueStr(expr); got != item.want { - t.Errorf("\n got: %v;\nwant: %v", got, item.want) + t.Errorf("\n got:\n%v\n want:\n%v", got, item.want) } }) } @@ -876,7 +886,7 @@ func TestDecoder(t *testing.T) { } got := strings.Join(values, "\n") if got != item.want { - t.Errorf("\n got: %v;\nwant: %v", got, item.want) + t.Errorf("\n got:\n%v\n want:\n%v", got, item.want) } }) } @@ -897,20 +907,20 @@ func TestUnmarshalNaN(t *testing.T) { var unmarshalErrorTests = []struct { data, error string }{ - {"\nv: !!float 'error'", "test.yaml:2: cannot decode !!str `error` as a !!float"}, - {"\nv: !!int 'error'", "test.yaml:2: cannot decode !!str `error` as a !!int"}, - {"\nv: !!int 123.456", "test.yaml:2: cannot decode !!float `123.456` as a !!int"}, + {"\nv: !!float 'error'", `test.yaml:2: cannot decode "error" as !!float`}, + {"\nv: !!int 'error'", `test.yaml:2: cannot decode "error" as !!int`}, + {"\nv: !!int 123.456", `test.yaml:2: cannot decode "123.456" as !!int`}, {"v: [A,", "test.yaml:1: did not find expected node content"}, {"v:\n- [A,", "test.yaml:2: did not find expected node content"}, {"a:\n- b: *,", "test.yaml:2: did not find expected alphabetic or numeric character"}, - {"a: *b\n", "test.yaml:1: unknown anchor 'b' referenced"}, - {"a: &a\n b: *a\n", "test.yaml:2: anchor 'a' value contains itself"}, - {"value: -", "test.yaml:1: block sequence entries are not allowed in this context"}, + {"a: *b\n", "test.yaml: unknown anchor 'b' referenced"}, + {"a: &a\n b: *a\n", `test.yaml:2: anchor "a" value contains itself`}, + {"value: -", "test.yaml: block sequence entries are not allowed in this context"}, {"a: !!binary ==", "test.yaml:1: !!binary value contains invalid base64 data"}, - {"{[.]}", `test.yaml:1: invalid map key: sequence`}, - {"{{.}}", `test.yaml:1: invalid map key: map`}, - {"b: *a\na: &a {c: 1}", `test.yaml:1: unknown anchor 'a' referenced`}, - {"%TAG !%79! tag:yaml.org,2002:\n---\nv: !%79!int '1'", "test.yaml:1: did not find expected whitespace"}, + {"{[.]}", `test.yaml:1: invalid map key: !!seq`}, + {"{{.}}", `test.yaml:1: invalid map key: !!map`}, + {"b: *a\na: &a {c: 1}", `test.yaml: unknown anchor 'a' referenced`}, + {"%TAG !%79! tag:yaml.org,2002:\n---\nv: !%79!int '1'", "test.yaml: did not find expected whitespace"}, } func TestUnmarshalErrors(t *testing.T) { @@ -963,7 +973,7 @@ func TestFiles(t *testing.T) { t.Fatal(err) } if want := string(b); got != want { - t.Errorf("\n got: %v;\nwant: %v", got, want) + t.Error(cmp.Diff(want, got)) } }) } diff --git a/internal/encoding/yaml/testdata/merge.out b/internal/encoding/yaml/testdata/merge.out index ee6e6bbe0..bc55dde7a 100644 --- a/internal/encoding/yaml/testdata/merge.out +++ b/internal/encoding/yaml/testdata/merge.out @@ -13,7 +13,6 @@ anchors: { } // All the following maps are equal: - plain: { // Explicit keys x: 1 @@ -21,56 +20,48 @@ plain: { r: 10 label: "center/big" } - mergeOne: { - x: 1 - y: 2 // Merge one map - + x: 1 + y: 2 r: 10 label: "center/big" } - mergeMultiple: { - r: 10 - x: 1 - y: 2 // Merge multiple maps - + r: 10 + x: 1 + y: 2 label: "center/big" } - override: { + // Override r: 10 x: 1 y: 2 label: "center/big" } - shortTag: { - r: 10 - x: 1 - y: 2 // Explicit short merge tag - + r: 10 + x: 1 + y: 2 label: "center/big" } - longTag: { - r: 10 - x: 1 - y: 2 // Explicit merge long tag - + r: 10 + x: 1 + y: 2 label: "center/big" } - inlineMap: { // Inlined map - x: 1, y: 2, r: 10 + x: 1 + y: 2 + r: 10 label: "center/big" } - inlineSequenceMap: { // Inlined map in sequence r: 10 diff --git a/pkg/encoding/yaml/manual.go b/pkg/encoding/yaml/manual.go index 5912c48dc..c65c3621e 100644 --- a/pkg/encoding/yaml/manual.go +++ b/pkg/encoding/yaml/manual.go @@ -23,7 +23,6 @@ import ( "cuelang.org/go/cue/errors" cueyaml "cuelang.org/go/internal/encoding/yaml" "cuelang.org/go/internal/pkg" - "cuelang.org/go/internal/third_party/yaml" ) // Marshal returns the YAML encoding of v. @@ -64,16 +63,12 @@ func MarshalStream(v cue.Value) (string, error) { // Unmarshal parses the YAML to a CUE expression. func Unmarshal(data []byte) (ast.Expr, error) { - return yaml.Unmarshal("", data) + return cueyaml.Unmarshal("", data) } // UnmarshalStream parses the YAML to a CUE list expression on success. func UnmarshalStream(data []byte) (ast.Expr, error) { - d, err := yaml.NewDecoder("", data) - if err != nil { - return nil, err - } - + d := cueyaml.NewDecoder("", data) a := []ast.Expr{} for { x, err := d.Decode() @@ -92,10 +87,7 @@ func UnmarshalStream(data []byte) (ast.Expr, error) { // Validate validates YAML and confirms it is an instance of the schema // specified by v. If the YAML source is a stream, every object must match v. func Validate(b []byte, v cue.Value) (bool, error) { - d, err := yaml.NewDecoder("yaml.Validate", b) - if err != nil { - return false, err - } + d := cueyaml.NewDecoder("yaml.Validate", b) r := v.Context() for { expr, err := d.Decode() @@ -141,10 +133,7 @@ func Validate(b []byte, v cue.Value) (bool, error) { // but does not have to be an instance of v. If the YAML source is a stream, // every object must match v. func ValidatePartial(b []byte, v cue.Value) (bool, error) { - d, err := yaml.NewDecoder("yaml.ValidatePartial", b) - if err != nil { - return false, err - } + d := cueyaml.NewDecoder("yaml.ValidatePartial", b) r := v.Context() for { expr, err := d.Decode()