Skip to content

Commit 97cbf7e

Browse files
committed
cmd/cue: actually drop unsupported Go std types in get go
The Go logic fell through if altType returned nil, meaning that we would try to generate and import Go std packages as if they were CUE std packages, which does not work at all. While here, move the extractor fields to clarify that pkg and consts are per-package and not per-file, and declare a pkg variable which simplifies the named type logic a bit and lets us see that a nil check further down was redundant. Some Go standard library packages may declare a structure which can be imported as CUE correctly, and our thinking is that they could be generated under a non-CUE-std namespace like cue.mod/gen/pkg.go.dev/reflect rather than cue.mod/gen/reflect. That is left for a future change in the form of TODOs in both the testscript, which includes two such examples, and the code. Fixes #648. Signed-off-by: Daniel Martí <[email protected]> Change-Id: Ic2bd39a2024c76449978c44ae70dc75b65297314 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1210516 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Roger Peppe <[email protected]>
1 parent 9a53c81 commit 97cbf7e

File tree

2 files changed

+35
-36
lines changed

2 files changed

+35
-36
lines changed

cmd/cue/cmd/get_go.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,13 @@ type extractor struct {
249249
done map[string]bool
250250

251251
// per package
252+
pkg *packages.Package
252253
orig map[types.Type]*ast.StructType
253254
usedPkgs map[string]bool
255+
consts map[string][]string
254256

255257
// per file
256258
cmap ast.CommentMap
257-
pkg *packages.Package
258-
consts map[string][]string
259259
pkgNames map[string]pkgInfo
260260

261261
exclusions []*regexp.Regexp
@@ -1040,21 +1040,22 @@ func (e *extractor) makeType(typ types.Type) (result cueast.Expr) {
10401040
switch typ := types.Unalias(typ).(type) {
10411041
case *types.Named:
10421042
obj := typ.Obj()
1043-
if obj.Pkg() == nil {
1043+
pkg := obj.Pkg()
1044+
if pkg == nil {
10441045
return e.ident("_", false)
10451046
}
10461047
// Check for builtin packages.
10471048
switch {
1048-
case obj.Pkg().Path() == "time" && obj.Name() == "Time":
1049-
ref := e.ident(e.pkgNames[obj.Pkg().Path()].name, false)
1049+
case pkg.Path() == "time" && obj.Name() == "Time":
1050+
ref := e.ident(e.pkgNames[pkg.Path()].name, false)
10501051
var name *cueast.Ident
10511052
if ref.Name != "time" {
10521053
name = e.ident(ref.Name, false)
10531054
}
10541055
ref.Node = cueast.NewImport(name, "time")
10551056
return cueast.NewSel(ref, obj.Name())
10561057

1057-
case obj.Pkg().Path() == "time" && obj.Name() == "Duration":
1058+
case pkg.Path() == "time" && obj.Name() == "Duration":
10581059
// Go's time.Duration is an int64 to represent nanoseconds,
10591060
// and even though most Go users would find the string representation
10601061
// like "3s" or "40m" most reasonable and readable for constant values,
@@ -1072,22 +1073,27 @@ func (e *extractor) makeType(typ types.Type) (result cueast.Expr) {
10721073
// and constants like '300 | *"300ns"' to support both at the same time?
10731074
return e.ident("int", false)
10741075

1075-
case obj.Pkg().Path() == "math/big" && obj.Name() == "Int":
1076+
case pkg.Path() == "math/big" && obj.Name() == "Int":
10761077
return e.ident("int", false)
10771078

10781079
default:
1079-
if !strings.ContainsAny(obj.Pkg().Path(), ".") {
1080-
// Drop any standard library type if they haven't been handled
1081-
// above.
1082-
// TODO: Doc?
1080+
// Any Go standard library type that hasn't been handled above is not supported;
1081+
// fall back to whatever alternative type we can, or just "top".
1082+
// Otherwise we would end up generating "std" packages which clash with our
1083+
// own standard library, for example cue.mod/gen/time.
1084+
// TODO: for cases where the Go std type could be supported, we could still generate
1085+
// and import it under a non-std CUE package, such as cue.mod/gen/pkg.go.dev/time.
1086+
// TODO: Doc?
1087+
if !strings.ContainsAny(pkg.Path(), ".") {
10831088
if s := e.altType(obj.Type()); s != nil {
10841089
return s
10851090
}
1091+
return e.ident("_", false)
10861092
}
10871093
}
10881094

10891095
result = e.ident(obj.Name(), true)
1090-
if pkg := obj.Pkg(); pkg != nil && pkg != e.pkg.Types {
1096+
if pkg != e.pkg.Types {
10911097
info := e.pkgNames[pkg.Path()]
10921098
if info.name == "" {
10931099
info.name = pkg.Name()

cmd/cue/cmd/testdata/script/get_go_std.txtar

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,12 @@ exec cue get go --local
55
cmp src_go_gen.cue src_go_gen.cue.golden
66

77
# Check which files were generated under cue.mod/gen and other directories.
8-
# TODO: we should be generating zero "std" packages under cue.mod/gen.
8+
# We cannot generate any std packages under cue.mod/gen either.
99
find-files cue.mod
10-
stdout '^cue.mod/gen/[^.]+/'
10+
! stdout '^cue.mod/gen/[^.]+/'
1111

1212
# Verify that the result can be loaded correctly.
13-
# TODO: this should succeed, not fail.
14-
! exec cue vet -c=false
15-
cmp stderr vet.stderr
13+
exec cue vet -c=false
1614

1715
-- go.mod --
1816
module mod.test
@@ -37,8 +35,9 @@ type Root struct {
3735
ReflectValue reflect.Value
3836

3937
// reflect.ValueError is an example of a struct made up of simple types,
40-
// in this case a Method string and a Kind uint. We could generate that structure,
41-
// as long as we don't place it in cue.mod/pkg/reflect, conflicting with CUE's own std.
38+
// in this case a Method string and a Kind uint.
39+
// TODO: generate that structure as long as we don't place it in cue.mod/pkg/reflect,
40+
// which conflicts with CUE's own std.
4241
ReflectValueError reflect.ValueError
4342

4443
// io.Writer is an interface, so there is no structure to be generated.
@@ -47,6 +46,8 @@ type Root struct {
4746

4847
// time.Month is just an int type, but this edge case is particularly interesting
4948
// given that CUE already has a "time" package with e.g. Duration and Time, but no Month.
49+
// TODO: generate that structure as long as we don't place it in cue.mod/pkg/reflect,
50+
// which conflicts with CUE's own std, or generate "int" directly.
5051
TimeMonth time.Month
5152
}
5253
-- src_go_gen.cue.golden --
@@ -56,31 +57,23 @@ type Root struct {
5657

5758
package x
5859

59-
import (
60-
"reflect"
61-
"io"
62-
"time"
63-
)
64-
6560
#Root: {
6661
// reflect types and values in Go are opaque and do not encode as useful JSON.
67-
ReflectType: reflect.#Type
62+
ReflectType: _ @go(,reflect.Type)
6863

6964
// reflect.ValueError is an example of a struct made up of simple types,
70-
// in this case a Method string and a Kind uint. We could generate that structure,
71-
// as long as we don't place it in cue.mod/pkg/reflect, conflicting with CUE's own std.
72-
ReflectValueError: reflect.#ValueError
65+
// in this case a Method string and a Kind uint.
66+
// TODO: generate that structure as long as we don't place it in cue.mod/pkg/reflect,
67+
// which conflicts with CUE's own std.
68+
ReflectValueError: _ @go(,reflect.ValueError)
7369

7470
// io.Writer is an interface, so there is no structure to be generated.
7571
// Moreover, even though "io" is a very common Go package, CUE has no such package.
76-
IoWriter: io.#Writer
72+
IoWriter: _ @go(,io.Writer)
7773

7874
// time.Month is just an int type, but this edge case is particularly interesting
7975
// given that CUE already has a "time" package with e.g. Duration and Time, but no Month.
80-
TimeMonth: time.#Month
76+
// TODO: generate that structure as long as we don't place it in cue.mod/pkg/reflect,
77+
// which conflicts with CUE's own std, or generate "int" directly.
78+
TimeMonth: _ @go(,time.Month)
8179
}
82-
-- vet.stderr --
83-
builtin package "reflect" undefined:
84-
./src_go_gen.cue:8:2
85-
builtin package "io" undefined:
86-
./src_go_gen.cue:9:2

0 commit comments

Comments
 (0)