Check whether the tag name is duplicate or not

rule/struct-tag.go

@@ -34,8 +34,9 @@ func (*StructTagRule) Name() string {
 }
 
 type lintStructTagRule struct {
-	onFailure  func(lint.Failure)
-	usedTagNbr map[string]bool // list of used tag numbers
+	onFailure   func(lint.Failure)
+	usedTagNbr  map[string]bool // list of used tag numbers
+	usedTagName map[string]bool // list of used tag keys
 }
 
 func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor {
@@ -44,7 +45,8 @@ func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor {
 		if n.Fields == nil || n.Fields.NumFields() < 1 {
 			return nil // skip empty structs
 		}
-		w.usedTagNbr = map[string]bool{} // init
+		w.usedTagNbr = map[string]bool{}  // init
+		w.usedTagName = map[string]bool{} // init
 		for _, f := range n.Fields.List {
 			if f.Tag != nil {
 				w.checkTaggedField(f)
@@ -55,6 +57,49 @@ func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor {
 	return w
 }
 
+func (w lintStructTagRule) checkTagNameIfNeed(tag *structtag.Tag) (string, bool) {
+	isUnnamedTag := tag.Name == "" || tag.Name == "-"
+	if isUnnamedTag {
+		return "", true
+	}
+
+	needsToCheckTagName := tag.Key == "bson" ||
+		tag.Key == "json" ||
+		tag.Key == "xml" ||
+		tag.Key == "yaml" ||
+		tag.Key == "protobuf"
+
+	if !needsToCheckTagName {
+		return "", true
+	}
+
+	tagName := w.getTagName(tag)
+	// We concat the key and name as the mapping key here
+	// to allow the same tag name in different tag type.
+	key := tag.Key + ":" + tagName
+	if _, ok := w.usedTagName[key]; ok {
+		return fmt.Sprintf("duplicate tag name: '%s'", tagName), false
+	}
+
+	w.usedTagName[key] = true
+
+	return "", true
+}
+
+func (lintStructTagRule) getTagName(tag *structtag.Tag) string {
+	switch tag.Key {
+	case "protobuf":
+		for _, option := range tag.Options {
+			if strings.HasPrefix(option, "name=") {
+				return strings.TrimLeft(option, "name=")
+			}
+		}
+		return "protobuf tag lacks name"
+	default:
+		return tag.Name
+	}
+}
+
 // checkTaggedField checks the tag of the given field.
 // precondition: the field has a tag
 func (w lintStructTagRule) checkTaggedField(f *ast.Field) {
@@ -69,6 +114,10 @@ func (w lintStructTagRule) checkTaggedField(f *ast.Field) {
 	}
 
 	for _, tag := range tags.Tags() {
+		if msg, ok := w.checkTagNameIfNeed(tag); !ok {
+			w.addFailure(f.Tag, msg)
+		}
+
 		switch key := tag.Key; key {
 		case "asn1":
 			msg, ok := w.checkASN1Tag(f.Type, tag)

testdata/struct-tag.go

@@ -4,25 +4,27 @@ import "time"
 
 type decodeAndValidateRequest struct {
 	// BEAWRE : the flag of URLParam should match the const string URLParam
-	URLParam         string          `json:"-" path:"url_param" validate:"numeric"`
-	Text             string          `json:"text" validate:"max=10"`
-	DefaultInt       int             `json:"defaultInt" default:"10.0"` // MATCH /field's type and default value's type mismatch/
-	DefaultInt2      int             `json:"defaultInt" default:"10"`
+	URLParam    string `json:"-" path:"url_param" validate:"numeric"`
+	Text        string `json:"text" validate:"max=10"`
+	DefaultInt  int    `json:"defaultInt" default:"10.0"` // MATCH /field's type and default value's type mismatch/
+	DefaultInt2 int    `json:"defaultInt2" default:"10"`
+	// MATCH:12 /unknown option 'inline' in JSON tag/
+	DefaultInt3      int             `json:"defaultInt2,inline" default:"11"` // MATCH /duplicate tag name: 'defaultInt2'/
 	DefaultString    string          `json:"defaultString" default:"foo"`
 	DefaultBool      bool            `json:"defaultBool" default:"trues"` // MATCH /field's type and default value's type mismatch/
-	DefaultBool2     bool            `json:"defaultBool" default:"true"`
-	DefaultBool3     bool            `json:"defaultBool" default:"false"`
+	DefaultBool2     bool            `json:"defaultBool2" default:"true"`
+	DefaultBool3     bool            `json:"defaultBool3" default:"false"`
 	DefaultFloat     float64         `json:"defaultFloat" default:"f10.0"` // MATCH /field's type and default value's type mismatch/
-	DefaultFloat2    float64         `json:"defaultFloat" default:"10.0"`
+	DefaultFloat2    float64         `json:"defaultFloat2" default:"10.0"`
 	MandatoryStruct  mandatoryStruct `json:"mandatoryStruct" required:"trues"` // MATCH /required should be 'true' or 'false'/
-	MandatoryStruct2 mandatoryStruct `json:"mandatoryStruct" required:"true"`
-	MandatoryStruct4 mandatoryStruct `json:"mandatoryStruct" required:"false"`
+	MandatoryStruct2 mandatoryStruct `json:"mandatoryStruct2" required:"true"`
+	MandatoryStruct4 mandatoryStruct `json:"mandatoryStruct4" required:"false"`
 	OptionalStruct   *optionalStruct `json:"optionalStruct,omitempty"`
 	OptionalQuery    string          `json:"-" querystring:"queryfoo"`
 	optionalQuery    string          `json:"-" querystring:"queryfoo"` // MATCH /tag on not-exported field optionalQuery/
 	// No-reg test for bug https://github.com/mgechev/revive/issues/208
-	Tiret			 string          `json:"-,"`
-	BadTiret         string          `json:"other,"` // MATCH /option can not be empty in JSON tag/
+	Tiret    string `json:"-,"`
+	BadTiret string `json:"other,"` // MATCH /option can not be empty in JSON tag/
 }
 
 type RangeAllocation struct {
@@ -57,3 +59,46 @@ type VirtualMachineRelocateSpecDiskLocator struct {
 	DiskBackingInfo BaseVirtualDeviceBackingInfo    `xml:"diskBackingInfo,omitempty,any"`
 	Profile         []BaseVirtualMachineProfileSpec `xml:"profile,omitempty,other"` // MATCH /unknown option 'other' in XML tag/
 }
+
+type TestDuplicatedXMLTags struct {
+	A int `xml:"a"`
+	B int `xml:"a"` // MATCH /duplicate tag name: 'a'/
+	C int `xml:"c"`
+}
+
+type TestDuplicatedBSONTags struct {
+	A int `bson:"b"`
+	B int `bson:"b"` // MATCH /duplicate tag name: 'b'/
+	C int `bson:"c"`
+}
+
+type TestDuplicatedYAMLTags struct {
+	A int `yaml:"b"`
+	B int `yaml:"c"`
+	C int `yaml:"c"` // MATCH /duplicate tag name: 'c'/
+}
+
+type TestDuplicatedProtobufTags struct {
+	A int `protobuf:"varint,name=b"`
+	B int `protobuf:"varint,name=c"`
+	C int `protobuf:"varint,name=c"` // MATCH /duplicate tag name: 'c'/
+}
+
+// test case from
+// sigs.k8s.io/kustomize/api/types/helmchartargs.go
+
+type HelmChartArgs struct {
+	ChartName        string                 `json:"chartName,omitempty" yaml:"chartName,omitempty"`
+	ChartVersion     string                 `json:"chartVersion,omitempty" yaml:"chartVersion,omitempty"`
+	ChartRepoURL     string                 `json:"chartRepoUrl,omitempty" yaml:"chartRepoUrl,omitempty"`
+	ChartHome        string                 `json:"chartHome,omitempty" yaml:"chartHome,omitempty"`
+	ChartRepoName    string                 `json:"chartRepoName,omitempty" yaml:"chartRepoName,omitempty"`
+	HelmBin          string                 `json:"helmBin,omitempty" yaml:"helmBin,omitempty"`
+	HelmHome         string                 `json:"helmHome,omitempty" yaml:"helmHome,omitempty"`
+	Values           string                 `json:"values,omitempty" yaml:"values,omitempty"`
+	ValuesLocal      map[string]interface{} `json:"valuesLocal,omitempty" yaml:"valuesLocal,omitempty"`
+	ValuesMerge      string                 `json:"valuesMerge,omitempty" yaml:"valuesMerge,omitempty"`
+	ReleaseName      string                 `json:"releaseName,omitempty" yaml:"releaseName,omitempty"`
+	ReleaseNamespace string                 `json:"releaseNamespace,omitempty" yaml:"releaseNamespace,omitempty"`
+	ExtraArgs        []string               `json:"extraArgs,omitempty" yaml:"extraArgs,omitempty"`
+}