cmd/vet: reporting "composite literal uses unkeyed fields" for slice types #9171

Closed
fsouza opened this Issue Nov 26, 2014 · 7 comments

Comments

Projects
None yet
5 participants
@fsouza
Contributor

fsouza commented Nov 26, 2014

I have a typed defined from a slice (type TemplateDataList []TemplateData), and when I
create instances of this type

% go version
go version devel +ffe33f1f1f17 Tue Nov 25 15:41:33 2014 +1100 darwin/amd64
% cd $GOPATH/src/golang.org/x/tools
% hg tip
changeset:   1264:e105dadc3014
tag:         tip
user:        Andrew Gerrand <adg@golang.org>
date:        Wed Nov 26 15:31:30 2014 +1100
summary:     x/tools/cmd/godoc: add golang.org/x/oauth2 import path
% cd -
% cat code_bad.go
package main

import (
    "fmt"
    "github.com/tsuru/tsuru/iaas"
)

type Template struct {
    Name string
    Data iaas.TemplateDataList
}

func main() {
    template := Template{
        Name: "My Template",
        Data: iaas.TemplateDataList{
            {Name: "color", Value: "red"},
            {Name: "size", Value: "100"},
        },
    }
    fmt.Println(template)
}
% go vet code_bad.go
code_bad.go:16: iaas.TemplateDataList composite literal uses unkeyed fields
exit status 1

Here's where TemplateDataList is defined:
https://github.com/tsuru/tsuru/blob/0ac00f9d08f67efa39b679b643f74ff6ebba8530/iaas/template.go#L21

Now, if I copy the TemplateDataList definition to the same file, the error disappears:

% cat code_good.go
package main

import (
    "fmt"
    "github.com/tsuru/tsuru/iaas"
)

type TemplateDataList []iaas.TemplateData

type Template struct {
    Name string
    Data TemplateDataList
}

func main() {
    template := Template{
        Name: "My Template",
        Data: TemplateDataList{
            {Name: "color", Value: "red"},
            {Name: "size", Value: "100"},
        },
    }
    fmt.Println(template)
}
% go vet code_good.go
# no failures
@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Nov 26, 2014

Contributor

Comment 1:

I don't know if there is anything that can be done here.

Labels changed: added repo-tools, release-none.

Contributor

ianlancetaylor commented Nov 26, 2014

Comment 1:

I don't know if there is anything that can be done here.

Labels changed: added repo-tools, release-none.

@robpike

This comment has been minimized.

Show comment
Hide comment
@robpike

robpike Dec 1, 2014

Contributor

Comment 2:

With only some of the files in the package presented to vet, it can't get the types
right. Nothing can be done.

Status changed to Unfortunate.

Contributor

robpike commented Dec 1, 2014

Comment 2:

With only some of the files in the package presented to vet, it can't get the types
right. Nothing can be done.

Status changed to Unfortunate.

fsouza added a commit to tsuru/tsuru that referenced this issue Dec 8, 2014

Revert "check-fmt: don't return exit status 1 when vet fails"
golang/go#9171 is closed as unfortunate, we will have to workaround this
issue.

This reverts commit 21bfa74.

fsouza added a commit to tsuru/tsuru that referenced this issue Dec 8, 2014

@mboersma mboersma referenced this issue in deis/deis Jul 29, 2015

Closed

feature(deisctl) allow for graceful in-place upgrades #4099

3 of 4 tasks complete

juliusv added a commit to prometheus/prometheus that referenced this issue Aug 26, 2015

Fix errors mentioned by "go vet".
I ignored all errors of the type "composite literal uses unkeyed
fields". Most of them are wrong because of
golang/go#9171.

juliusv added a commit to prometheus/prometheus that referenced this issue Aug 26, 2015

Fix "go vet" errors.
I ignored all errors of the type "composite literal uses unkeyed
fields". Most of them are wrong because of
golang/go#9171.

@juliusv juliusv referenced this issue in prometheus/prometheus Aug 26, 2015

Merged

Fix "go vet" errors. #1029

@corylanou corylanou referenced this issue in influxdata/influxdb Sep 21, 2015

Merged

Fix go vet warnings #4184

plfiorini added a commit to hawaii-desktop/builder that referenced this issue Oct 11, 2015

Disable vet for now
Reports weird issues like:

  composite literal uses unkeyed fields

See golang/go#9171

fabxc added a commit to prometheus/prometheus that referenced this issue Jan 11, 2016

Fix "go vet" errors.
I ignored all errors of the type "composite literal uses unkeyed
fields". Most of them are wrong because of
golang/go#9171.

jprobinson added a commit to NYTimes/gizmo that referenced this issue Jan 13, 2016

jen20 added a commit to hashicorp/terraform that referenced this issue Feb 18, 2016

provider/chef: Fix go vet issues
This is rather hacky but it should get rid of our last remaining go vet
warning. This appears to be golang/go#9171, which was closed as
"Unfortunate"

bigkraig added a commit to bigkraig/terraform that referenced this issue Mar 1, 2016

provider/chef: Fix go vet issues
This is rather hacky but it should get rid of our last remaining go vet
warning. This appears to be golang/go#9171, which was closed as
"Unfortunate"
@luan

This comment has been minimized.

Show comment
Hide comment
@luan

luan Apr 13, 2016

Contributor

@fsouza @ianlancetaylor

I had a conversation with a coworker (@tedsuo) about this today, we have this which doesn't pass vet, but that's really only possible with a slice literal, so I went and tried the following:

    // Check if the CompositeLit contains an unkeyed field.
    allKeyValue := true
    for _, e := range c.Elts {
        if _, ok := e.(*ast.KeyValueExpr); !ok {
            if cl, ok := e.(*ast.CompositeLit); !ok || cl.Type != nil {
                allKeyValue = false
                break
            }
        }
    }
    if allKeyValue {
        return
    }

To detect composite literals with no type, seems like we can use that logic to not show this unfortunate warning on slice type aliases.
Glad to submit code for code review if folks think this is reasonable.

Contributor

luan commented Apr 13, 2016

@fsouza @ianlancetaylor

I had a conversation with a coworker (@tedsuo) about this today, we have this which doesn't pass vet, but that's really only possible with a slice literal, so I went and tried the following:

    // Check if the CompositeLit contains an unkeyed field.
    allKeyValue := true
    for _, e := range c.Elts {
        if _, ok := e.(*ast.KeyValueExpr); !ok {
            if cl, ok := e.(*ast.CompositeLit); !ok || cl.Type != nil {
                allKeyValue = false
                break
            }
        }
    }
    if allKeyValue {
        return
    }

To detect composite literals with no type, seems like we can use that logic to not show this unfortunate warning on slice type aliases.
Glad to submit code for code review if folks think this is reasonable.

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Apr 13, 2016

Contributor

Seems plausible. Sure, send a CL.

Contributor

ianlancetaylor commented Apr 13, 2016

Seems plausible. Sure, send a CL.

@luan

This comment has been minimized.

Show comment
Hide comment
@gopherbot

This comment has been minimized.

Show comment
Hide comment

CL https://golang.org/cl/22000 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 14, 2016

cmd/vet: allow untyped composite literals to be unkeyed
We can trust that untyped composite literals are part of a slice literal
and not emit a vet warning for those.

Fixes #9171

Change-Id: Ia7c081e543b850f8be1fd1f9e711520061e70bed
Reviewed-on: https://go-review.googlesource.com/22000
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
@gopherbot

This comment has been minimized.

Show comment
Hide comment

CL https://golang.org/cl/22318 mentions this issue.

@joeblew99 joeblew99 referenced this issue in Microsoft/vscode-go Apr 25, 2016

Closed

Vet and new warnings in latest version #311

gopherbot pushed a commit that referenced this issue Apr 28, 2016

cmd/vet: improve checking unkeyed fields in composite literals
- Simplified the code.

- Removed types for slice aliases from composite literals' whitelist, since they
are properly handled by vet.

Fixes #15408
Updates #9171
Updates #11041

Change-Id: Ia1806c9eb3f327c09d2e28da4ffdb233b5a159b0
Reviewed-on: https://go-review.googlesource.com/22318
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

@2opremio 2opremio referenced this issue in weaveworks/scope Aug 2, 2016

Merged

Use slices instead of linked lists for Metric #1732

1 of 1 task complete

@bradfitz bradfitz referenced this issue in golang/crypto Nov 27, 2016

Closed

fix Compile Error #19

@golang golang locked and limited conversation to collaborators Apr 23, 2017

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.