struct-tag (new rule) (#47)

Gopkg.toml

@@ -28,6 +28,10 @@
   name = "github.com/fatih/color"
   version = "1.5.0"
 
+[[constraint]]
+  branch = "master"
+  name = "github.com/fatih/structtag"
+
 [[constraint]]
   branch = "master"
   name = "github.com/olekukonko/tablewriter"

README.md

@@ -262,6 +262,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | `add-constant`        |  map   | Suggests using constant for magic numbers and string literals    |    no    |  no   |
 | `flag-parameter`      |  n/a   | Warns on boolean parameters that create a control coupling       |    no    |  no   |
 | `unnecessary-stmt`    |  n/a   | Suggests removing or simplifying unnecessary statements          |    no    |  no   |
+| `struct-tag`          |  n/a   | Checks common struct tags like `json`,`xml`,`yaml`               |    no    |  no   |
 
 ## Available Formatters
 

config.go

@@ -59,6 +59,7 @@ var allRules = append([]lint.Rule{
 	&rule.AddConstantRule{},
 	&rule.FlagParamRule{},
 	&rule.UnnecessaryStmtRule{},
+	&rule.StructTagRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

fixtures/struct-tag.go

@@ -0,0 +1,37 @@
+package fixtures
+
+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"`
+	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"`
+	DefaultFloat     float64         `json:"defaultFloat" default:"f10.0"` // MATCH /field's type and default value's type mismatch/
+	DefaultFloat2    float64         `json:"defaultFloat" 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"`
+	OptionalStruct   *optionalStruct `json:"optionalStruct,omitempty"`
+	OptionalQuery    string          `json:"-" querystring:"queryfoo"`
+}
+
+type RangeAllocation struct {
+	metav1.TypeMeta   `json:",inline"` // MATCH /unknown option 'inline' in JSON tag/
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+	Range             string `json:"range,flow"`  // MATCH /unknown option 'flow' in JSON tag/
+	Data              []byte `json:"data,inline"` // MATCH /unknown option 'inline' in JSON tag/
+}
+
+type VirtualMachineRelocateSpecDiskLocator struct {
+	DynamicData
+
+	DiskId          int32                           `xml:"diskId,attr,cdata"`
+	Datastore       ManagedObjectReference          `xml:"datastore,chardata,innerxml"`
+	DiskMoveType    string                          `xml:"diskMoveType,omitempty,comment"`
+	DiskBackingInfo BaseVirtualDeviceBackingInfo    `xml:"diskBackingInfo,omitempty,any"`
+	Profile         []BaseVirtualMachineProfileSpec `xml:"profile,omitempty,typeattr"` // MATCH /unknown option 'typeattr' in XML tag/
+}

rule/struct-tag.go

@@ -0,0 +1,167 @@
+package rule
+
+import (
+	"fmt"
+	"github.com/fatih/structtag"
+	"github.com/mgechev/revive/lint"
+	"go/ast"
+	"strconv"
+	"strings"
+)
+
+// StructTagRule lints struct tags.
+type StructTagRule struct{}
+
+// Apply applies the rule to given file.
+func (r *StructTagRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := lintStructTagRule{onFailure: onFailure}
+
+	ast.Walk(w, file.AST)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *StructTagRule) Name() string {
+	return "struct-tag"
+}
+
+type lintStructTagRule struct {
+	onFailure func(lint.Failure)
+}
+
+func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor {
+	switch n := node.(type) {
+	case *ast.StructType:
+		if n.Fields == nil || n.Fields.NumFields() < 1 {
+			return nil // skip empty structs
+		}
+
+		for _, f := range n.Fields.List {
+			if f.Tag != nil {
+				w.checkTaggedField(f)
+			}
+		}
+	}
+
+	return w
+
+}
+
+// checkTaggedField checks the tag of the given field.
+// precondition: the field has a tag
+func (w lintStructTagRule) checkTaggedField(f *ast.Field) {
+	tags, err := structtag.Parse(strings.Trim(f.Tag.Value, "`"))
+	if err != nil || tags == nil {
+		return
+	}
+
+	for _, tag := range tags.Tags() {
+		switch key := tag.Key; key {
+		case "asn1":
+			// Not implemented yet
+		case "default":
+			if !w.typeValueMatch(f.Type, tag.Name) {
+				w.addFailure(f.Tag, "field's type and default value's type mismatch")
+			}
+		case "json":
+			msg, ok := w.checkJSONTag(tag.Options)
+			if !ok {
+				w.addFailure(f.Tag, msg)
+			}
+		case "protobuf":
+			// Not implemented yet
+		case "required":
+			if tag.Name != "true" && tag.Name != "false" {
+				w.addFailure(f.Tag, "required should be 'true' or 'false'")
+			}
+		case "xml":
+			msg, ok := w.checkXMLTag(tag.Options)
+			if !ok {
+				w.addFailure(f.Tag, msg)
+			}
+		case "yaml":
+			msg, ok := w.checkYAMLTag(tag.Options)
+			if !ok {
+				w.addFailure(f.Tag, msg)
+			}
+		default:
+			// unknown key
+		}
+	}
+}
+
+func (w lintStructTagRule) checkJSONTag(options []string) (string, bool) {
+	for _, opt := range options {
+		switch opt {
+		case "omitempty", "string":
+		default:
+			return fmt.Sprintf("unknown option '%s' in JSON tag", opt), false
+		}
+	}
+
+	return "", true
+}
+
+func (w lintStructTagRule) checkXMLTag(options []string) (string, bool) {
+	for _, opt := range options {
+		switch opt {
+		case "attr", "cdata", "chardata", "innerxml", "comment", "any", "omitempty":
+		default:
+			return fmt.Sprintf("unknown option '%s' in XML tag", opt), false
+		}
+	}
+
+	return "", true
+}
+
+func (w lintStructTagRule) checkYAMLTag(options []string) (string, bool) {
+	for _, opt := range options {
+		switch opt {
+		case "flow", "inline", "omitempty":
+		default:
+			return fmt.Sprintf("unknown option '%s' in YAML tag", opt), false
+		}
+	}
+
+	return "", true
+}
+
+func (w lintStructTagRule) typeValueMatch(t ast.Expr, val string) bool {
+	tID, ok := t.(*ast.Ident)
+	if !ok {
+		return true
+	}
+
+	typeMatches := true
+	switch tID.Name {
+	case "bool":
+		typeMatches = val == "true" || val == "false"
+	case "float64":
+		_, err := strconv.ParseFloat(val, 64)
+		typeMatches = err == nil
+	case "int":
+		_, err := strconv.ParseInt(val, 10, 64)
+		typeMatches = err == nil
+	case "string":
+	case "nil":
+	default:
+		// unchecked type
+	}
+
+	return typeMatches
+}
+
+func (w lintStructTagRule) addFailure(n ast.Node, msg string) {
+	w.onFailure(lint.Failure{
+		Node:       n,
+		Failure:    msg,
+		Confidence: 1,
+	})
+}

test/struct-tag_test.go

@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// TestStructTag tests struct-tag rule
+func TestStructTag(t *testing.T) {
+	testRule(t, "struct-tag", &rule.StructTagRule{})
+}