Skip to content

Commit

Permalink
internal/encoding/yaml: add the new Decoder
Browse files Browse the repository at this point in the history
This implementation builds directly on top of gopkg.in/yaml.v3's Decoder
as opposed to internal/third_party/yaml.Decoder, which forked go-yaml.

There are a number of differences between the old and new decoders.
Starting with the disadvantages or minor regressions per decode_test.go:

* yaml.v3's Node only has each node's start position, lacking any end
  or closing token position. As such, empty lists or objects are always
  decoded into a single line even when the YAML was multi-line.

* yaml.v3's Node does not record precise comment positions, so we must
  assume that any HeadComment lines immediately precede the YAML node.
  In some cases this isn't true, and we remove an empty line.

* yaml.v3's errors do not always contain line information, such as
  the "unknown anchor" errors already covered in our test suite.

There are a number of advantages or improvements, though:

* We no longer lose inline or foot/trailing comments in some cases,
  although we continue to lose comments inside empty objects and lists.

* Floating point scalars which are valid CUE integral literals
  are now decoded as `number & N` rather than `float & N`,
  as the latter simply fails, e.g. `float & 123` is an error.

* The test case using simply `v: ! test` works as expected now.

It should also be noted that an empty YAML input or document decodes
as a nil ast.Expr in both the old and new decoder, even though the old
decoder seemingly intended to decode empty input as a CUE null literal.
It returned that null literal alongside an io.EOF error,
which consumers such as internal/encoding and pkg/encoding/yaml ignored
as they stopped looking at ast.Expr result values at the first error.

Add a TODO to consider switching the behavior for empty inputs
to produce a single "null" value instead as a separate change.
And for now, continue with the simple design of never returning io.EOF
alongside any non-nil valid result, to not make changes elsewhere.

The new decoder is disabled by default and can be enabled via
CUE_EXPERIMENT=yamlv3decoder - a follow-up CL will enable it by default.

Updates #3027.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I538725c5157924639d7084228d59e3f4e58c6d23
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1191897
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mvdan committed Apr 17, 2024
1 parent b508b29 commit 9fba4fa
Show file tree
Hide file tree
Showing 9 changed files with 764 additions and 93 deletions.
4 changes: 2 additions & 2 deletions cmd/cue/cmd/import.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 4 additions & 3 deletions encoding/yaml/yaml.go
Expand Up @@ -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"
)

Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions internal/cueexperiment/exp.go
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion internal/cueexperiment/exp_test.go
Expand Up @@ -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))
}
6 changes: 3 additions & 3 deletions internal/encoding/encoding.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 9fba4fa

Please sign in to comment.