tools/trim: undo faulty optimization
The removal of this optimization also means
trim will sometimes not remove fields it logically
could. But that is better than being faulty.
Closes #302
Closes #303
Change-Id: I7e9d87c42242482a04da2fd438826bff5506219d
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/5247
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/testdata/script/issue302.txt b/cmd/cue/cmd/testdata/script/issue302.txt
new file mode 100644
index 0000000..835b558
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/issue302.txt
@@ -0,0 +1,16 @@
+cue trim foo.cue
+cmp foo.cue rewritten
+
+-- foo.cue --
+package p
+
+foo?: Foo
+
+Foo :: "hello"
+
+-- rewritten --
+package p
+
+foo?: Foo
+
+Foo :: "hello"
\ No newline at end of file
diff --git a/cmd/cue/cmd/testdata/script/issue303.txt b/cmd/cue/cmd/testdata/script/issue303.txt
new file mode 100644
index 0000000..ca7d9b8
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/issue303.txt
@@ -0,0 +1,16 @@
+cue trim foo.cue
+cmp foo.cue rewritten
+
+-- foo.cue --
+package example
+
+foo: c: true
+foo: M
+M :: c?: bool
+
+-- rewritten --
+package example
+
+foo: c: true
+foo: M
+M :: c?: bool
diff --git a/cmd/cue/cmd/trim.go b/cmd/cue/cmd/trim.go
index a180b09..6f6c9bf 100644
--- a/cmd/cue/cmd/trim.go
+++ b/cmd/cue/cmd/trim.go
@@ -24,7 +24,6 @@
"cuelang.org/go/cue/format"
"cuelang.org/go/cue/load"
- "cuelang.org/go/internal"
"cuelang.org/go/internal/diff"
"cuelang.org/go/tools/trim"
)
@@ -94,16 +93,6 @@
}
func runTrim(cmd *Command, args []string) error {
- // TODO: Do something more fine-grained. Optional fields are mostly not
- // useful to consider as an optional field will almost never subsume
- // another value. However, an optional field may subsume and therefore
- // trigger the removal of another optional field.
- // For now this is the better approach: trimming is not 100% accurate,
- // and optional fields are just more likely to cause edge cases that may
- // block a removal.
- internal.DropOptional = true
- defer func() { internal.DropOptional = false }()
-
binst := loadFromArgs(cmd, args, defaultConfig)
if binst == nil {
return nil
diff --git a/cue/ast.go b/cue/ast.go
index b60e421..c7432c3 100644
--- a/cue/ast.go
+++ b/cue/ast.go
@@ -436,9 +436,6 @@
v.object.addTemplate(v.ctx(), token.NoPos, nil, template)
case *ast.BasicLit, *ast.Ident:
- if internal.DropOptional && opt {
- break
- }
v.sel, _, _ = ast.LabelName(x)
if v.sel == "_" {
if _, ok := x.(*ast.BasicLit); ok {
diff --git a/internal/internal.go b/internal/internal.go
index 2e73e88..ae44dd5 100644
--- a/internal/internal.go
+++ b/internal/internal.go
@@ -65,10 +65,6 @@
// The returned value is a cue.Value, which the caller must cast to.
var FromGoType func(instance, x interface{}) interface{}
-// DropOptional is a blanket override of handling optional values during
-// compilation. TODO: should we make this a build option?
-var DropOptional bool
-
// UnifyBuiltin returns the given Value unified with the given builtin template.
var UnifyBuiltin func(v interface{}, kind string) interface{}
diff --git a/tools/trim/trim.go b/tools/trim/trim.go
index 7c632c8..45110f4 100644
--- a/tools/trim/trim.go
+++ b/tools/trim/trim.go
@@ -84,9 +84,6 @@
// Trimming is done on a best-effort basis and only when the removed field
// is clearly implied by another field, rather than equal sibling fields.
func Files(files []*ast.File, inst *cue.Instance, cfg *Config) error {
- internal.DropOptional = true
- defer func() { internal.DropOptional = false }()
-
gen := newTrimSet(cfg)
gen.files = files
for _, f := range files {
diff --git a/tools/trim/trim_test.go b/tools/trim/trim_test.go
index 95c0289..7b4587e 100644
--- a/tools/trim/trim_test.go
+++ b/tools/trim/trim_test.go
@@ -102,6 +102,17 @@
out: `[_]: x: "hello"
a: {}
`,
+ }, {
+ name: "issue303",
+ in: `
+ foo: c: true
+ foo: M
+ M :: c?: bool
+ `,
+ out: `foo: c: true
+foo: M
+M :: c?: bool
+`,
// TODO: This used to work.
// name: "remove implied interpolations",
// in: `
