go/types: type aliases can lead to invalid types

[kjk]

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

go version go1.10.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/kjk/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/kjk/src/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10.3/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10.3/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
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=/var/folders/v_/ksw1dqvd59v790zk2wqf_t_80000gn/T/go-build335215962=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

$ git clone https://github.com/kjk/ravendb-go-client.git
$ cd ravendb-go-client.git
$ go vet
./metadata_as_dictionary.go:163:9: cannot use list (variable of type []*MetadataAsDictionary) as []*invalid type value in return statement
./document_query_customization_delegate.go:22:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:27:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:32:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:37:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:42:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:47:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:52:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:57:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:62:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:67:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:72:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:77:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./in_memory_document_session_operations.go:175:9: cannot use s._documentStore (variable of type *DocumentStore) as *invalid type value in return statement
./in_memory_document_session_operations.go:185:26: invalid operation: s.getDocumentStore() (value of type *invalid type) has no field or method operations
./abstract_index_creation_task.go:107:9: invalid operation: store (variable of type *invalid type) has no field or method executeIndex
./abstract_index_creation_task.go:123:10: invalid operation: store (variable of type *invalid type) has no field or method getConventions
./abstract_index_creation_task.go:140:14: invalid operation: store (variable of type *invalid type) has no field or method getDatabase
./abstract_index_creation_task.go:142:9: invalid operation: store (variable of type *invalid type) has no field or method maintenance
./document_store.go:267:23: cannot use s (variable of type *DocumentStore) as *invalid type value in argument to task.execute2
./index_creation.go:18:24: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to index.execute2
./attachments_revisions_test.go:326:16: invalid operation: attachment (variable of type *invalid type) has no field or method get
./index_operations_test.go:28:22: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to index.execute
./index_operations_test.go:54:22: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to index.execute
./index_operations_test.go:94:22: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to index.execute
./index_operations_test.go:110:22: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to index.execute
./index_operations_test.go:128:22: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to index.execute
./index_operations_test.go:366:26: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to userIndex.execute
./indexes_from_client_test.go:133:34: cannot use store (variable of type *DocumentStore) as *invalid type value in argument to NewUsers_ByName().execute
vet: typecheck failures

What did you expect to see?

This is a valid program that compiles. go vet should not complain.

What did you see instead?

go vet complaints.

Those errors are due to using type Foo = Bar aliases. Those invalid types are type aliases.

I know that using type aliases is bad style. Not handling them is still a bug.

I tried to minimize the repro but I failed to create a small example so I'm showing the whole code.

I assume this is a problem with ast package because other tools break on this codebase as well, like rename symbol functionality in Visual Studio Code.

[mvdan]

It seems like go/types is getting confused somewhere and ending up with invalid type, as you can see from the error messages. It's unlikely that the go/ast package is the cause of the issue, as it doesn't deal with type checking.

Not a regression from 1.10, but this seems bad enough to warrant consideration for 1.11. I'll leave it to @griesemer and @alandonovan to decide.

[mvdan]

@kjk also, it appears that your instructions to reproduce the issue aren't correct. Now I'm getting errors like:

./document_session_attachments.go:43:55: cannot use operation (variable of type *GetAttachmentOperation) as IOperation value in argument to s.session.getOperations().sendWithSessionInfo: wrong type for method getCommand

It looks like you have been pushing commits to your branch. Exactly what commit did you use to get the invalid types? Could you push that code to a completely separate repo that builds on its own?

[kjk]

@mvdan It must be something on your end.

I just re-ran my instructions, on mac, using go 1.10.3:

kjkmacpro:src kjk$ git clone https://github.com/kjk/ravendb-go-client.git
Cloning into 'ravendb-go-client'...
remote: Counting objects: 2671, done.
remote: Compressing objects: 100% (7/7), done.
remote: Total 2671 (delta 2), reused 4 (delta 2), pack-reused 2662
Receiving objects: 100% (2671/2671), 674.51 KiB | 1.37 MiB/s, done.
Resolving deltas: 100% (1607/1607), done.

kjkmacpro:src kjk$ cd ravendb-go-client/

kjkmacpro:ravendb-go-client kjk$ git status
On branch v4.0
Your branch is up to date with 'origin/v4.0'.

nothing to commit, working tree clean

kjkmacpro:ravendb-go-client kjk$ git rev-parse HEAD
bec86311c8847b60a97fce1d4c06e4bf7d199153

kjkmacpro:ravendb-go-client kjk$ go build

Notice it builds.

kjkmacpro:ravendb-go-client kjk$ go vet
# _/Users/kjk/src/ravendb-go-client
./metadata_as_dictionary.go:163:9: cannot use list (variable of type []*MetadataAsDictionary) as []*invalid type value in return statement
./document_query_customization_delegate.go:22:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement
./document_query_customization_delegate.go:27:9: cannot use d (variable of type *DocumentQueryCustomizationDelegate) as *invalid type value in return statement

I even have compile-time check that *GetAttachmentOperation is IOperation: https://github.com/kjk/ravendb-go-client/blob/v4.0/get_attachment_operation.go#L10

And every revision is compiled and tested on Travis: https://travis-ci.org/kjk/ravendb-go-client

[griesemer]

@kjk Is this still an issue with the latest Go 1.11 release candidate? In your instructions above you mention go1.10.3. We have fixed several alias-related bugs for go 1.11 in go/types (where there would be an issue, if any).

[kjk]

It's even worse on HEAD in that go HEAD doesn't even compile code that go 1.10.3 was able to compile.

In 1.10.3 the problem was limited to go vet.

Current repro:

Checkout https://github.com/kjk/ravendb-go-client/commit/90cab786842d66d12e3cbc8548e95af227235623 (the latest version when I write this).

ravendb-go-client kjk$ go version
go version devel +6d0f757bb9 Mon Aug 6 21:16:18 2018 +0000 darwin/amd64
ravendb-go-client kjk$ go build
ravendb-go-client kjk$ go test
# github.com/ravendb/ravendb-go-client
./query_test.go:140:8: cannot use q2.selectKey() (value of type *invalid type) as *GroupByDocumentQuery value in assignment
vet: typecheck failures
FAIL	github.com/ravendb/ravendb-go-client [build failed]

This is on mac using go I just installed with brew install go --HEAD.

When you look at the code, selectKey returns *IGroupByDocumentQuery and IGroupByDocumentQuery is an alias to GroupByDocumentQuery:

https://github.com/kjk/ravendb-go-client/commit/90cab786842d66d12e3cbc8548e95af227235623#diff-1504c96b11521e224a9e8753b3188bcdR13

https://github.com/kjk/ravendb-go-client/commit/90cab786842d66d12e3cbc8548e95af227235623#diff-78842efc04a47184bb27987f50ea08e6R3

And proof that this code compiles with 1.10.3: https://travis-ci.org/kjk/ravendb-go-client/builds/412873534

[griesemer]

@kjk Thanks for the quick response. It does compile but the issue is that go test runs go vet first which then fails. In 1.10, go vet was not run for go test. But this needs investigation. (See also #22890).

[mvdan]

@griesemer thanks for picking this up - it appears I dropped the ball on the investigation. Happy to help review a CL if you end up with one.

[griesemer]

Simple reproducer:

package p

type A = T

func (t *T) m() *A { return t }

type T struct{}

Calling gotype on this package produces:

x.go:5:29: cannot use t (variable of type *T) as *invalid type value in return statement

On the other hand, if type T is declared first, the error goes away. Investigating.

[griesemer]

@kjk Can you please let me know if go test works with the flag -vet=off ? Thanks.

[griesemer]

Only related to #23203 (not a duplicate) - this is really a separate issue that would be solved by the same approach that would fix #23203. For now, working on a temporary solution.

[kjk]

@griesemer Yes, go test -vet=off works.

[griesemer]

@kjk Thanks for the confirmation. I do have a smallish fix for this in go/types (assuming it gets ok'ed), but it's just a work-around as we can't do the real fix at this point of the release process (the real fix is slated for 1.12). Will post a CL either tonight or tomorrow.

[gopherbot]

Change https://golang.org/cl/128435 mentions this issue: go/types: if base type for an alias is known, use it when needed

[griesemer]

@kjk If you could verify that the issue is fixed for your build with the latest tip, that would be great. Thanks.

[kjk]

@griesemer Yes, it works with HEAD. Thanks!

[griesemer]

@kjk Great - thanks for confirming!

[gopherbot]

Change https://golang.org/cl/139899 mentions this issue: go/types: remove work-around for issue #26390

[gopherbot]

Change https://golang.org/cl/139900 mentions this issue: go/types: remove work-around for issue #26390