From 6fd2c76c7d2beefd5caa830479b8d42ced272d84 Mon Sep 17 00:00:00 2001 From: chavacava Date: Wed, 15 Mar 2023 00:16:12 +0100 Subject: [PATCH] fix issue #520 (#802) --- README.md | 2 +- RULES_DESCRIPTIONS.md | 11 ++- rule/struct-tag.go | 123 +++++++++++++++++++++++------ test/struct-tag_test.go | 7 ++ testdata/struct-tag-useroptions.go | 15 ++++ 5 files changed, 132 insertions(+), 26 deletions(-) create mode 100644 testdata/struct-tag-useroptions.go diff --git a/README.md b/README.md index 9bbc0b0..413b06c 100644 --- a/README.md +++ b/README.md @@ -465,7 +465,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`add-constant`](./RULES_DESCRIPTIONS.md#add-constant) | map | Suggests using constant for magic numbers and string literals | no | no | | [`flag-parameter`](./RULES_DESCRIPTIONS.md#flag-parameter) | n/a | Warns on boolean parameters that create a control coupling | no | no | | [`unnecessary-stmt`](./RULES_DESCRIPTIONS.md#unnecessary-stmt) | n/a | Suggests removing or simplifying unnecessary statements | no | no | -| [`struct-tag`](./RULES_DESCRIPTIONS.md#struct-tag) | n/a | Checks common struct tags like `json`,`xml`,`yaml` | no | no | +| [`struct-tag`](./RULES_DESCRIPTIONS.md#struct-tag) | []string | Checks common struct tags like `json`,`xml`,`yaml` | no | no | | [`modifies-value-receiver`](./RULES_DESCRIPTIONS.md#modifies-value-receiver) | n/a | Warns on assignments to value-passed method receivers | no | yes | | [`constant-logical-expr`](./RULES_DESCRIPTIONS.md#constant-logical-expr) | n/a | Warns on constant logical expressions | no | no | | [`bool-literal-in-expr`](./RULES_DESCRIPTIONS.md#bool-literal-in-expr)| n/a | Suggests removing Boolean literals from logic expressions | no | no | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 3865f86..e37640b 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -608,7 +608,16 @@ Example: _Description_: Struct tags are not checked at compile time. This rule, checks and warns if it finds errors in common struct tags types like: asn1, default, json, protobuf, xml, yaml. -_Configuration_: N/A +_Configuration_: (optional) list of user defined options. + +Example: +To accept the `inline` option in JSON tags (and `outline` and `gnu` in BSON tags) you must provide the following configuration + +```toml +[rule.struct-tag] + arguments = ["json,inline","bson,outline,gnu"] +``` + ## superfluous-else diff --git a/rule/struct-tag.go b/rule/struct-tag.go index 3accf58..45308d3 100644 --- a/rule/struct-tag.go +++ b/rule/struct-tag.go @@ -5,23 +5,55 @@ import ( "go/ast" "strconv" "strings" + "sync" "github.com/fatih/structtag" "github.com/mgechev/revive/lint" ) // StructTagRule lints struct tags. -type StructTagRule struct{} +type StructTagRule struct { + userDefined map[string][]string // map: key -> []option + sync.Mutex +} + +func (r *StructTagRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() + if r.userDefined == nil && len(arguments) > 0 { + checkNumberOfArguments(1, arguments, r.Name()) + r.userDefined = make(map[string][]string, len(arguments)) + for _, arg := range arguments { + item, ok := arg.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string, got %v (of type %T)", r.Name(), arg, arg)) + } + parts := strings.Split(item, ",") + if len(parts) < 2 { + panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string of the form key[,option]+, got %s", r.Name(), item)) + } + key := strings.TrimSpace(parts[0]) + for i := 1; i < len(parts); i++ { + option := strings.TrimSpace(parts[i]) + r.userDefined[key] = append(r.userDefined[key], option) + } + } + } +} // Apply applies the rule to given file. -func (*StructTagRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { - var failures []lint.Failure +func (r *StructTagRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.configure(args) + var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) } - w := lintStructTagRule{onFailure: onFailure} + w := lintStructTagRule{ + onFailure: onFailure, + userDefined: r.userDefined, + } ast.Walk(w, file.AST) @@ -35,8 +67,9 @@ func (*StructTagRule) Name() string { type lintStructTagRule struct { onFailure func(lint.Failure) - usedTagNbr map[int]bool // list of used tag numbers - usedTagName map[string]bool // list of used tag keys + userDefined map[string][]string // map: key -> []option + usedTagNbr map[int]bool // list of used tag numbers + usedTagName map[string]bool // list of used tag keys } func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor { @@ -57,17 +90,26 @@ func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor { return w } +const keyASN1 = "asn1" +const keyBSON = "bson" +const keyDefault = "default" +const keyJSON = "json" +const keyProtobuf = "protobuf" +const keyRequired = "required" +const keyXML = "xml" +const keyYAML = "yaml" + 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" + needsToCheckTagName := tag.Key == keyBSON || + tag.Key == keyJSON || + tag.Key == keyXML || + tag.Key == keyYAML || + tag.Key == keyProtobuf if !needsToCheckTagName { return "", true @@ -92,7 +134,7 @@ func (w lintStructTagRule) checkTagNameIfNeed(tag *structtag.Tag) (string, bool) func (lintStructTagRule) getTagName(tag *structtag.Tag) string { switch tag.Key { - case "protobuf": + case keyProtobuf: for _, option := range tag.Options { if strings.HasPrefix(option, "name=") { return strings.TrimLeft(option, "name=") @@ -123,40 +165,40 @@ func (w lintStructTagRule) checkTaggedField(f *ast.Field) { } switch key := tag.Key; key { - case "asn1": + case keyASN1: msg, ok := w.checkASN1Tag(f.Type, tag) if !ok { w.addFailure(f.Tag, msg) } - case "bson": + case keyBSON: msg, ok := w.checkBSONTag(tag.Options) if !ok { w.addFailure(f.Tag, msg) } - case "default": + case keyDefault: if !w.typeValueMatch(f.Type, tag.Name) { w.addFailure(f.Tag, "field's type and default value's type mismatch") } - case "json": + case keyJSON: msg, ok := w.checkJSONTag(tag.Name, tag.Options) if !ok { w.addFailure(f.Tag, msg) } - case "protobuf": + case keyProtobuf: msg, ok := w.checkProtobufTag(tag) if !ok { w.addFailure(f.Tag, msg) } - case "required": + case keyRequired: if tag.Name != "true" && tag.Name != "false" { w.addFailure(f.Tag, "required should be 'true' or 'false'") } - case "xml": + case keyXML: msg, ok := w.checkXMLTag(tag.Options) if !ok { w.addFailure(f.Tag, msg) } - case "yaml": + case keyYAML: msg, ok := w.checkYAMLTag(tag.Options) if !ok { w.addFailure(f.Tag, msg) @@ -201,6 +243,10 @@ func (w lintStructTagRule) checkASN1Tag(t ast.Expr, tag *structtag.Tag) (string, continue } + if w.isUserDefined(keyASN1, opt) { + continue + } + return fmt.Sprintf("unknown option '%s' in ASN1 tag", opt), false } } @@ -208,11 +254,14 @@ func (w lintStructTagRule) checkASN1Tag(t ast.Expr, tag *structtag.Tag) (string, return "", true } -func (lintStructTagRule) checkBSONTag(options []string) (string, bool) { +func (w lintStructTagRule) checkBSONTag(options []string) (string, bool) { for _, opt := range options { switch opt { case "inline", "minsize", "omitempty": default: + if w.isUserDefined(keyBSON, opt) { + continue + } return fmt.Sprintf("unknown option '%s' in BSON tag", opt), false } } @@ -220,7 +269,7 @@ func (lintStructTagRule) checkBSONTag(options []string) (string, bool) { return "", true } -func (lintStructTagRule) checkJSONTag(name string, options []string) (string, bool) { +func (w lintStructTagRule) checkJSONTag(name string, options []string) (string, bool) { for _, opt := range options { switch opt { case "omitempty", "string": @@ -230,6 +279,9 @@ func (lintStructTagRule) checkJSONTag(name string, options []string) (string, bo return "option can not be empty in JSON tag", false } default: + if w.isUserDefined(keyJSON, opt) { + continue + } return fmt.Sprintf("unknown option '%s' in JSON tag", opt), false } } @@ -237,11 +289,14 @@ func (lintStructTagRule) checkJSONTag(name string, options []string) (string, bo return "", true } -func (lintStructTagRule) checkXMLTag(options []string) (string, bool) { +func (w lintStructTagRule) checkXMLTag(options []string) (string, bool) { for _, opt := range options { switch opt { case "any", "attr", "cdata", "chardata", "comment", "innerxml", "omitempty", "typeattr": default: + if w.isUserDefined(keyXML, opt) { + continue + } return fmt.Sprintf("unknown option '%s' in XML tag", opt), false } } @@ -249,11 +304,14 @@ func (lintStructTagRule) checkXMLTag(options []string) (string, bool) { return "", true } -func (lintStructTagRule) checkYAMLTag(options []string) (string, bool) { +func (w lintStructTagRule) checkYAMLTag(options []string) (string, bool) { for _, opt := range options { switch opt { case "flow", "inline", "omitempty": default: + if w.isUserDefined(keyYAML, opt) { + continue + } return fmt.Sprintf("unknown option '%s' in YAML tag", opt), false } } @@ -330,6 +388,9 @@ func (w lintStructTagRule) checkProtobufTag(tag *structtag.Tag) (string, bool) { case "name", "json": // do nothing default: + if w.isUserDefined(keyProtobuf, k) { + continue + } return fmt.Sprintf("unknown option '%s' in protobuf tag", k), false } } @@ -344,3 +405,17 @@ func (w lintStructTagRule) addFailure(n ast.Node, msg string) { Confidence: 1, }) } + +func (w lintStructTagRule) isUserDefined(key, opt string) bool { + if w.userDefined == nil { + return false + } + + options := w.userDefined[key] + for _, o := range options { + if opt == o { + return true + } + } + return false +} diff --git a/test/struct-tag_test.go b/test/struct-tag_test.go index d8b2a0a..3d677a2 100644 --- a/test/struct-tag_test.go +++ b/test/struct-tag_test.go @@ -3,6 +3,7 @@ package test import ( "testing" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) @@ -10,3 +11,9 @@ import ( func TestStructTag(t *testing.T) { testRule(t, "struct-tag", &rule.StructTagRule{}) } + +func TestStructTagWithUserOptions(t *testing.T) { + testRule(t, "struct-tag-useroptions", &rule.StructTagRule{}, &lint.RuleConfig{ + Arguments: []interface{}{"json,inline,outline", "bson,gnu"}, + }) +} diff --git a/testdata/struct-tag-useroptions.go b/testdata/struct-tag-useroptions.go new file mode 100644 index 0000000..0b48c4a --- /dev/null +++ b/testdata/struct-tag-useroptions.go @@ -0,0 +1,15 @@ +package fixtures + +type RangeAllocation struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + Range string `json:"range,outline"` + Data []byte `json:"data,flow"` // MATCH /unknown option 'flow' in JSON tag/ +} + +type RangeAllocation struct { + metav1.TypeMeta `bson:",minsize,gnu"` + metav1.ObjectMeta `bson:"metadata,omitempty"` + Range string `bson:"range,flow"` // MATCH /unknown option 'flow' in BSON tag/ + Data []byte `bson:"data,inline"` +}