tools/fix: remove support to rewrite old definitions
Change-Id: I4aeaa7a387fc6728deed50195c6d4c326c596824
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/6656
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/testdata/script/fix.txt b/cmd/cue/cmd/testdata/script/fix.txt
index 6f705d6..537738b 100644
--- a/cmd/cue/cmd/testdata/script/fix.txt
+++ b/cmd/cue/cmd/testdata/script/fix.txt
@@ -1,3 +1,5 @@
+skip 'No longer supported, but keeping around until for sure.'
+
cue fix
cmp foo/foo.cue expect/foo/foo_cue
diff --git a/cmd/cue/cmd/testdata/script/fix_files.txt b/cmd/cue/cmd/testdata/script/fix_files.txt
index 53068ef..0297103 100644
--- a/cmd/cue/cmd/testdata/script/fix_files.txt
+++ b/cmd/cue/cmd/testdata/script/fix_files.txt
@@ -1,3 +1,5 @@
+skip 'No longer supported, but keeping around until for sure.'
+
cue fix ./foo/foo.cue ./bar/bar.cue
cmp foo/foo.cue expect/foo/foo_cue
diff --git a/cmd/cue/cmd/testdata/script/fix_pkg.txt b/cmd/cue/cmd/testdata/script/fix_pkg.txt
index bd7796a..0919cfe 100644
--- a/cmd/cue/cmd/testdata/script/fix_pkg.txt
+++ b/cmd/cue/cmd/testdata/script/fix_pkg.txt
@@ -1,3 +1,5 @@
+skip 'No longer supported, but keeping around until for sure.'
+
cue fix ./bar foobar.org/notimported
cmp foo/foo.cue expect/foo/foo_cue
diff --git a/tools/fix/fixall.go b/tools/fix/fixall.go
index 9069f20..c7a94b0 100644
--- a/tools/fix/fixall.go
+++ b/tools/fix/fixall.go
@@ -15,20 +15,11 @@
package fix
import (
- "fmt"
- "hash/fnv"
"os"
- "path/filepath"
- "strings"
- "cuelang.org/go/cue"
"cuelang.org/go/cue/ast"
- "cuelang.org/go/cue/ast/astutil"
"cuelang.org/go/cue/build"
"cuelang.org/go/cue/errors"
- "cuelang.org/go/cue/format"
- "cuelang.org/go/cue/token"
- "cuelang.org/go/internal"
)
// Instances modifies all files contained in the given build instances at once.
@@ -41,20 +32,10 @@
p := processor{
instances: a,
cwd: cwd,
-
- done: map[ast.Node]bool{},
- rename: map[*ast.Ident]string{},
- ambiguous: map[string][]token.Pos{},
}
p.visitAll(func(f *ast.File) { File(f) })
- instances := cue.Build(a)
- p.updateValues(instances)
- p.visitAll(p.tagAmbiguous)
- p.rewriteIdents()
- p.visitAll(p.renameFields)
-
return p.err
}
@@ -62,238 +43,9 @@
instances []*build.Instance
cwd string
- done map[ast.Node]bool
- // Evidence for rewrites. Rewrite in a later pass.
- rename map[*ast.Ident]string
- ambiguous map[string][]token.Pos
-
- stack []cue.Value
-
err errors.Error
}
-func (p *processor) updateValues(instances []*cue.Instance) {
- for _, inst := range instances {
- inst.Value().Walk(p.visit, nil)
- }
-}
-
-func (p *processor) visit(v cue.Value) bool {
- if e, ok := v.Elem(); ok {
- p.updateValue(e)
- p.visit(e)
- }
-
- if v.Kind() != cue.StructKind {
- p.updateValue(v)
- return true
- }
-
- p.stack = append(p.stack, v)
- defer func() { p.stack = p.stack[:len(p.stack)-1] }()
-
- for it, _ := v.Fields(cue.All()); it.Next(); {
- p.updateValue(it.Value())
- p.visit(it.Value())
- }
-
- for _, kv := range v.BulkOptionals() {
- p.updateValue(kv[0])
- p.visit(kv[0])
- p.updateValue(kv[1])
- p.visit(kv[1])
- }
-
- return false
-}
-
-func (p *processor) updateValue(v cue.Value) cue.Value {
- switch op, a := v.Expr(); op {
- case cue.NoOp:
- return v
-
- case cue.SelectorOp:
- v := p.updateValue(a[0])
-
- switch x := a[1].Source().(type) {
- case *ast.SelectorExpr:
- return p.lookup(v, x.Sel)
-
- case *ast.Ident:
- v := p.updateValue(a[0])
- return p.lookup(v, x)
- }
-
- default:
- for _, v := range a {
- p.updateValue(v)
- }
- }
- return v
-}
-
-func (p *processor) lookup(v cue.Value, l ast.Expr) cue.Value {
- label, ok := l.(ast.Label)
- if !ok {
- return cue.Value{}
- }
-
- name, isIdent, err := ast.LabelName(label)
- if err != nil {
- return cue.Value{}
- }
- f, err := v.FieldByName(name, isIdent)
- if err != nil {
- f := v.Template()
- if f == nil {
- return cue.Value{}
- }
- return v.Template()(name)
- }
-
- switch {
- case !p.done[l]:
- p.done[l] = true
-
- if !f.IsDefinition {
- break
- }
-
- if !ast.IsValidIdent(name) {
- p.err = errors.Append(p.err, errors.Newf(
- l.Pos(),
- "cannot convert reference to definition with invalid identifier %q",
- name))
- break
- }
-
- if ident, ok := l.(*ast.Ident); ok && !internal.IsDef(name) {
- p.rename[ident] = "#" + name
- }
- }
-
- return f.Value
-}
-
-// tagAmbiguous marks identifier fields were not handled by the previous pass.
-// These can be identifiers within unused templates, for instance. It is
-// possible to do further resolution within templates, but for now we will
-// punt on this.
-func (p *processor) tagAmbiguous(f *ast.File) {
- ast.Walk(f, p.tagRef, nil)
-}
-
-func (p *processor) tagRef(n ast.Node) bool {
- switch x := n.(type) {
- case *ast.Field:
- ast.Walk(x.Value, p.tagRef, nil)
-
- lab := x.Label
- if a, ok := x.Label.(*ast.Alias); ok {
- lab, _ = a.Expr.(ast.Label)
- }
-
- switch lab.(type) {
- case *ast.Ident, *ast.BasicLit:
- default: // list, paren, or interpolation
- ast.Walk(lab, p.tagRef, nil)
- }
-
- return false
-
- case *ast.Ident:
- if _, ok := p.done[x]; !ok {
- p.ambiguous[x.Name] = append(p.ambiguous[x.Name], x.Pos())
- }
- return false
- }
- return true
-}
-
-func (p *processor) rewriteIdents() {
- for x, name := range p.rename {
- x.Name = name
- }
-}
-
-func (p *processor) renameFields(f *ast.File) {
- hasErr := false
- _ = astutil.Apply(f, func(c astutil.Cursor) bool {
- switch x := c.Node().(type) {
- case *ast.Field:
- if x.Token != token.ISA {
- return true
- }
-
- label, isIdent, err := ast.LabelName(x.Label)
- if err != nil {
- b, _ := format.Node(x.Label)
- hasErr = true
- p.err = errors.Append(p.err, errors.Newf(x.Pos(),
- `cannot convert dynamic definition for '%s'`, string(b)))
- return false
- }
-
- if !isIdent && !ast.IsValidIdent(label) {
- hasErr = true
- p.err = errors.Append(p.err, errors.Newf(x.Pos(),
- `invalid identifier %q; definition must be valid label`, label))
- return false
- }
-
- if refs, ok := p.ambiguous[label]; ok {
- h := fnv.New32()
- _, _ = h.Write([]byte(label))
- opt := fmt.Sprintf("@tmpNoExportNewDef(%x)", h.Sum32()&0xffff)
-
- f := &ast.Field{
- Label: ast.NewIdent(label),
- Value: ast.NewIdent("#" + label),
- Attrs: []*ast.Attribute{{Text: opt}},
- }
- c.InsertAfter(f)
-
- b := &strings.Builder{}
- fmt.Fprintln(b, "Possible references to this location:")
- for _, r := range refs {
- s, err := filepath.Rel(p.cwd, r.String())
- if err != nil {
- s = r.String()
- }
- s = filepath.ToSlash(s)
- fmt.Fprintf(b, "\t%s\n", s)
- }
-
- cg := internal.NewComment(true, b.String())
- astutil.CopyPosition(cg, c.Node())
- ast.AddComment(c.Node(), cg)
- }
-
- x.Label = ast.NewIdent("#" + label)
- x.Token = token.COLON
- }
-
- return true
- }, nil)
-
- if hasErr {
- p.err = errors.Append(p.err, errors.Newf(token.NoPos, `Incompatible definitions detected:
-
-A trick that can be used is to rename this to a regular identifier and then
-move the definition to a sub field. For instance, rewrite
-
- "foo-bar" :: baz
- "foo\(bar)" :: baz
-
- to
-
- #defmap: "foo-bar": baz
- #defmap: "foo\(bar)": baz
-
-Errors:`))
- }
-}
-
func (p *processor) visitAll(fn func(f *ast.File)) {
if p.err != nil {
return
diff --git a/tools/fix/fixall_test.go b/tools/fix/fixall_test.go
index 2180e2e..cd19f90 100644
--- a/tools/fix/fixall_test.go
+++ b/tools/fix/fixall_test.go
@@ -26,6 +26,8 @@
var update = flag.Bool("update", false, "update the test files")
func TestInstances(t *testing.T) {
+ t.Skip()
+
test := cuetxtar.TxTarTest{
Root: "./testdata",
Name: "fixmod",
diff --git a/tools/fix/testdata/alias.txtar b/tools/fix/testdata/alias.txtar
deleted file mode 100644
index 2e5a133..0000000
--- a/tools/fix/testdata/alias.txtar
+++ /dev/null
@@ -1,21 +0,0 @@
-
--- cue.mod/module.cue --
-module: "acme.com"
-
--- bar/bar.cue --
-package bar
-
-a: [X=b.c]: int
-a: Y=[X=b.c]: int
-
-b: c :: int
-
--- out/fixmod --
---- bar/bar.cue
-package bar
-
-a: [X=b.#c]: int
-a: Y=[X=b.#c]: int
-
-b: #c: int
-
diff --git a/tools/fix/testdata/basic.txtar b/tools/fix/testdata/basic.txtar
deleted file mode 100644
index 49c6aef..0000000
--- a/tools/fix/testdata/basic.txtar
+++ /dev/null
@@ -1,61 +0,0 @@
--- cue.mod/module.cue --
-module: "acme.com"
-
--- bar/bar.cue --
-package bar
-
-import "acme.com/foo"
-
-A :: foo.Def & {
- a: >10
-
- B :: { c: int }
-}
-
-b: A & { a: 11 }
-
-c: foo.Def & { a: 11 }
-
-d: A.B.c
-e: [...A.B.c]
-
--- foo/foo.cue --
-package foo
-
-Def :: {
- a: int
- b: string
-}
-
-a: Def
-c: [Def.b]: int
-
--- out/fixmod --
---- bar/bar.cue
-package bar
-
-import "acme.com/foo"
-
-#A: foo.#Def & {
- a: >10
- #B: {c: int}
-}
-
-b: #A & {a: 11}
-
-c: foo.#Def & {a: 11}
-
-d: #A.#B.c
-e: [...#A.#B.c]
-
---- foo/foo.cue
-package foo
-
-#Def: {
- a: int
- b: string
-}
-
-a: #Def
-c: [#Def.b]: int
-
diff --git a/tools/fix/testdata/incompatible.txtar b/tools/fix/testdata/incompatible.txtar
deleted file mode 100644
index e773453..0000000
--- a/tools/fix/testdata/incompatible.txtar
+++ /dev/null
@@ -1,38 +0,0 @@
-Things currently possible not supported by new-style
-definitions.
-
--- foo/bar.cue --
-package bar
-
-"\(foo)" :: int
-foo: string
-
-"foo-bar" :: int
-
--- out/fixmod --
-Incompatible definitions detected:
-
-A trick that can be used is to rename this to a regular identifier and then
-move the definition to a sub field. For instance, rewrite
-
- "foo-bar" :: baz
- "foo\(bar)" :: baz
-
- to
-
- #defmap: "foo-bar": baz
- #defmap: "foo\(bar)": baz
-
-Errors:
-cannot convert dynamic definition for '"\(foo)"':
- ./foo/bar.cue:3:1
-invalid identifier "foo-bar"; definition must be valid label:
- ./foo/bar.cue:6:1
---- foo/bar.cue
-package bar
-
-"\(foo)" :: int
-foo: string
-
-"foo-bar" :: int
-
diff --git a/tools/fix/testdata/invalid.txtar b/tools/fix/testdata/invalid.txtar
deleted file mode 100644
index a5beffb..0000000
--- a/tools/fix/testdata/invalid.txtar
+++ /dev/null
@@ -1 +0,0 @@
-#skip
\ No newline at end of file
diff --git a/tools/fix/testdata/template.txtar b/tools/fix/testdata/template.txtar
deleted file mode 100644
index 3b987da..0000000
--- a/tools/fix/testdata/template.txtar
+++ /dev/null
@@ -1,74 +0,0 @@
--- cue.mod/module.cue --
-module: "acme.com"
-
--- bar/bar.cue --
-package bar
-
-A :: {
- foo: [string]: {
- B :: {
- foo: int
- }
-
- bar: {
- C :: { D :: int }
- }
-
- c: B.foo
- d: B
- e: B & { foo: 3 }
- f: bar.C
- g: bar.C.D
- }
-
- used: [=~"Foo"]: {
- C :: {}
- c: C
- }
- used: "FooBar": {}
-
- unused: [=~"Foo"]: {
- X :: {
-
- }
- x: X
- }
-}
-
--- out/fixmod --
---- bar/bar.cue
-package bar
-
-#A: {
- foo: [string]: {
- #B: {
- foo: int
- }
-
- bar: {
- #C: {
- #D: int
- }
- }
-
- c: #B.foo
- d: #B
- e: #B & {foo: 3}
- f: bar.#C
- g: bar.#C.#D
- }
-
- used: [=~"Foo"]: {
- #C: {}
- c: #C
- }
- used: "FooBar": {}
-
- unused: [=~"Foo"]: {
- #X: {
-
- }
- x: #X
- }
-}
-
diff --git a/tools/fix/testdata/unsupported.txtar b/tools/fix/testdata/unsupported.txtar
deleted file mode 100644
index 560e652..0000000
--- a/tools/fix/testdata/unsupported.txtar
+++ /dev/null
@@ -1,34 +0,0 @@
-This file contains cases that could be supported but aren't,
-as the cost for supporting them is likely too high.
-
--- cue.mod/module.cue --
-module: "acme.com"
-
-
--- foo/foo.cue --
-package foo
-
-Def :: {
- a: int
- b: "foo"
-}
-
-"\(Def.b)": int
-b: "\(Def.b)": int
-
--- out/fixmod --
---- foo/foo.cue
-package foo
-
-// Possible references to this location:
-// testdata/foo/foo.cue:8:4
-// testdata/foo/foo.cue:9:7
-#Def: {
- a: int
- b: "foo"
-}
-Def: #Def @tmpNoExportNewDef(7aca)
-
-"\(Def.b)": int
-b: "\(Def.b)": int
-
diff --git a/tools/fix/testdata/unused.txtar b/tools/fix/testdata/unused.txtar
deleted file mode 100644
index 2eaefec..0000000
--- a/tools/fix/testdata/unused.txtar
+++ /dev/null
@@ -1,22 +0,0 @@
--- cue.mod/module.cue --
-module: "acme.com"
-
--- bar/bar.cue --
-package bar
-
-a: [string]: {
- D :: int
-}
-
-b: a.str.D
-
--- out/fixmod --
---- bar/bar.cue
-package bar
-
-a: [string]: {
- #D: int
-}
-
-b: a.str.#D
-
