Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/vet: flags composite literals of slice types from other packages as using unkeyed fields #11394

Closed
akrennmair opened this issue Jun 25, 2015 · 9 comments

Comments

Projects
None yet
8 participants
@akrennmair
Copy link

commented Jun 25, 2015

My Go version is go version go1.4.2 linux/amd64.

I've got the following situation:

$ for f in `find unkeyed-example -type f` ; do printf "\n=== $f ===\n" ; cat $f ; done

=== unkeyed-example/slice/slice.go ===
package slice

type Int []int

=== unkeyed-example/slice/slice_test.go ===
package slice

import "testing"

func TestIntSlice(t *testing.T) {
        value := 42

        data := Int{
                value,
        }

        t.Logf("data = %#v\n", data)
}

=== unkeyed-example/ex2/main.go ===
package main

import (
        "fmt"
        "unkeyed-example/slice"
)

func main() {
        value := 42

        data := slice.Int{
                0: value,
        }

        fmt.Printf("data = %#v\n", data)
}

=== unkeyed-example/ex1/main.go ===
package main

import (
        "fmt"
        "unkeyed-example/slice"
)

func main() {
        value := 42

        data := slice.Int{
                value,
        }

        fmt.Printf("data = %#v\n", data)
}

When running go vet in unkeyed-example/slice, I get no error.

When running go vet in unkeyed-example/ex1, I get the following output: main.go:11: slice.Int composite literal uses unkeyed fields. In this case, I expect go vet to treat the literal slice.Int{ value } the same as Int{ value }, and thus, not to indicate an error.

When running go vet in unkeyed-example/ex2, I again get no error. This is meant as validation that go vetindeed expects an explicit key and will show no error when provided with one (in this case, the key is an index to the slice).

My general expectation here is that go vet would behave consistently, no matter whether a literal refers to a slice type within the same package or to a slice type from another package.

@cespare

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

I also ran into this today. Here is a repro as small as I can easily make it (it requires an imported package):

$ mkdir p
$ cat << EOF > p/p.go
> package p
> type X []interface{}
> EOF
$ cat << EOF > vet.go
> package main
> import "./p"
> func main() {
>   var x p.X
>   x = append(x, p.X{1})
> }
> EOF
$ go vet .
vet.go:5: p.X composite literal uses unkeyed fields
exit status 1

To be clear, this seems like a totally bogus warning.

@adg adg changed the title x/tools/cmd/vet flags composite literals of slice types from other packages as using unkeyed fields cmd/vet: flags composite literals of slice types from other packages as using unkeyed fields Jun 30, 2015

@adg

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

The vet check needs to know about types. cc @josharian @robpike

@josharian

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

I believe that this is yet another case of using gcimporter when what we really want to do is to load from source. Note that if you put @cespare's example into GOPATH and use an absolute import rather than a relative import, vet does the right thing.

This is a perennial problem at this point. The correct, deep fix is probably to stabilize the go/loader API (cc @alandonovan), put go/loader in the stdlib, and use it in cmd/vet.

The superficial fix is to not complain when we are missing type information, which seems like a better default anyway. I can send a CL for that if @robpike agrees.

@cespare

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

Note that if you put @cespare's example into GOPATH and use an absolute import rather than a relative import, vet does the right thing.

I don't think this is true. I originally found this problem with real packages and only used a relative path for my repro. Here's another GOPATH-based repro:

$ mkdir -p src/p
$ cat << EOF > src/p/p.go
package p
type X []interface{}
EOF
$ mkdir src/vet
$ cat << EOF > src/vet/vet.go
> package main
> import "p"
> func main() {
>   var x p.X
>   x = append(x, p.X{1})
> }
> EOF
$ GOPATH=`pwd` go vet vet
src/vet/vet.go:5: p.X composite literal uses unkeyed fields
exit status 1

@josharian

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

@cespare if you go install p vet and then try again, does it still reproduce?

@cespare

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

@josharian ah, it does not.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

Yep. :) Loading from source would fix that.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2015

Josh: Maybe open an issue for goimporter and link it to here?

@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2017

This appears to have been fixed in Go 1.7. It's extra-fixed now because in Go 1.10 go vet makes sure that vet has full type information 100% of the time.

@rsc rsc closed this Nov 1, 2017

@golang golang locked and limited conversation to collaborators Nov 1, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.