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

Jump to bottom

cmd/vet: does not understand multiple keys in key:value pair in struct tag #43083

Closed
myitcv opened this issue Dec 8, 2020 · 12 comments
Closed

cmd/vet: does not understand multiple keys in key:value pair in struct tag #43083

myitcv opened this issue Dec 8, 2020 · 12 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.16

Comments

@myitcv
Copy link
Member

myitcv commented Dec 8, 2020

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

$ go version
go version devel +e10c94af26 Mon Dec 7 02:31:33 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +e10c94af26 Mon Dec 7 02:31:33 2020 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.Y0bBClLQls/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build628121616=/tmp/go-build -gno-record-gcc-switches"

What did you do?

go vet .

-- go.mod --
module mod.com

go 1.16
-- main.go --
package main

type MyStruct struct {
	Field1 string `json bson xml form:"field_1,omitempty" other:"value"`
}

What did you expect to see?

No error (because this example is taken from #40281 (comment))

What did you see instead?

./main.go:4:2: struct field tag `json bson xml form:"field_1,omitempty" other:"value"` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair

Marking as a release-blocker because I believe it to be one.

Despite raising this issue, I share the concerns raised by @rogpeppe in #40281 (comment).

cc @rsc @mvdan @ianlancetaylor
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go release-blocker labels Dec 8, 2020
@myitcv myitcv added this to the Go1.16 milestone Dec 8, 2020
@dmitshur
Copy link
Contributor

Also CC @timothy-king.

@mvdan
Copy link
Member

mvdan commented Dec 10, 2020

Hm, I'm surprised that the feature was merged into master without teaching vet about it.

@dmitshur
Copy link
Contributor

@ianlancetaylor has offered to take a look, so I'll update the issue status to make that more visible.

@ianlancetaylor
Copy link
Contributor

Sent https://golang.org/cl/277092 but it needs reviewers.

@gopherbot
Copy link

Change https://golang.org/cl/277092 mentions this issue: go/analysis/passes/structtag: recognize multiple keys per tag

@timothy-king
Copy link
Contributor

@ianlancetaylor I can review this.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 11, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Dec 11, 2020 •
edited

This issue was closed because CL 277092 had a Fixes golang/go#43083 line.

Re-opening to update the vendored copy of golang.org/x/tools in the standard library to pull in that change.

@dmitshur dmitshur reopened this Dec 11, 2020
@ianlancetaylor
Copy link
Contributor

Thanks.

@gopherbot
Copy link

Change https://golang.org/cl/277076 mentions this issue: cmd/vet: vendor in x/tools, update structtag vet check

marwan-at-work pushed a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
@ianlancetaylor @marwan-at-work
go/analysis/passes/structtag: recognize multiple keys per tag
fe95316 
As of Go 1.16 the reflect package now supports multiple keys per tag.

For golang/go#40281
Fixes golang/go#43083

Change-Id: I55cdc35c857a5e73dc009c2842d7bd83c63d7712
Reviewed-on: https://go-review.googlesource.com/c/tools/+/277092
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/281973 mentions this issue: Revert "go/analysis/passes/structtag: recognize multiple keys per tag"

gopherbot pushed a commit to golang/tools that referenced this issue Jan 7, 2021
@ianlancetaylor
Revert "go/analysis/passes/structtag: recognize multiple keys per tag"
4ed967d 
This reverts CL 277092.

Proposal golang/go#40281 was initially accepted, then declined.  Stop accepting
the new, now once again invalid, format.

For golang/go#40281
For golang/go#43083
For golang/go#43226

Change-Id: Ida97263048f3a048f904a844f577d8353e3a1afa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/281973
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/282412 mentions this issue: cmd: update to latest golang.org/x/tools

@ianlancetaylor
Copy link
Contributor

Note: proposal #40281 was declined after all, so this change was rolled back.

gopherbot pushed a commit that referenced this issue Jan 8, 2021
@ianlancetaylor
cmd: update to latest golang.org/x/tools
54bd1cc 
In particular bring in CL 201973, which reverts support for multiple
keys in a struct tag.

For #40281
For #43083
For #43226

Change-Id: I66e76639cbbca55bdbff6956acdb0a97650fdd31
Reviewed-on: https://go-review.googlesource.com/c/go/+/282412
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
gopherbot pushed a commit to golang/tools that referenced this issue Jan 11, 2021
@ianlancetaylor @dmitshur
Revert "go/analysis/passes/structtag: recognize multiple keys per tag"
21398c4 
This reverts CL 277092.

Proposal golang/go#40281 was initially accepted, then declined.  Stop accepting
the new, now once again invalid, format.

For golang/go#40281
For golang/go#43083
For golang/go#43226

Change-Id: Ida97263048f3a048f904a844f577d8353e3a1afa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/281973
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@golang golang locked and limited conversation to collaborators Jan 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.16
Development

No branches or pull requests
7 participants
@timothy-king @dmitshur @ianlancetaylor @myitcv @mvdan @gopherbot and others