diff --git a/rule/struct-tag.go b/rule/struct-tag.go index 876104d79..6e3e00b5b 100644 --- a/rule/struct-tag.go +++ b/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) diff --git a/testdata/struct-tag.go b/testdata/struct-tag.go index e4aa07446..600895a23 100644 --- a/testdata/struct-tag.go +++ b/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"` +}