Looks like this is due to the special name testdata. Any other module name works.

@bcmills @matloob I think this behavior may be a bug, or should be documented at https://go.dev/ref/mod, which appears to mention only testdata directories, not module names. It's particularly surprising that e.g. module foo.com/testdata seems to work. It's really just the module name testdata that does not work.

Repurposing this issue to mitigate the surprise.

Change https://go.dev/cl/559457 mentions this issue: gopls/internal/test/marker: reject "module testdata"

supposedly almost all undotted names are considered reserved #37641 ?

@seankhliao yes, if that were uniformly enforced I think that would be a fine solution. But any other undotted name works in this case -- testdata is unique.

I assigned it to myself for the gopls CL above; reassigning to bcmills for the go command behavior.

The go list behavior looks correct to me:

$ go version
go version devel go1.23-d278d5bb Fri Jan 26 04:31:42 2024 +0000 linux/amd64

$ go mod init testdata
go: creating new go.mod: module testdata

$ cat > main.go
package main

func main() {}

$ go list .
ambiguous import: found package testdata in multiple modules:
        testdata (/tmp/tmp.tYGTQN1PjH)
         (/usr/local/google/home/bcmills/sdk/gotip/src/testdata)

@adonovan, can you give more concrete steps to reproduce the behavior seen in the marker tests?

Hmm. I guess in theory this shouldn't be ambiguous, because the testdata directory in the Go standard library doesn't include any .go source files:
https://cs.opensource.google/go/go/+/master:src/testdata/

So I think this is arguably a bug in internal/goroot.IsStandardPackage, which (as an optimization) only checks for the existence of the directory, not source files within that directory. (That is in contrast to the logic for resolving modules, which requires at least one .go source file to be present in order to locate a package.)

This session reduces the problem to the two packages.Load calls made by gopls.

First we use the name testdata.com:

$ cd /tmp/xx
$ cat go.mod
$ echo "module testdata.com" > go.mod
$ echo "package p" > p.go
$ gopackages $(pwd)/... builtin
Go package "testdata.com":
	package p
	has no exported type info
	file /tmp/xx/p.go

Go package "builtin":
	package builtin
	has no exported type info
	file /Users/adonovan/w/goroot/src/builtin/builtin.go
	import "cmp"

$ gopackages file=$(pwd)/p.go
Go package "command-line-arguments":
	package p
	has no exported type info
	file /tmp/xx/p.go

Now we use testdata:

$ echo "module testdata" > go.mod
$ gopackages $(pwd)/... builtin
Go package "testdata":
	package 
	has no exported type info
	-: ambiguous import: found package testdata in multiple modules:
	testdata (/tmp/xx)
	 (/Users/adonovan/w/goroot/src/testdata)

Go package "builtin":
	package builtin
	has no exported type info
	file /Users/adonovan/w/goroot/src/builtin/builtin.go
	import "cmp"

$ gopackages file=$(pwd)/p.go
Go package "command-line-arguments":
	package p
	has no exported type info
	file /tmp/xx/p.go

The difference is that the dir/... query in the second case matches the testdata directory in the standard library, making it ambiguous. This causes gopls to issue the second query using file=, which creates the ad hoc package.

So this reduces to:

$ go list testdata
ambiguous import: found package testdata in multiple modules:
	testdata (/tmp/xx)
	 (/Users/adonovan/w/goroot/src/testdata)

So the question is: is testdata really a standard package?

The same behavior occurs when the go.mod name conflicts with any other standard library directory name, such as encoding or fmt, even if it has no .go files, like testdata or text. Is that a bug? Not sure.

@bcmills. Do you think we should change IsStandardImportPath (and the moduleindex variant of it) to check to see that there's at least one .go file in the directory?

We probably should, yeah. We can probably get rid of the call to os.Stat and use ReadDir instead, although it may be somewhat more expensive. I wonder if we should also cache the results using a sync.Map. 🤔

Change https://go.dev/cl/561338 mentions this issue: internal/goroot: in IsStandardPackage check for go source files

Usually we're using the module index, so we probably won't hit the goroot.IsStandardPackage function so caching it probably wouldn't make a big difference.

Oh! I just remembered https://go.dev/cl/504063, which seems very closely related.

I wonder if it would be less error-prone for future maintenance to establish the invariant that modindex.GetPackage always returns a non-nil error for a directory that does not contain a Go package, similar to go/build.ImportDir.