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: No error for unkeyed use of composite literals where type is omitted in slice element #23539

Closed
dfawley opened this issue Jan 24, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@dfawley
Copy link

commented Jan 24, 2018

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

go version devel +165e7523fb linux/amd64; same with all earlier versions tested (1.8, 1.9 at least)

Does this issue reproduce with the latest release?

Yes

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

(can provide if needed)

What did you do?

See illegal use of composite literals here:
https://github.com/grpc/grpc-go/blob/5ba054bf3709c0a51d1ed76f68d274a8c1eef459/examples/route_guide/client/client.go#L103-L110

$ go vet examples/route_guide/client/client.go
<no output>
$

What did you expect to see?

An error

What did you see instead?

No error


Changing the lines to specify &pb.RouteNote{} results in an error as expected:

$ go vet examples/route_guide/client/client.go 
# command-line-arguments
examples/route_guide/client/client.go:104: google.golang.org/grpc/examples/route_guide/routeguide.RouteNote composite literal uses unkeyed fields

EDIT: grpc-Go has since been updated to stop using unkeyed fields here with grpc/grpc-go#1829.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2018

Can you show us a standalone example that hasn't been changed? Just to clarify exactly what is happening. Thanks.

@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jan 24, 2018

@dfawley

This comment has been minimized.

Copy link
Author

commented Jan 24, 2018

The link above is still valid: https://github.com/grpc/grpc-go/blob/5ba054bf3709c0a51d1ed76f68d274a8c1eef459/examples/route_guide/client/client.go#L103-L110

	notes := []*pb.RouteNote{
		{&pb.Point{Latitude: 0, Longitude: 1}, "First message"},
		{&pb.Point{Latitude: 0, Longitude: 2}, "Second message"},
		{&pb.Point{Latitude: 0, Longitude: 3}, "Third message"},
		{&pb.Point{Latitude: 0, Longitude: 1}, "Fourth message"},
		{&pb.Point{Latitude: 0, Longitude: 2}, "Fifth message"},
		{&pb.Point{Latitude: 0, Longitude: 3}, "Sixth message"},
	}

Note the slice elements (type *pb.RouteNote) do not use keyed fields.

@dfawley

This comment has been minimized.

Copy link
Author

commented Jan 24, 2018

The fix to properly use field names can be seen here: https://github.com/grpc/grpc-go/pull/1829/files

And adding the type to the slice elements allows vet to detect the error, i.e.:

notes := []*pb.RouteNote{
		&pb.RouteNote{&pb.Point{Latitude: 0, Longitude: 1}, "First message"},
		&pb.RouteNote{&pb.Point{Latitude: 0, Longitude: 2}, "Second message"},
		&pb.RouteNote{&pb.Point{Latitude: 0, Longitude: 3}, "Third message"},
		&pb.RouteNote{&pb.Point{Latitude: 0, Longitude: 1}, "Fourth message"},
		&pb.RouteNote{&pb.Point{Latitude: 0, Longitude: 2}, "Fifth message"},
		&pb.RouteNote{&pb.Point{Latitude: 0, Longitude: 3}, "Sixth message"},
	}

@mvdan mvdan self-assigned this Jan 25, 2018

@dotaheor

This comment has been minimized.

Copy link

commented Jan 25, 2018

What is the problem here? It looks all the following literal forms are legal.

package main

type Point struct {
    Latitude  int32 `protobuf:"varint,1,opt,name=latitude" json:"latitude,omitempty"`
    Longitude int32 `protobuf:"varint,2,opt,name=longitude" json:"longitude,omitempty"`
}

type RouteNote struct {
    // The location from which the message is sent.
    Location *Point `protobuf:"bytes,1,opt,name=location" json:"location,omitempty"`
    // The message to be sent.
    Message string `protobuf:"bytes,2,opt,name=message" json:"message,omitempty"`
}

var notes = []*RouteNote{
	{&Point{Latitude: 0, Longitude: 1}, "First message"},
	{&Point{Latitude: 0, Longitude: 2}, "Second message"},
	{&Point{Latitude: 0, Longitude: 3}, "Third message"},
	{&Point{Latitude: 0, Longitude: 1}, "Fourth message"},
	{&Point{Latitude: 0, Longitude: 2}, "Fifth message"},
	{&Point{Latitude: 0, Longitude: 3}, "Sixth message"},
}

var notes2 = []*RouteNote{
	&RouteNote{&Point{Latitude: 0, Longitude: 1}, "First message"},
	&RouteNote{&Point{Latitude: 0, Longitude: 2}, "Second message"},
	&RouteNote{&Point{Latitude: 0, Longitude: 3}, "Third message"},
	&RouteNote{&Point{Latitude: 0, Longitude: 1}, "Fourth message"},
	&RouteNote{&Point{Latitude: 0, Longitude: 2}, "Fifth message"},
	&RouteNote{&Point{Latitude: 0, Longitude: 3}, "Sixth message"},
}

var notes3 = []*RouteNote{
	{Location: &Point{Latitude: 0, Longitude: 1}, Message: "First message"},
	{Location: &Point{Latitude: 0, Longitude: 2}, Message: "Second message"},
	{Location: &Point{Latitude: 0, Longitude: 3}, Message: "Third message"},
	{Location: &Point{Latitude: 0, Longitude: 1}, Message: "Fourth message"},
	{Location: &Point{Latitude: 0, Longitude: 2}, Message: "Fifth message"},
	{Location: &Point{Latitude: 0, Longitude: 3}, Message: "Sixth message"},
}

var notes4 = []*RouteNote{
	&RouteNote{Location: &Point{Latitude: 0, Longitude: 1}, Message: "First message"},
	&RouteNote{Location: &Point{Latitude: 0, Longitude: 2}, Message: "Second message"},
	&RouteNote{Location: &Point{Latitude: 0, Longitude: 3}, Message: "Third message"},
	&RouteNote{Location: &Point{Latitude: 0, Longitude: 1}, Message: "Fourth message"},
	&RouteNote{Location: &Point{Latitude: 0, Longitude: 2}, Message: "Fifth message"},
	&RouteNote{Location: &Point{Latitude: 0, Longitude: 3}, Message: "Sixth message"},
}

func main() {}
@mvdan

This comment has been minimized.

Copy link
Member

commented Jan 25, 2018

@dotaheor this is not about valid and invalid programs - this is about vet, a static analysis tool. What it's warning about here is when one initialises struct values of types in other packages, and sets their fields without using keys. This is very error-prone, as struct types tend to get new fields added, and one has no control over that if it's in another package.

The problem here is the pointer - I found a fix, CL incoming.

@mvdan

This comment has been minimized.

Copy link
Member

commented Jan 25, 2018

Slight correction - vet does support struct pointer literals, but it was getting confused in this case where they are elements with omitted types.

@gopherbot

This comment has been minimized.

Copy link

commented Jan 25, 2018

Change https://golang.org/cl/89715 mentions this issue: cmd/vet: warn on unkeyed struct pointer literals

mvdan added a commit to mvdan/pprof that referenced this issue Jan 25, 2018

internal/driver: fix new unkeyed composite lit vet warnings
The current version of go vet does not flag these as noted in
golang/go#23539. A fix is in the works, and will likely be included in
Go 1.11. Make the updated vet tool happy, so that the vendored copy of
pprof in the Go tree can be updated and make the vet trybot happy.

Fixes google#297.

aalexand added a commit to google/pprof that referenced this issue Jan 25, 2018

internal/driver: fix new unkeyed composite lit vet warnings (#298)
The current version of go vet does not flag these as noted in
golang/go#23539. A fix is in the works, and will likely be included in
Go 1.11. Make the updated vet tool happy, so that the vendored copy of
pprof in the Go tree can be updated and make the vet trybot happy.

Fixes #297.

lannadorai added a commit to lannadorai/pprof that referenced this issue Feb 13, 2018

internal/driver: fix new unkeyed composite lit vet warnings (google#298)
The current version of go vet does not flag these as noted in
golang/go#23539. A fix is in the works, and will likely be included in
Go 1.11. Make the updated vet tool happy, so that the vendored copy of
pprof in the Go tree can be updated and make the vet trybot happy.

Fixes google#297.

@gopherbot gopherbot closed this in 6ded116 Feb 21, 2018

@golang golang locked and limited conversation to collaborators Feb 21, 2019

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.