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: 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

Comments

@myitcv
Copy link
Member

@myitcv 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

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Dec 10, 2020

Also CC @timothy-king.

@mvdan
Copy link
Member

@mvdan 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

@dmitshur dmitshur commented Dec 10, 2020

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

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 10, 2020

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

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 10, 2020

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

@timothy-king
Copy link
Contributor

@timothy-king timothy-king commented Dec 10, 2020

@ianlancetaylor I can review this.

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Dec 11, 2020

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

@ianlancetaylor ianlancetaylor commented Dec 11, 2020

Thanks.

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 11, 2020

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

@gopherbot gopherbot closed this in 58e381b Dec 11, 2020
marwan-at-work added a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
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

@gopherbot gopherbot commented Jan 6, 2021

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

@gopherbot gopherbot commented Jan 7, 2021

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

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 8, 2021

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

gopherbot pushed a commit that referenced this issue Jan 8, 2021
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
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
@timothy-king @dmitshur @ianlancetaylor @myitcv @mvdan @gopherbot and others