Skip to content

Commit

Permalink
cue/load: restore the error result on an empty string argument
Browse files Browse the repository at this point in the history
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í <[email protected]>
Change-Id: I0af58e59bb75b8046ec83f4b5cae57c28b138bde
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194410
Reviewed-by: Paul Jolly <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
mvdan committed May 7, 2024
1 parent 3ebadf1 commit 5e9aa4e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 20 deletions.
26 changes: 8 additions & 18 deletions cue/load/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"testing"

"cuelang.org/go/cue/build"
"cuelang.org/go/cue/token"
)

func testdata(elems ...string) string {
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cue/load/loader_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 5e9aa4e

Please sign in to comment.