diff --git a/cmd/cue/cmd/fix.go b/cmd/cue/cmd/fix.go index be5370046..eccab530f 100644 --- a/cmd/cue/cmd/fix.go +++ b/cmd/cue/cmd/fix.go @@ -15,20 +15,17 @@ package cmd import ( - "fmt" "io/ioutil" "os" "path/filepath" "strings" "cuelang.org/go/cue/ast" - "cuelang.org/go/cue/ast/astutil" "cuelang.org/go/cue/errors" "cuelang.org/go/cue/format" "cuelang.org/go/cue/load" - "cuelang.org/go/cue/parser" "cuelang.org/go/cue/token" - "cuelang.org/go/internal" + "cuelang.org/go/tools/fix" "github.com/spf13/cobra" ) @@ -87,7 +84,7 @@ func runFixAll(cmd *Command, args []string) error { } } - errs := fixAll(instances) + errs := fix.Instances(instances) if errs != nil && flagForce.Bool(cmd) { return errs @@ -129,238 +126,3 @@ func appendDirs(a []string, base string) []string { }) return a } - -func fix(f *ast.File) *ast.File { - // Isolate bulk optional fields into a single struct. - ast.Walk(f, func(n ast.Node) bool { - var decls []ast.Decl - switch x := n.(type) { - case *ast.StructLit: - decls = x.Elts - case *ast.File: - decls = x.Decls - } - - if len(decls) <= 1 { - return true - } - - for i, d := range decls { - if internal.IsBulkField(d) { - decls[i] = internal.EmbedStruct(ast.NewStruct(d)) - } - } - - return true - }, nil) - - // Rewrite an old-style alias to a let clause. - ast.Walk(f, func(n ast.Node) bool { - var decls []ast.Decl - switch x := n.(type) { - case *ast.StructLit: - decls = x.Elts - case *ast.File: - decls = x.Decls - } - for i, d := range decls { - if a, ok := d.(*ast.Alias); ok { - x := &ast.LetClause{ - Ident: a.Ident, - Equal: a.Equal, - Expr: a.Expr, - } - astutil.CopyMeta(x, a) - decls[i] = x - } - } - return true - }, nil) - - // Rewrite block comments to regular comments. - ast.Walk(f, func(n ast.Node) bool { - switch x := n.(type) { - case *ast.CommentGroup: - comments := []*ast.Comment{} - for _, c := range x.List { - s := c.Text - if !strings.HasPrefix(s, "/*") || !strings.HasSuffix(s, "*/") { - comments = append(comments, c) - continue - } - if x.Position > 0 { - // Moving to the end doesn't work, as it still - // may inject at a false line break position. - x.Position = 0 - x.Doc = true - } - s = strings.TrimSpace(s[2 : len(s)-2]) - for _, s := range strings.Split(s, "\n") { - for i := 0; i < 3; i++ { - if strings.HasPrefix(s, " ") || strings.HasPrefix(s, "*") { - s = s[1:] - } - } - comments = append(comments, &ast.Comment{Text: "// " + s}) - } - } - x.List = comments - return false - } - return true - }, nil) - - // Referred nodes and used identifiers. - referred := map[ast.Node]string{} - used := map[string]bool{} - replacement := map[ast.Node]string{} - - ast.Walk(f, func(n ast.Node) bool { - if i, ok := n.(*ast.Ident); ok { - str, err := ast.ParseIdent(i) - if err != nil { - return false - } - referred[i.Node] = str - used[str] = true - } - return true - }, nil) - - num := 0 - newIdent := func() string { - for num++; ; num++ { - str := fmt.Sprintf("X%d", num) - if !used[str] { - used[str] = true - return str - } - } - } - - // Rewrite TemplateLabel to ListLit. - // Note: there is a chance that the name will clash with the - // scope in which it is defined. We drop the alias if it is not - // used to mitigate this issue. - f = astutil.Apply(f, func(c astutil.Cursor) bool { - n := c.Node() - switch x := n.(type) { - case *ast.TemplateLabel: - var expr ast.Expr = ast.NewIdent("string") - if _, ok := referred[x]; ok { - expr = &ast.Alias{ - Ident: x.Ident, - Expr: ast.NewIdent("_"), - } - } - c.Replace(ast.NewList(expr)) - } - return true - }, nil).(*ast.File) - - // Rewrite quoted identifier fields that are referenced. - f = astutil.Apply(f, func(c astutil.Cursor) bool { - n := c.Node() - switch x := n.(type) { - case *ast.Field: - m, ok := referred[x.Value] - if !ok { - break - } - - if b, ok := x.Label.(*ast.Ident); ok { - str, err := ast.ParseIdent(b) - var expr ast.Expr = b - - switch { - case token.Lookup(str) != token.IDENT: - // quote keywords - expr = ast.NewString(b.Name) - - case err != nil || str != m || str == b.Name: - return true - - case ast.IsValidIdent(str): - x.Label = astutil.CopyMeta(ast.NewIdent(str), x.Label).(ast.Label) - return true - } - - ident := newIdent() - replacement[x.Value] = ident - expr = &ast.Alias{Ident: ast.NewIdent(ident), Expr: expr} - ast.SetRelPos(x.Label, token.NoRelPos) - x.Label = astutil.CopyMeta(expr, x.Label).(ast.Label) - } - } - return true - }, nil).(*ast.File) - - // Replace quoted references with their alias identifier. - astutil.Apply(f, func(c astutil.Cursor) bool { - n := c.Node() - switch x := n.(type) { - case *ast.Ident: - if r, ok := replacement[x.Node]; ok { - c.Replace(astutil.CopyMeta(ast.NewIdent(r), n)) - break - } - str, err := ast.ParseIdent(x) - if err != nil || str == x.Name { - break - } - // Either the identifier is valid, in which can be replaced simply - // as here, or it is a complicated identifier and the original - // destination must have been quoted, in which case it is handled - // above. - if ast.IsValidIdent(str) && token.Lookup(str) == token.IDENT { - c.Replace(astutil.CopyMeta(ast.NewIdent(str), n)) - } - } - return true - }, nil) - - // TODO: we are probably reintroducing slices. Disable for now. - // - // Rewrite slice expression. - // f = astutil.Apply(f, func(c astutil.Cursor) bool { - // n := c.Node() - // getVal := func(n ast.Expr) ast.Expr { - // if n == nil { - // return nil - // } - // if id, ok := n.(*ast.Ident); ok && id.Name == "_" { - // return nil - // } - // return n - // } - // switch x := n.(type) { - // case *ast.SliceExpr: - // ast.SetRelPos(x.X, token.NoRelPos) - - // lo := getVal(x.Low) - // hi := getVal(x.High) - // if lo == nil { // a[:j] - // lo = mustParseExpr("0") - // astutil.CopyMeta(lo, x.Low) - // } - // if hi == nil { // a[i:] - // hi = ast.NewCall(ast.NewIdent("len"), x.X) - // astutil.CopyMeta(lo, x.High) - // } - // if pkg := c.Import("list"); pkg != nil { - // c.Replace(ast.NewCall(ast.NewSel(pkg, "Slice"), x.X, lo, hi)) - // } - // } - // return true - // }, nil).(*ast.File) - - return f -} - -func mustParseExpr(expr string) ast.Expr { - ex, err := parser.ParseExpr("fix", expr) - if err != nil { - panic(err) - } - return ex -} diff --git a/cmd/cue/cmd/fmt.go b/cmd/cue/cmd/fmt.go index 5b927051a..def1d97d3 100644 --- a/cmd/cue/cmd/fmt.go +++ b/cmd/cue/cmd/fmt.go @@ -22,6 +22,7 @@ import ( "cuelang.org/go/cue/format" "cuelang.org/go/cue/load" "cuelang.org/go/internal/encoding" + "cuelang.org/go/tools/fix" ) func newFmtCmd(c *Command) *cobra.Command { @@ -67,7 +68,7 @@ func newFmtCmd(c *Command) *cobra.Command { f := d.File() if file.Encoding == build.CUE { - f = fix(f) + f = fix.File(f) } files = append(files, f) diff --git a/tools/fix/fix.go b/tools/fix/fix.go new file mode 100644 index 000000000..e617edff7 --- /dev/null +++ b/tools/fix/fix.go @@ -0,0 +1,258 @@ +// Copyright 2019 CUE Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package fix contains functionality for writing CUE files with legacy +// syntax to newer ones. +// +// Note: the transformations that are supported in this package will change +// over time. +package fix + +import ( + "fmt" + "strings" + + "cuelang.org/go/cue/ast" + "cuelang.org/go/cue/ast/astutil" + "cuelang.org/go/cue/token" + "cuelang.org/go/internal" +) + +// File applies fixes to f and returns it. It alters the original f. +func File(f *ast.File) *ast.File { + // Isolate bulk optional fields into a single struct. + ast.Walk(f, func(n ast.Node) bool { + var decls []ast.Decl + switch x := n.(type) { + case *ast.StructLit: + decls = x.Elts + case *ast.File: + decls = x.Decls + } + + if len(decls) <= 1 { + return true + } + + for i, d := range decls { + if internal.IsBulkField(d) { + decls[i] = internal.EmbedStruct(ast.NewStruct(d)) + } + } + + return true + }, nil) + + // Rewrite an old-style alias to a let clause. + ast.Walk(f, func(n ast.Node) bool { + var decls []ast.Decl + switch x := n.(type) { + case *ast.StructLit: + decls = x.Elts + case *ast.File: + decls = x.Decls + } + for i, d := range decls { + if a, ok := d.(*ast.Alias); ok { + x := &ast.LetClause{ + Ident: a.Ident, + Equal: a.Equal, + Expr: a.Expr, + } + astutil.CopyMeta(x, a) + decls[i] = x + } + } + return true + }, nil) + + // Rewrite block comments to regular comments. + ast.Walk(f, func(n ast.Node) bool { + switch x := n.(type) { + case *ast.CommentGroup: + comments := []*ast.Comment{} + for _, c := range x.List { + s := c.Text + if !strings.HasPrefix(s, "/*") || !strings.HasSuffix(s, "*/") { + comments = append(comments, c) + continue + } + if x.Position > 0 { + // Moving to the end doesn't work, as it still + // may inject at a false line break position. + x.Position = 0 + x.Doc = true + } + s = strings.TrimSpace(s[2 : len(s)-2]) + for _, s := range strings.Split(s, "\n") { + for i := 0; i < 3; i++ { + if strings.HasPrefix(s, " ") || strings.HasPrefix(s, "*") { + s = s[1:] + } + } + comments = append(comments, &ast.Comment{Text: "// " + s}) + } + } + x.List = comments + return false + } + return true + }, nil) + + // Referred nodes and used identifiers. + referred := map[ast.Node]string{} + used := map[string]bool{} + replacement := map[ast.Node]string{} + + ast.Walk(f, func(n ast.Node) bool { + if i, ok := n.(*ast.Ident); ok { + str, err := ast.ParseIdent(i) + if err != nil { + return false + } + referred[i.Node] = str + used[str] = true + } + return true + }, nil) + + num := 0 + newIdent := func() string { + for num++; ; num++ { + str := fmt.Sprintf("X%d", num) + if !used[str] { + used[str] = true + return str + } + } + } + + // Rewrite TemplateLabel to ListLit. + // Note: there is a chance that the name will clash with the + // scope in which it is defined. We drop the alias if it is not + // used to mitigate this issue. + f = astutil.Apply(f, func(c astutil.Cursor) bool { + n := c.Node() + switch x := n.(type) { + case *ast.TemplateLabel: + var expr ast.Expr = ast.NewIdent("string") + if _, ok := referred[x]; ok { + expr = &ast.Alias{ + Ident: x.Ident, + Expr: ast.NewIdent("_"), + } + } + c.Replace(ast.NewList(expr)) + } + return true + }, nil).(*ast.File) + + // Rewrite quoted identifier fields that are referenced. + f = astutil.Apply(f, func(c astutil.Cursor) bool { + n := c.Node() + switch x := n.(type) { + case *ast.Field: + m, ok := referred[x.Value] + if !ok { + break + } + + if b, ok := x.Label.(*ast.Ident); ok { + str, err := ast.ParseIdent(b) + var expr ast.Expr = b + + switch { + case token.Lookup(str) != token.IDENT: + // quote keywords + expr = ast.NewString(b.Name) + + case err != nil || str != m || str == b.Name: + return true + + case ast.IsValidIdent(str): + x.Label = astutil.CopyMeta(ast.NewIdent(str), x.Label).(ast.Label) + return true + } + + ident := newIdent() + replacement[x.Value] = ident + expr = &ast.Alias{Ident: ast.NewIdent(ident), Expr: expr} + ast.SetRelPos(x.Label, token.NoRelPos) + x.Label = astutil.CopyMeta(expr, x.Label).(ast.Label) + } + } + return true + }, nil).(*ast.File) + + // Replace quoted references with their alias identifier. + astutil.Apply(f, func(c astutil.Cursor) bool { + n := c.Node() + switch x := n.(type) { + case *ast.Ident: + if r, ok := replacement[x.Node]; ok { + c.Replace(astutil.CopyMeta(ast.NewIdent(r), n)) + break + } + str, err := ast.ParseIdent(x) + if err != nil || str == x.Name { + break + } + // Either the identifier is valid, in which can be replaced simply + // as here, or it is a complicated identifier and the original + // destination must have been quoted, in which case it is handled + // above. + if ast.IsValidIdent(str) && token.Lookup(str) == token.IDENT { + c.Replace(astutil.CopyMeta(ast.NewIdent(str), n)) + } + } + return true + }, nil) + + // TODO: we are probably reintroducing slices. Disable for now. + // + // Rewrite slice expression. + // f = astutil.Apply(f, func(c astutil.Cursor) bool { + // n := c.Node() + // getVal := func(n ast.Expr) ast.Expr { + // if n == nil { + // return nil + // } + // if id, ok := n.(*ast.Ident); ok && id.Name == "_" { + // return nil + // } + // return n + // } + // switch x := n.(type) { + // case *ast.SliceExpr: + // ast.SetRelPos(x.X, token.NoRelPos) + + // lo := getVal(x.Low) + // hi := getVal(x.High) + // if lo == nil { // a[:j] + // lo = mustParseExpr("0") + // astutil.CopyMeta(lo, x.Low) + // } + // if hi == nil { // a[i:] + // hi = ast.NewCall(ast.NewIdent("len"), x.X) + // astutil.CopyMeta(lo, x.High) + // } + // if pkg := c.Import("list"); pkg != nil { + // c.Replace(ast.NewCall(ast.NewSel(pkg, "Slice"), x.X, lo, hi)) + // } + // } + // return true + // }, nil).(*ast.File) + + return f +} diff --git a/cmd/cue/cmd/fix_test.go b/tools/fix/fix_test.go similarity index 98% rename from cmd/cue/cmd/fix_test.go rename to tools/fix/fix_test.go index f4d5eb949..06d2bd402 100644 --- a/cmd/cue/cmd/fix_test.go +++ b/tools/fix/fix_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package cmd +package fix import ( "testing" @@ -21,7 +21,7 @@ import ( "cuelang.org/go/cue/parser" ) -func TestFix(t *testing.T) { +func TestFile(t *testing.T) { testCases := []struct { name string in string @@ -161,7 +161,7 @@ c: { if err != nil { t.Fatal(err) } - n := fix(f) + n := File(f) b, err := format.Node(n) if err != nil { t.Fatal(err) diff --git a/cmd/cue/cmd/fixall.go b/tools/fix/fixall.go similarity index 96% rename from cmd/cue/cmd/fixall.go rename to tools/fix/fixall.go index 6209659bc..9069f20ae 100644 --- a/cmd/cue/cmd/fixall.go +++ b/tools/fix/fixall.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package cmd +package fix import ( "fmt" @@ -31,7 +31,10 @@ import ( "cuelang.org/go/internal" ) -func fixAll(a []*build.Instance) errors.Error { +// Instances modifies all files contained in the given build instances at once. +// +// It also applies fix.File. +func Instances(a []*build.Instance) errors.Error { cwd, _ := os.Getwd() // Collect all @@ -44,7 +47,7 @@ func fixAll(a []*build.Instance) errors.Error { ambiguous: map[string][]token.Pos{}, } - p.visitAll(func(f *ast.File) { fix(f) }) + p.visitAll(func(f *ast.File) { File(f) }) instances := cue.Build(a) p.updateValues(instances) diff --git a/cmd/cue/cmd/fixall_test.go b/tools/fix/fixall_test.go similarity index 85% rename from cmd/cue/cmd/fixall_test.go rename to tools/fix/fixall_test.go index 83f2d141e..2180e2ec2 100644 --- a/cmd/cue/cmd/fixall_test.go +++ b/tools/fix/fixall_test.go @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package cmd +package fix import ( + "flag" "fmt" "testing" @@ -22,16 +23,18 @@ import ( "cuelang.org/go/internal/cuetxtar" ) -func TestFixAll(t *testing.T) { +var update = flag.Bool("update", false, "update the test files") + +func TestInstances(t *testing.T) { test := cuetxtar.TxTarTest{ - Root: "./testdata/fix", + Root: "./testdata", Name: "fixmod", Update: *update, } test.Run(t, func(t *cuetxtar.Test) { a := t.ValidInstances("./...") - err := fixAll(a) + err := Instances(a) t.WriteErrors(err) for _, b := range a { for _, f := range b.Files { diff --git a/cmd/cue/cmd/testdata/fix/alias.txtar b/tools/fix/testdata/alias.txtar similarity index 100% rename from cmd/cue/cmd/testdata/fix/alias.txtar rename to tools/fix/testdata/alias.txtar diff --git a/cmd/cue/cmd/testdata/fix/basic.txtar b/tools/fix/testdata/basic.txtar similarity index 100% rename from cmd/cue/cmd/testdata/fix/basic.txtar rename to tools/fix/testdata/basic.txtar diff --git a/cmd/cue/cmd/testdata/fix/incompatible.txtar b/tools/fix/testdata/incompatible.txtar similarity index 100% rename from cmd/cue/cmd/testdata/fix/incompatible.txtar rename to tools/fix/testdata/incompatible.txtar diff --git a/cmd/cue/cmd/testdata/fix/invalid.txtar b/tools/fix/testdata/invalid.txtar similarity index 100% rename from cmd/cue/cmd/testdata/fix/invalid.txtar rename to tools/fix/testdata/invalid.txtar diff --git a/cmd/cue/cmd/testdata/fix/template.txtar b/tools/fix/testdata/template.txtar similarity index 100% rename from cmd/cue/cmd/testdata/fix/template.txtar rename to tools/fix/testdata/template.txtar diff --git a/cmd/cue/cmd/testdata/fix/unsupported.txtar b/tools/fix/testdata/unsupported.txtar similarity index 87% rename from cmd/cue/cmd/testdata/fix/unsupported.txtar rename to tools/fix/testdata/unsupported.txtar index 4fd5f0a0a..560e65254 100644 --- a/cmd/cue/cmd/testdata/fix/unsupported.txtar +++ b/tools/fix/testdata/unsupported.txtar @@ -21,8 +21,8 @@ b: "\(Def.b)": int package foo // Possible references to this location: -// testdata/fix/foo/foo.cue:8:4 -// testdata/fix/foo/foo.cue:9:7 +// testdata/foo/foo.cue:8:4 +// testdata/foo/foo.cue:9:7 #Def: { a: int b: "foo" diff --git a/cmd/cue/cmd/testdata/fix/unused.txtar b/tools/fix/testdata/unused.txtar similarity index 100% rename from cmd/cue/cmd/testdata/fix/unused.txtar rename to tools/fix/testdata/unused.txtar