tools/fix: move fix functionality to separate package
This will allow users to apply fixes programmatically.
Also handy for tests.
Change-Id: Iec662c773d0957abf3aa79e7473a3408bb971c80
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/6202
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/fix.go b/cmd/cue/cmd/fix.go
index be53700..eccab53 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 @@
}
}
- errs := fixAll(instances)
+ errs := fix.Instances(instances)
if errs != nil && flagForce.Bool(cmd) {
return errs
@@ -129,238 +126,3 @@
})
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 5b92705..def1d97 100644
--- a/cmd/cue/cmd/fmt.go
+++ b/cmd/cue/cmd/fmt.go
@@ -22,6 +22,7 @@
"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 @@
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 0000000..e617edf
--- /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 97%
rename from cmd/cue/cmd/fix_test.go
rename to tools/fix/fix_test.go
index f4d5eb9..06d2bd4 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 @@
"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 @@
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 6209659..9069f20 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 @@
"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 @@
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 83f2d14..2180e2e 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 @@
"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 4fd5f0a..560e652 100644
--- a/cmd/cue/cmd/testdata/fix/unsupported.txtar
+++ b/tools/fix/testdata/unsupported.txtar
@@ -21,8 +21,8 @@
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
