struct-tag: support codec struct tag (#1507)

rule/struct_tag.go

@@ -23,6 +23,7 @@ type tagKey string
 const (
 	keyASN1         tagKey = "asn1"
 	keyBSON         tagKey = "bson"
+	keyCodec        tagKey = "codec"
 	keyDatastore    tagKey = "datastore"
 	keyDefault      tagKey = "default"
 	keyJSON         tagKey = "json"
@@ -38,11 +39,12 @@ const (
 	keyYAML         tagKey = "yaml"
 )
 
-type tagChecker func(checkCtx *checkContext, tag *structtag.Tag, fieldType ast.Expr) (message string, succeeded bool)
+type tagChecker func(checkCtx *checkContext, tag *structtag.Tag, field *ast.Field) (message string, succeeded bool)
 
 var tagCheckers = map[tagKey]tagChecker{
 	keyASN1:         checkASN1Tag,
 	keyBSON:         checkBSONTag,
+	keyCodec:        checkCodecTag,
 	keyDatastore:    checkDatastoreTag,
 	keyDefault:      checkDefaultTag,
 	keyJSON:         checkJSONTag,
@@ -62,6 +64,7 @@ type checkContext struct {
 	userDefined    map[tagKey][]string // map: key -> []option
 	usedTagNbr     map[int]bool        // list of used tag numbers
 	usedTagName    map[string]bool     // list of used tag keys
+	commonOptions  map[string]bool     // list of options defined for all fields
 	isAtLeastGo124 bool
 }
 
@@ -74,6 +77,23 @@ func (checkCtx checkContext) isUserDefined(key tagKey, opt string) bool {
 	return slices.Contains(options, opt)
 }
 
+func (checkCtx *checkContext) isCommonOption(opt string) bool {
+	if checkCtx.commonOptions == nil {
+		return false
+	}
+
+	_, ok := checkCtx.commonOptions[opt]
+	return ok
+}
+
+func (checkCtx *checkContext) addCommonOption(opt string) {
+	if checkCtx.commonOptions == nil {
+		checkCtx.commonOptions = map[string]bool{}
+	}
+
+	checkCtx.commonOptions[opt] = true
+}
+
 // Configure validates the rule configuration, and configures the rule accordingly.
 //
 // Configuration implements the [lint.ConfigurableRule] interface.
@@ -164,36 +184,66 @@ func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor {
 
 // checkTaggedField checks the tag of the given field.
 // precondition: the field has a tag
-func (w lintStructTagRule) checkTaggedField(checkCtx *checkContext, f *ast.Field) {
+func (w lintStructTagRule) checkTaggedField(checkCtx *checkContext, field *ast.Field) {
-	if len(f.Names) > 0 && !f.Names[0].IsExported() {
+	tags, err := structtag.Parse(strings.Trim(field.Tag.Value, "`"))
-		w.addFailuref(f, "tag on not-exported field %s", f.Names[0].Name)
-	}
-
-	tags, err := structtag.Parse(strings.Trim(f.Tag.Value, "`"))
 	if err != nil || tags == nil {
-		w.addFailuref(f.Tag, "malformed tag")
+		w.addFailuref(field.Tag, "malformed tag")
 		return
 	}
 
+	analyzedTags := map[tagKey]struct{}{}
 	for _, tag := range tags.Tags() {
 		if msg, ok := w.checkTagNameIfNeed(checkCtx, tag); !ok {
-			w.addFailureWithTagKey(f.Tag, msg, tag.Key)
+			w.addFailureWithTagKey(field.Tag, msg, tag.Key)
 		}
 
 		if msg, ok := checkOptionsOnIgnoredField(tag); !ok {
-			w.addFailureWithTagKey(f.Tag, msg, tag.Key)
+			w.addFailureWithTagKey(field.Tag, msg, tag.Key)
 		}
 
-		checker, ok := w.tagCheckers[tagKey(tag.Key)]
+		key := tagKey(tag.Key)
+		checker, ok := w.tagCheckers[key]
 		if !ok {
 			continue // we don't have a checker for the tag
 		}
 
-		msg, ok := checker(checkCtx, tag, f.Type)
+		msg, ok := checker(checkCtx, tag, field)
 		if !ok {
-			w.addFailureWithTagKey(f.Tag, msg, tag.Key)
+			w.addFailureWithTagKey(field.Tag, msg, tag.Key)
+		}
+
+		analyzedTags[key] = struct{}{}
+	}
+
+	if w.shallWarnOnUnexportedField(field.Names, analyzedTags) {
+		w.addFailuref(field, "tag on not-exported field %s", field.Names[0].Name)
+	}
+}
+
+// tagKeyToSpecialField maps tag keys to their "special" meaning struct fields.
+var tagKeyToSpecialField = map[tagKey]string{
+	"codec": structTagCodecSpecialField,
+}
+
+func (lintStructTagRule) shallWarnOnUnexportedField(fieldNames []*ast.Ident, tags map[tagKey]struct{}) bool {
+	if len(fieldNames) != 1 { // only handle the case of single field  name (99.999% of cases)
+		return false
+	}
+
+	if fieldNames[0].IsExported() {
+		return false
+	}
+
+	fieldNameStr := fieldNames[0].Name
+
+	for key := range tags {
+		specialField, ok := tagKeyToSpecialField[key]
+		if ok && specialField == fieldNameStr {
+			return false
 		}
 	}
+
+	return true
 }
 
 func (w lintStructTagRule) checkTagNameIfNeed(checkCtx *checkContext, tag *structtag.Tag) (message string, succeeded bool) {
@@ -204,7 +254,7 @@ func (w lintStructTagRule) checkTagNameIfNeed(checkCtx *checkContext, tag *struc
 
 	key := tagKey(tag.Key)
 	switch key {
-	case keyBSON, keyJSON, keyXML, keyYAML, keyProtobuf, keySpanner:
+	case keyBSON, keyCodec, keyJSON, keyProtobuf, keySpanner, keyXML, keyYAML: // keys that need to check for duplicated tags
 	default:
 		return "", true
 	}
@@ -241,7 +291,8 @@ func (lintStructTagRule) getTagName(tag *structtag.Tag) string {
 	}
 }
 
-func checkASN1Tag(checkCtx *checkContext, tag *structtag.Tag, fieldType ast.Expr) (message string, succeeded bool) {
+func checkASN1Tag(checkCtx *checkContext, tag *structtag.Tag, field *ast.Field) (message string, succeeded bool) {
+	fieldType := field.Type
 	checkList := slices.Concat(tag.Options, []string{tag.Name})
 	for _, opt := range checkList {
 		switch opt {
@@ -282,7 +333,7 @@ func checkCompoundANS1Option(checkCtx *checkContext, opt string, fieldType ast.E
 	return "", true
 }
 
-func checkDatastoreTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkDatastoreTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "flatten", "noindex", "omitempty":
@@ -297,15 +348,15 @@ func checkDatastoreTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (
 	return "", true
 }
 
-func checkDefaultTag(_ *checkContext, tag *structtag.Tag, fieldType ast.Expr) (message string, succeeded bool) {
+func checkDefaultTag(_ *checkContext, tag *structtag.Tag, field *ast.Field) (message string, succeeded bool) {
-	if !typeValueMatch(fieldType, tag.Name) {
+	if !typeValueMatch(field.Type, tag.Name) {
 		return msgTypeMismatch, false
 	}
 
 	return "", true
 }
 
-func checkBSONTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkBSONTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "inline", "minsize", "omitempty":
@@ -320,7 +371,32 @@ func checkBSONTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messa
 	return "", true
 }
 
-func checkJSONTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+const structTagCodecSpecialField = "_struct"
+
+func checkCodecTag(checkCtx *checkContext, tag *structtag.Tag, field *ast.Field) (message string, succeeded bool) {
+	fieldNames := field.Names
+	mustAddToCommonOptions := len(fieldNames) == 1 && fieldNames[0].Name == structTagCodecSpecialField // see https://github.com/mgechev/revive/issues/1477#issuecomment-3191493076
+	for _, opt := range tag.Options {
+		if mustAddToCommonOptions {
+			checkCtx.addCommonOption(opt)
+		} else if checkCtx.isCommonOption(opt) {
+			return fmt.Sprintf("redundant option %q, already set for all fields", opt), false
+		}
+
+		switch opt {
+		case "omitempty", "toarray", "int", "uint", "float", "-", "omitemptyarray":
+		default:
+			if checkCtx.isUserDefined(keyCodec, opt) {
+				continue
+			}
+			return fmt.Sprintf(msgUnknownOption, opt), false
+		}
+	}
+
+	return "", true
+}
+
+func checkJSONTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "omitempty", "string":
@@ -345,7 +421,7 @@ func checkJSONTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messa
 	return "", true
 }
 
-func checkMapstructureTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkMapstructureTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "omitempty", "reminder", "squash":
@@ -360,13 +436,14 @@ func checkMapstructureTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr
 	return "", true
 }
 
-func checkPropertiesTag(_ *checkContext, tag *structtag.Tag, fieldType ast.Expr) (message string, succeeded bool) {
+func checkPropertiesTag(_ *checkContext, tag *structtag.Tag, field *ast.Field) (message string, succeeded bool) {
 	options := tag.Options
 	if len(options) == 0 {
 		return "", true
 	}
 
 	seenOptions := map[string]bool{}
+	fieldType := field.Type
 	for _, opt := range options {
 		msg, ok := fmt.Sprintf("unknown or malformed option %q", opt), false
 		if key, value, found := strings.Cut(opt, "="); found {
@@ -405,7 +482,7 @@ func checkCompoundPropertiesOption(key, value string, fieldType ast.Expr, seenOp
 	return "", true
 }
 
-func checkProtobufTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkProtobufTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	// check name
 	switch tag.Name {
 	case "bytes", "fixed32", "fixed64", "group", "varint", "zigzag32", "zigzag64":
@@ -458,7 +535,7 @@ func checkProtobufOptions(checkCtx *checkContext, options []string) (message str
 	return "", true
 }
 
-func checkRequiredTag(_ *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkRequiredTag(_ *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	switch tag.Name {
 	case "true", "false":
 		return "", true
@@ -467,7 +544,7 @@ func checkRequiredTag(_ *checkContext, tag *structtag.Tag, _ ast.Expr) (message
 	}
 }
 
-func checkTOMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkTOMLTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "omitempty":
@@ -482,7 +559,7 @@ func checkTOMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messa
 	return "", true
 }
 
-func checkURLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkURLTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	var delimiter = ""
 	for _, opt := range tag.Options {
 		switch opt {
@@ -505,7 +582,7 @@ func checkURLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messag
 	return "", true
 }
 
-func checkValidateTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkValidateTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	previousOption := ""
 	seenKeysOption := false
 	options := append([]string{tag.Name}, tag.Options...)
@@ -534,7 +611,7 @@ func checkValidateTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (m
 	return "", true
 }
 
-func checkXMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkXMLTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "any", "attr", "cdata", "chardata", "comment", "innerxml", "omitempty", "typeattr":
@@ -549,7 +626,7 @@ func checkXMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messag
 	return "", true
 }
 
-func checkYAMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkYAMLTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		switch opt {
 		case "flow", "inline", "omitempty":
@@ -564,7 +641,7 @@ func checkYAMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messa
 	return "", true
 }
 
-func checkSpannerTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
+func checkSpannerTag(checkCtx *checkContext, tag *structtag.Tag, _ *ast.Field) (message string, succeeded bool) {
 	for _, opt := range tag.Options {
 		if !checkCtx.isUserDefined(keySpanner, opt) {
 			return fmt.Sprintf(msgUnknownOption, opt), false

test/struct_tag_test.go

@@ -22,6 +22,7 @@ func TestStructTagWithUserOptions(t *testing.T) {
 			"validate,displayName",
 			"toml,unknown",
 			"spanner,mySpannerOption",
+			"codec,myCodecOption",
 		},
 	})
 }

testdata/struct_tag.go

@@ -196,3 +196,21 @@ type SpannerUser struct {
 	CreatedAt time.Time `spanner:"created_at"`
 	UpdatedAt time.Time `spanner:"updated_at,unknown"` // MATCH /unknown option "unknown" in spanner tag/
 }
+
+type Codec struct {
+	_something struct{} `codec:",omitempty,int"` // MATCH /tag on not-exported field _something/
+	_struct    struct{} `codec:",omitempty,int"` // do not match, _struct has special meaning for codec tag
+	Field1     string   `codec:"-"`
+	Field2     int      `codec:"myName"`
+	Field3     int32    `codec:",omitempty"` // MATCH /redundant option "omitempty", already set for all fields in codec tag/
+	Field4     bool     `codec:"f4,int"`     // MATCH /redundant option "int", already set for all fields in codec tag/
+	field5     bool     // unexported, so skipped
+	Anon
+}
+
+type TestDuplicatedCodecTags struct {
+	_struct struct{} `json:",omitempty"` // MATCH /tag on not-exported field _struct/
+	A       int      `codec:"field_a"`
+	B       int      `codec:"field_a"` // MATCH /duplicated tag name "field_a" in codec tag/
+	C       int      `codec:"field_c"`
+}

testdata/struct_tag_user_options.go

@@ -97,3 +97,8 @@ type uselessOptions struct {
 	// MATCH:83 /unknown option "" in xml tag/
 	// MATCH:86 /unknown option "" in yaml tag/
 }
+
+type CodecUserOptions struct {
+	ID   int    `codec:"user_id,myCodecOption"`
+	Name string `codec:"full_name,unknownOption"` // MATCH /unknown option "unknownOption" in codec tag/
+}