From 5e9aa4eb548e463a5e154f0afda45066e3aef31a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Tue, 7 May 2024 13:05:26 +0100
Subject: [PATCH] cue/load: restore the error result on an empty string
 argument
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TestEmptyImport was added a long time ago to ensure that calling
load.Instances with an input of []string{""} would result in an error.

Calling load.Instances with no arguments like `[]string{}` is fine,
as it is an alias for the common case of `[]string{"."}`,
much like `cue export` is an alias for `cue export .`.
However, `cue export ""` should fail, as it's not a valid package
pattern nor is it a valid file or directory name.

As part of the early refactors of cue/load for modules support in
https://cuelang.org/cl/548987 we rewrote this test to use internal APIs.
As such, it actually stopped testing what it was meant to test,
and cue/load silently started treating `[]string{}` as `[]string{"."}`.

The cleanest solution appears to be in cleanPatterns;
it never makes any sense to "clean" an empty pattern as the dot pattern.
cleanPatterns already replaces an empty list with a dot pattern,
which is the only case where we want to alias to the dot pattern.

TestEmptyImport is also tweaked to use a testdata directory
with a known CUE package inside a CUE module to double check that
we fail even when the dot pattern would match a valid CUE package.
Otherwise the empty Config.Dir results in using the current directory.

While here, also rewrite getInst to no longer use internal APIs.
It's really just a wrapper over load.Instances which expects
a single instance to be returned. All else it did was to simplify
passing a cwd and checking the resulting instance's error.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I0af58e59bb75b8046ec83f4b5cae57c28b138bde
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194410
Reviewed-by: Paul Jolly <paul@myitcv.io>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
---
 cue/load/import_test.go   | 26 ++++++++------------------
 cue/load/loader_common.go |  4 ++--
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/cue/load/import_test.go b/cue/load/import_test.go
index a98d695cab2..d431cf46bbc 100644
--- a/cue/load/import_test.go
+++ b/cue/load/import_test.go
@@ -22,7 +22,6 @@ import (
 	"testing"
 
 	"cuelang.org/go/cue/build"
-	"cuelang.org/go/cue/token"
 )
 
 func testdata(elems ...string) string {
@@ -34,27 +33,18 @@ func getInst(pkg, cwd string) (*build.Instance, error) {
 	// all the way to the root of the git repository, causing Go's test caching
 	// to never kick in, as the .git directory almost always changes.
 	// Moreover, it's extra work that isn't useful to the tests.
-	c, err := (&Config{ModuleRoot: ".", Dir: cwd}).complete()
-	if err != nil {
-		return nil, fmt.Errorf("unexpected error on Config.complete: %v", err)
+	insts := Instances([]string{pkg}, &Config{ModuleRoot: ".", Dir: cwd})
+	if len(insts) != 1 {
+		return nil, fmt.Errorf("expected one instance, got %d", len(insts))
 	}
-	l := newLoader(c, nil, nil)
-	inst := l.newRelInstance(token.NoPos, pkg, c.Package)
-	p := l.importPkg(token.NoPos, inst)[0]
-	return p, p.Err
+	inst := insts[0]
+	return inst, inst.Err
 }
 
 func TestEmptyImport(t *testing.T) {
-	c, err := (&Config{
-		ModuleRoot: ".",
-	}).complete()
-	if err != nil {
-		t.Fatal(err)
-	}
-	l := newLoader(c, nil, nil)
-	inst := l.newInstance(token.NoPos, "")
-	p := l.importPkg(token.NoPos, inst)[0]
-	if p.Err == nil {
+	path := testdata("testmod", "hello")
+	p, err := getInst("", path)
+	if err == nil {
 		t.Fatal(`Import("") returned nil error.`)
 	}
 	if p == nil {
diff --git a/cue/load/loader_common.go b/cue/load/loader_common.go
index de988032f33..b7c6db01aaa 100644
--- a/cue/load/loader_common.go
+++ b/cue/load/loader_common.go
@@ -466,13 +466,13 @@ func cleanPatterns(patterns []string) []string {
 			a = strings.Replace(a, `\`, `/`, -1)
 		}
 
-		// Put argument in canonical form, but preserve leading ./.
+		// Put argument in canonical form, but preserve leading "./".
 		if strings.HasPrefix(a, "./") {
 			a = "./" + pathpkg.Clean(a)
 			if a == "./." {
 				a = "."
 			}
-		} else {
+		} else if a != "" {
 			a = pathpkg.Clean(a)
 		}
 		out = append(out, a)