cmd/cue/cmd: fix get go package handling

A referenced alias in a different file in the same package
for a package that was not used within the file in which
the alias was referenced itself, resulted in improper
handling.

Implementation now uses astutil.Sanitize, which allow for
more local import handling, resulting in more robust
generation. This also allows eliminating some package
tracking and the `usedInFile` field.

We could rely more on Sanitize, but that is left up to a
next round.

Fixes #464

Change-Id: Iaaf98f321a7947a6f24ced85edd5218139cbe785
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/6961
Reviewed-by: Marcel van Lohuizen <[email protected]>
Reviewed-by: CUE cueckoo <[email protected]>

Author: mpvl

cmd/cue/cmd/get_go.go

@@ -27,7 +27,6 @@ import (
 	"path/filepath"
 	"reflect"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
 	"unicode"
@@ -36,6 +35,7 @@ import (
 	"golang.org/x/tools/go/packages"
 
 	cueast "cuelang.org/go/cue/ast"
+	"cuelang.org/go/cue/ast/astutil"
 	"cuelang.org/go/cue/format"
 	"cuelang.org/go/cue/load"
 	"cuelang.org/go/cue/parser"
@@ -259,11 +259,10 @@ type extractor struct {
 	usedPkgs map[string]bool
 
 	// per file
-	cmap       ast.CommentMap
-	pkg        *packages.Package
-	consts     map[string][]string
-	pkgNames   map[string]pkgInfo
-	usedInFile map[string]bool
+	cmap     ast.CommentMap
+	pkg      *packages.Package
+	consts   map[string][]string
+	pkgNames map[string]pkgInfo
 
 	exclusions []*regexp.Regexp
 	exclude    string
@@ -282,7 +281,6 @@ func (e *extractor) logf(format string, args ...interface{}) {
 
 func (e *extractor) usedPkg(pkg string) {
 	e.usedPkgs[pkg] = true
-	e.usedInFile[pkg] = true
 }
 
 func initInterfaces() error {
@@ -427,7 +425,6 @@ func (e *extractor) extractPkg(root string, p *packages.Package) error {
 		e.cmap = ast.NewCommentMap(p.Fset, f, f.Comments)
 
 		e.pkgNames = map[string]pkgInfo{}
-		e.usedInFile = map[string]bool{}
 
 		for _, spec := range f.Imports {
 			pkgPath, _ := strconv.Unquote(spec.Path.Value)
@@ -457,12 +454,6 @@ func (e *extractor) extractPkg(root string, p *packages.Package) error {
 			continue
 		}
 
-		pkgs := []string{}
-		for k := range e.usedInFile {
-			pkgs = append(pkgs, k)
-		}
-		sort.Strings(pkgs)
-
 		pName := flagPackage.String(e.cmd)
 		if pName == "" {
 			pName = p.Name
@@ -478,22 +469,12 @@ func (e *extractor) extractPkg(root string, p *packages.Package) error {
 			}},
 			pkg,
 		}}
+		f.Decls = append(f.Decls, decls...)
 
-		if len(pkgs) > 0 {
-			imports := &cueast.ImportDecl{}
-			f.Decls = append(f.Decls, imports)
-			for _, s := range pkgs {
-				info := e.pkgNames[s]
-				spec := cueast.NewImport(nil, info.id)
-				if p.Imports[s].Name != info.name {
-					spec.Name = e.ident(info.name, false)
-				}
-				imports.Specs = append(imports.Specs, spec)
-			}
+		if err := astutil.Sanitize(f); err != nil {
+			return err
 		}
 
-		f.Decls = append(f.Decls, decls...)
-
 		file := filepath.Base(p.CompiledGoFiles[i])
 
 		file = strings.Replace(file, ".go", "_go", 1)
@@ -936,9 +917,12 @@ func (e *extractor) makeType(expr types.Type) (result cueast.Expr) {
 		// builtin type.
 		switch obj.Type().String() {
 		case "time.Time":
-			e.usedInFile["time"] = true
 			ref := e.ident(e.pkgNames[obj.Pkg().Path()].name, false)
-			ref.Node = cueast.NewImport(nil, "time")
+			var name *cueast.Ident
+			if ref.Name != "time" {
+				name = e.ident(ref.Name, false)
+			}
+			ref.Node = cueast.NewImport(name, "time")
 			return cueast.NewSel(ref, obj.Name())
 
 		case "math/big.Int":
@@ -956,16 +940,26 @@ func (e *extractor) makeType(expr types.Type) (result cueast.Expr) {
 		}
 
 		result = e.ident(obj.Name(), true)
-		if pkg := obj.Pkg(); pkg != nil {
-			if info := e.pkgNames[pkg.Path()]; info.name != "" {
-				p := e.ident(info.name, false)
-				// TODO: set package name et. at.
-				p.Node = cueast.NewImport(nil, pkg.Path())
-				// makeType is always called to describe a type, so whatever
-				// this is referring to, it must be a definition.
-				result = cueast.NewSel(p, "#"+obj.Name())
-				e.usedPkg(pkg.Path())
+		if pkg := obj.Pkg(); pkg != nil && pkg != e.pkg.Types {
+			info := e.pkgNames[pkg.Path()]
+			if info.name == "" {
+				info.name = pkg.Name()
+			}
+			p := e.ident(info.name, false)
+			var name *cueast.Ident
+			if info.name != pkg.Name() {
+				name = e.ident(info.name, false)
+			}
+			if info.id == "" {
+				// This may happen if an alias is defined in a different file
+				// within this package referring to yet another package.
+				info.id = pkg.Path()
 			}
+			p.Node = cueast.NewImport(name, info.id)
+			// makeType is always called to describe a type, so whatever
+			// this is referring to, it must be a definition.
+			result = cueast.NewSel(p, "#"+obj.Name())
+			e.usedPkg(pkg.Path())
 		}
 		return
 	}

cmd/cue/cmd/testdata/code/go/pkg1/alias.go

@@ -0,0 +1,19 @@
+// Copyright 2020 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 pkg1
+
+import p3 "cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg3"
+
+type MyBarzer = p3.Barzer

cmd/cue/cmd/testdata/code/go/pkg1/file1.go

@@ -47,6 +47,8 @@ type Foozer struct {
 
 	Barzer p2.Barzer
 
+	Alias1 *MyBarzer
+
 	Map    map[string]*CustomJSON
 	Slice1 []int
 	Slice2 []interface{}

cmd/cue/cmd/testdata/code/go/pkg3/pkg3.go

@@ -0,0 +1,19 @@
+// Copyright 2020 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 pkg3
+
+import pkgtwo "cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg2"
+
+type Barzer = pkgtwo.Barzer

cmd/cue/cmd/testdata/pkg/cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg1/alias_go_gen.cue

@@ -0,0 +1,9 @@
+// Code generated by cue get go. DO NOT EDIT.
+
+//cue:generate cue get go cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg1
+
+package pkg1
+
+import "cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg2"
+
+#MyBarzer: pkgtwo.#Barzer

cmd/cue/cmd/testdata/pkg/cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg1/file1_go_gen.cue

@@ -6,8 +6,8 @@
 package pkg1
 
 import (
-	p2 "cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg2:pkgtwo"
 	"time"
+	p2 "cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg2:pkgtwo"
 )
 
 // Foozer foozes a jaman.
@@ -24,8 +24,9 @@ import (
 	bar?:        int & >10          @go(Bar)
 
 	// Time is mapped to CUE's internal type.
-	Time:   time.Time
-	Barzer: p2.#Barzer
+	Time:    time.Time
+	Barzer:  p2.#Barzer
+	Alias1?: null | p2.#Barzer @go(,*p2.Barzer)
 	Map: {[string]: null | #CustomJSON} @go(,map[string]*CustomJSON)
 	Slice1: [...int] @go(,[]int)
 	Slice2: [...] @go(,[]interface{})

cmd/cue/cmd/testdata/pkg/cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg3/pkg3_go_gen.cue

@@ -0,0 +1,9 @@
+// Code generated by cue get go. DO NOT EDIT.
+
+//cue:generate cue get go cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg3
+
+package pkg3
+
+import "cuelang.org/go/cmd/cue/cmd/testdata/code/go/pkg2:pkgtwo"
+
+#Barzer: pkgtwo.#Barzer

cue/testdata/eval/errunifiy.txtar

@@ -0,0 +1,31 @@
+Incomplete errors should not unify with values.
+
+-- in.cue --
+a: or([])
+a: "t"
+
+b: _|_
+b: "t"
+
+-- out/eval --
+Errors:
+from source:
+    ./in.cue:4:4
+
+Result:
+(_|_){
+  // [user]
+  a: (string){ "t" }
+  b: (_|_){
+    // [user] from source:
+    //     ./in.cue:4:4
+  }
+}
+-- out/compile --
+--- in.cue
+{
+  a: or([])
+  a: "t"
+  b: _|_(from source)
+  b: "t"
+}