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.

Sign up for GitHub

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

Jump to bottom

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
Closed

cmd/vet: No error for unkeyed use of composite literals where type is omitted in slice element #23539

dfawley opened this issue Jan 24, 2018 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@dfawley
Copy link

dfawley commented Jan 24, 2018 •
edited

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.
@dfawley dfawley mentioned this issue Jan 24, 2018
Closed
@ianlancetaylor
Copy link
Contributor

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

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 24, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jan 24, 2018
@dfawley
Copy link
Author

dfawley 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
Copy link
Author

dfawley commented Jan 24, 2018 •
edited

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
Copy link

dotaheor commented Jan 25, 2018 •
edited

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
Copy link
Member

mvdan 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
Copy link
Member

mvdan 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
Copy link

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

This was referenced Jan 25, 2018
Closed
Merged
aalexand pushed a commit to google/pprof that referenced this issue Jan 25, 2018
@mvdan @aalexand
internal/driver: fix new unkeyed composite lit vet warnings (#298)
37d0150 
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 pushed a commit to lannadorai/pprof that referenced this issue Feb 13, 2018
@mvdan @lannadorai
internal/driver: fix new unkeyed composite lit vet warnings (google#298)
2c8d79e 
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.
@golang golang locked and limited conversation to collaborators Feb 21, 2019
@rsc rsc unassigned mvdan Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
5 participants
@ianlancetaylor @mvdan @gopherbot @dotaheor @dfawley