From 4ed967dd8efff4825c99d25809154e3f5ac7a6b3 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 6 Jan 2021 14:38:44 -0800 Subject: [PATCH] Revert "go/analysis/passes/structtag: recognize multiple keys per tag" 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 Reviewed-by: Heschi Kreinick --- go/analysis/passes/structtag/structtag.go | 98 ++++++++----------- .../passes/structtag/structtag_go16_test.go | 21 ---- .../structtag/testdata/src/go16/go16.go | 27 ----- 3 files changed, 40 insertions(+), 106 deletions(-) delete mode 100644 go/analysis/passes/structtag/structtag_go16_test.go delete mode 100644 go/analysis/passes/structtag/testdata/src/go16/go16.go diff --git a/go/analysis/passes/structtag/structtag.go b/go/analysis/passes/structtag/structtag.go index 02555648a..f0b15051c 100644 --- a/go/analysis/passes/structtag/structtag.go +++ b/go/analysis/passes/structtag/structtag.go @@ -207,12 +207,12 @@ var ( ) // validateStructTag parses the struct tag and returns an error if it is not -// in the canonical format, as defined by reflect.StructTag. +// in the canonical format, which is a space-separated list of key:"value" +// settings. The value may contain spaces. func validateStructTag(tag string) error { // This code is based on the StructTag.Get code in package reflect. n := 0 - var keys []string for ; tag != ""; n++ { if n > 0 && tag != "" && tag[0] != ' ' { // More restrictive than reflect, but catches likely mistakes @@ -240,27 +240,14 @@ func validateStructTag(tag string) error { if i == 0 { return errTagKeySyntax } - if i+1 >= len(tag) || tag[i] < ' ' || tag[i] == 0x7f { + if i+1 >= len(tag) || tag[i] != ':' { return errTagSyntax } - key := tag[:i] - keys = append(keys, key) - tag = tag[i:] - - // If we found a space char here - assume that we have a tag with - // multiple keys. - if tag[0] == ' ' { - continue - } - - // Spaces were filtered above so we assume that here we have - // only valid tag value started with `:"`. - if tag[0] != ':' || tag[1] != '"' { + if tag[i+1] != '"' { return errTagValueSyntax } - - // Remove the colon leaving tag at the start of the quoted string. - tag = tag[1:] + key := tag[:i] + tag = tag[i+1:] // Scan quoted string to find value. i = 1 @@ -276,56 +263,51 @@ func validateStructTag(tag string) error { qvalue := tag[:i+1] tag = tag[i+1:] - wholeValue, err := strconv.Unquote(qvalue) + value, err := strconv.Unquote(qvalue) if err != nil { return errTagValueSyntax } - for _, key := range keys { - if !checkTagSpaces[key] { + if !checkTagSpaces[key] { + continue + } + + switch key { + case "xml": + // If the first or last character in the XML tag is a space, it is + // suspicious. + if strings.Trim(value, " ") != value { + return errTagValueSpace + } + + // If there are multiple spaces, they are suspicious. + if strings.Count(value, " ") > 1 { + return errTagValueSpace + } + + // If there is no comma, skip the rest of the checks. + comma := strings.IndexRune(value, ',') + if comma < 0 { continue } - value := wholeValue - switch key { - case "xml": - // If the first or last character in the XML tag is a space, it is - // suspicious. - if strings.Trim(value, " ") != value { - return errTagValueSpace - } - - // If there are multiple spaces, they are suspicious. - if strings.Count(value, " ") > 1 { - return errTagValueSpace - } - - // If there is no comma, skip the rest of the checks. - comma := strings.IndexRune(value, ',') - if comma < 0 { - continue - } - - // If the character before a comma is a space, this is suspicious. - if comma > 0 && value[comma-1] == ' ' { - return errTagValueSpace - } - value = value[comma+1:] - case "json": - // JSON allows using spaces in the name, so skip it. - comma := strings.IndexRune(value, ',') - if comma < 0 { - continue - } - value = value[comma+1:] - } - - if strings.IndexByte(value, ' ') >= 0 { + // If the character before a comma is a space, this is suspicious. + if comma > 0 && value[comma-1] == ' ' { return errTagValueSpace } + value = value[comma+1:] + case "json": + // JSON allows using spaces in the name, so skip it. + comma := strings.IndexRune(value, ',') + if comma < 0 { + continue + } + value = value[comma+1:] } - keys = keys[:0] + if strings.IndexByte(value, ' ') >= 0 { + return errTagValueSpace + } } return nil } diff --git a/go/analysis/passes/structtag/structtag_go16_test.go b/go/analysis/passes/structtag/structtag_go16_test.go deleted file mode 100644 index 2b0867f18..000000000 --- a/go/analysis/passes/structtag/structtag_go16_test.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +build go1.16 - -package structtag_test - -import ( - "testing" - - "golang.org/x/tools/go/analysis/analysistest" - "golang.org/x/tools/go/analysis/passes/structtag" -) - -// Test the multiple key format added in Go 1.16. - -func TestGo16(t *testing.T) { - testdata := analysistest.TestData() - analysistest.Run(t, testdata, structtag.Analyzer, "go16") -} diff --git a/go/analysis/passes/structtag/testdata/src/go16/go16.go b/go/analysis/passes/structtag/testdata/src/go16/go16.go deleted file mode 100644 index 895d55abd..000000000 --- a/go/analysis/passes/structtag/testdata/src/go16/go16.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Tests for the new multiple-key struct tag format supported in 1.16. - -package go16 - -type Go16StructTagTest struct { - OK int `multiple keys can:"share a value"` - OK2 int `json bson xml form:"field_1,omitempty" other:"value"` -} - -type Go16UnexportedEncodingTagTest struct { - F int `json xml:"ff"` - - // We currently always check json first, and return after an error. - f1 int `json xml:"f1"` // want "struct field f1 has json tag but is not exported" - f2 int `xml json:"f2"` // want "struct field f2 has json tag but is not exported" - f3 int `xml bson:"f3"` // want "struct field f3 has xml tag but is not exported" - f4 int `bson xml:"f4"` // want "struct field f4 has xml tag but is not exported" -} - -type Go16DuplicateFields struct { - JSONXML int `json xml:"c"` - DuplicateJSONXML int `json xml:"c"` // want "struct field DuplicateJSONXML repeats json tag .c. also at go16.go:25" "struct field DuplicateJSONXML repeats xml tag .c. also at go16.go:25" -}