go/types: include import path in type error messages

[stapelberg]

What version of Go are you using (go version)?

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/stapelberg/.cache/go-build"
GOENV="/usr/local/google/home/stapelberg/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/tmp/bug"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/google-golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build918009785=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have the following Go source files (directory structure is relevant, so no play.golang.org link):

% head -100 pkg1/pkg1.go nested/pkg1/pkg1.go third/third.go demo/demo.go ast/ast.go
==> pkg1/pkg1.go <==
package pkg1

type Client struct{}

==> nested/pkg1/pkg1.go <==
package pkg1

type Client struct{}

==> third/third.go <==
package third

import "pkg1"

func Something(*pkg1.Client) {}

==> demo/demo.go <==
package main

import (
	"nested/pkg1"
	"third"
)

func main() {
	h := &pkg1.Client{}
	third.Something(h)
}

==> ast/ast.go <==
package main

import (
	"fmt"
	"os"

	"golang.org/x/tools/go/packages"
)

func main() {
	cfg := &packages.Config{
		Mode: packages.NeedFiles | packages.NeedSyntax | packages.NeedImports | packages.NeedTypes,
	}
	pkgs, err := packages.Load(cfg, "demo")
	if err != nil {
		fmt.Fprintf(os.Stderr, "load: %v\n", err)
		os.Exit(1)
	}
	if packages.PrintErrors(pkgs) > 0 {
		os.Exit(1)
	}
	fmt.Printf("pkgs: %v", pkgs)
}

When using go run demo.go, I get:

./demo.go:10:17: cannot use h (type *"nested/pkg1".Client) as type *"pkg1".Client in argument to third.Something

However, when using go run ast.go, I get:

/tmp/bug/src/demo/demo.go:10:18: cannot use h (variable of type *pkg1.Client) as *pkg1.Client value in argument to third.Something

The latter message does not include the import path, only the package, and is hence very confusing especially for newcomers to the language: the two *pkg1.Client look exactly the same, so the error message looks non-sensical.

The message is produced by the go/types package in

go/src/go/types/assignments.go

Line 64 in 8054b13

check.errorf(x.pos(), "cannot use %s as %s value in %s", x, T, context)

Note that some users in some environments might be exposed to error messages from go/types before they see the error message from the Go compiler because of how IDE integration works.

Could we include the import path in the error messages produced by go/types as well please?

[mvdan]

Same issue, but for runtime errors: #11634

[dmitshur]

/cc @griesemer per owners.

[griesemer]

Marking tentatively for 1.14 since this only affects an error message.

[gopherbot]

Change https://golang.org/cl/209578 mentions this issue: go/types: print full package path for imported packages in error messages

[griesemer]

The fix is trivial (https://golang.org/cl/209578) but may have implications for client code that parses error messages. Also, we're currently in the 1.14 freeze and this may cause needless instability. Marking NeedsDecision for input from others.

[dmitshur]

may have implications for client code that parses error messages

/cc @neild who worked on defining an internal policy related to that, which can be helpful when making decisions about changing error messages.

I'm okay with either, but I think it makes sense to leave to 1.15, since it's not fixing an existing bug.

[rsc]

Note that the compiler only prints the full import path when it sees ambiguity (same package name in multiple imports, I think).

[ianlancetaylor]

The relevant code for the compiler is in cmd/compile/internal/gc/fmt.go:

			// If the name was used by multiple packages, display the full path,
			if s.Pkg.Name != "" && numImport[s.Pkg.Name] > 1 {
				return fmt.Sprintf("%q.%s", s.Pkg.Path, s.Name)
			}
			return s.Pkg.Name + "." + s.Name