Skip to content

Commit

Permalink
fix issue #520 (#802)
Browse files Browse the repository at this point in the history
  • Loading branch information
chavacava authored Mar 14, 2023
1 parent d7bedbd commit 6fd2c76
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 26 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
11 changes: 10 additions & 1 deletion RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
123 changes: 99 additions & 24 deletions rule/struct-tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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=")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -201,26 +243,33 @@ 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
}
}

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

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":
Expand All @@ -230,30 +279,39 @@ 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
}
}

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

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
}
}
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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
}
7 changes: 7 additions & 0 deletions test/struct-tag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@ package test
import (
"testing"

"github.com/mgechev/revive/lint"
"github.com/mgechev/revive/rule"
)

// TestStructTag tests struct-tag rule
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"},
})
}
15 changes: 15 additions & 0 deletions testdata/struct-tag-useroptions.go
Original file line number Diff line number Diff line change
@@ -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"`
}

0 comments on commit 6fd2c76

Please sign in to comment.