Repository: mgechev/revive (go)

Issue:
Default settings do not match recommended settings
**Describe the bug**

The default settings...

https://github.com/mgechev/revive/blob/master/defaults.toml

... do not match the recommended settings:

https://github.com/mgechev/revive#recommended-configuration

As a user, I find this confusing.

**To Reproduce**
Follow the links above and compare the defaults with the recommendations.

**Expected behavior**
The defaults should match the recommendations.

**NOTE**
I just [fixed actual bugs in my code by enabling unused-parameters warnings](https://github.com/walles/moar/commit/7bd5d3e0990e975ae9aa342dd237a3f8b9ebc58c#diff-f5d6cd5ff2d53fa27c0d76a18c3613153a8b43b492bc07e1ae12671c6e94ddba), so I totally see why this is recommended.

Pull request:
Update default rule set to match recommendations
Before this change, Revive's defaults did not match the recommended configuration:

https://github.com/mgechev/revive#recommended-configuration

I found that to be confusing.

With this change in place, Revive now defaults to following its own recommendations.

Closes #800 

And as a (side?) note, I just [fixed actual bugs in my code by enabling unused-parameters warnings](https://github.com/walles/moar/commit/7bd5d3e0990e975ae9aa342dd237a3f8b9ebc58c#diff-f5d6cd5ff2d53fa27c0d76a18c3613153a8b43b492bc07e1ae12671c6e94ddba). Woho for unused parameters warnings!

Patch:
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/config/config.go b/config/config.go
index d6b4f41..4fbab0e 100644
--- a/config/config.go
+++ b/config/config.go
@@ -31,21 +31,23 @@ var defaultRules = []lint.Rule{
 	&rule.TimeNamingRule{},
 	&rule.ContextKeysType{},
 	&rule.ContextAsArgumentRule{},
+	&rule.IfReturnRule{},
+	&rule.EmptyBlockRule{},
+	&rule.SuperfluousElseRule{},
+	&rule.UnusedParamRule{},
+	&rule.UnreachableCodeRule{},
+	&rule.RedefinesBuiltinIDRule{},
 }
 
 var allRules = append([]lint.Rule{
 	&rule.ArgumentsLimitRule{},
 	&rule.CyclomaticRule{},
 	&rule.FileHeaderRule{},
-	&rule.EmptyBlockRule{},
-	&rule.SuperfluousElseRule{},
 	&rule.ConfusingNamingRule{},
 	&rule.GetReturnRule{},
 	&rule.ModifiesParamRule{},
 	&rule.ConfusingResultsRule{},
 	&rule.DeepExitRule{},
-	&rule.UnusedParamRule{},
-	&rule.UnreachableCodeRule{},
 	&rule.AddConstantRule{},
 	&rule.FlagParamRule{},
 	&rule.UnnecessaryStmtRule{},
@@ -53,7 +55,6 @@ var allRules = append([]lint.Rule{
 	&rule.ModifiesValRecRule{},
 	&rule.ConstantLogicalExprRule{},
 	&rule.BoolLiteralRule{},
-	&rule.RedefinesBuiltinIDRule{},
 	&rule.ImportsBlacklistRule{},
 	&rule.FunctionResultsLimitRule{},
 	&rule.MaxPublicStructsRule{},
@@ -79,7 +80,6 @@ var allRules = append([]lint.Rule{
 	&rule.UnexportedNamingRule{},
 	&rule.FunctionLength{},
 	&rule.NestedStructs{},
-	&rule.IfReturnRule{},
 	&rule.UselessBreak{},
 	&rule.TimeEqualRule{},
 	&rule.BannedCharsRule{},
diff --git a/defaults.toml b/defaults.toml
index 2e99473..f9e2405 100644
--- a/defaults.toml
+++ b/defaults.toml
@@ -23,3 +23,8 @@ warningCode = 0
 [rule.unexported-return]
 [rule.indent-error-flow]
 [rule.errorf]
+[rule.empty-block]
+[rule.superfluous-else]
+[rule.unused-parameter]
+[rule.unreachable-code]
+[rule.redefines-builtin-id]
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
+}


Test patch:
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"`
+}