diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 4b48b526d..1709188dd 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -670,13 +670,17 @@ _Configuration_: N/A _Description_: This rule warns when [initialism](https://github.com/golang/go/wiki/CodeReviewComments#initialisms), [variable](https://github.com/golang/go/wiki/CodeReviewComments#variable-names) or [package](https://github.com/golang/go/wiki/CodeReviewComments#package-names) naming conventions are not followed. -_Configuration_: This rule accepts two slices of strings, a whitelist and a blacklist of initialisms. By default, the rule behaves exactly as the alternative in `golint` but optionally, you can relax it (see [golint/lint/issues/89](https://github.com/golang/lint/issues/89)) +_Configuration_: This rule accepts two slices of strings and one optional slice with single map with named parameters. +(it's due to TOML hasn't "slice of any" and we keep backward compatibility with previous config version) +First slice is a whitelist and second one is a blacklist of initialisms. +In map, you can add "upperCaseConst=true" parameter to allow `UPPER_CASE` for `const` +By default, the rule behaves exactly as the alternative in `golint` but optionally, you can relax it (see [golint/lint/issues/89](https://github.com/golang/lint/issues/89)) Example: ```toml [rule.var-naming] - arguments = [["ID"], ["VM"]] + arguments = [["ID"], ["VM"], [{upperCaseConst=true}]] ``` ## var-declaration diff --git a/rule/var-naming.go b/rule/var-naming.go index fa4a18864..2aa5ae2fd 100644 --- a/rule/var-naming.go +++ b/rule/var-naming.go @@ -13,11 +13,15 @@ import ( var anyCapsRE = regexp.MustCompile(`[A-Z]`) +// regexp for constant names like `SOME_CONST`, `SOME_CONST_2`, `X123_3` (#851) +var upperCaseConstRE = regexp.MustCompile(`^[A-Z][A-Z\d]*(_[A-Z\d]+)*$`) + // VarNamingRule lints given else constructs. type VarNamingRule struct { - configured bool - whitelist []string - blacklist []string + configured bool + whitelist []string + blacklist []string + upperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants sync.Mutex } @@ -31,6 +35,23 @@ func (r *VarNamingRule) configure(arguments lint.Arguments) { if len(arguments) >= 2 { r.blacklist = getList(arguments[1], "blacklist") } + + if len(arguments) >= 3 { + // not pretty code because should keep compatibility with TOML (no mixed array types) and new map parameters + thirdArgument := arguments[2] + asSlice, ok := thirdArgument.([]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, got %T", "options", arguments[2])) + } + if len(asSlice) != 1 { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, but %d", "options", len(asSlice))) + } + args, ok := asSlice[0].(map[string]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, with map, but %T", "options", asSlice[0])) + } + r.upperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true" + } r.configured = true } r.Unlock() @@ -52,6 +73,7 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint. onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, + upperCaseConst: r.upperCaseConst, } // Package names need slightly different handling than other names. @@ -82,18 +104,18 @@ func (*VarNamingRule) Name() string { return "var-naming" } -func checkList(fl *ast.FieldList, thing string, w *lintNames) { +func (w *lintNames) checkList(fl *ast.FieldList, thing string) { if fl == nil { return } for _, f := range fl.List { for _, id := range f.Names { - check(id, thing, w) + w.check(id, thing) } } } -func check(id *ast.Ident, thing string, w *lintNames) { +func (w *lintNames) check(id *ast.Ident, thing string) { if id.Name == "_" { return } @@ -101,6 +123,12 @@ func check(id *ast.Ident, thing string, w *lintNames) { return } + // #851 upperCaseConst support + // if it's const + if thing == token.CONST.String() && w.upperCaseConst && upperCaseConstRE.Match([]byte(id.Name)) { + return + } + // Handle two common styles from other languages that don't belong in Go. if len(id.Name) >= 5 && allCapsRE.MatchString(id.Name) && strings.Contains(id.Name, "_") { w.onFailure(lint.Failure{ @@ -144,11 +172,12 @@ func check(id *ast.Ident, thing string, w *lintNames) { } type lintNames struct { - file *lint.File - fileAst *ast.File - onFailure func(lint.Failure) - whitelist []string - blacklist []string + file *lint.File + fileAst *ast.File + onFailure func(lint.Failure) + whitelist []string + blacklist []string + upperCaseConst bool } func (w *lintNames) Visit(n ast.Node) ast.Visitor { @@ -159,7 +188,7 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { } for _, exp := range v.Lhs { if id, ok := exp.(*ast.Ident); ok { - check(id, "var", w) + w.check(id, "var") } } case *ast.FuncDecl: @@ -181,31 +210,24 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { // not exported in the Go API. // See https://github.com/golang/lint/issues/144. if ast.IsExported(v.Name.Name) || !isCgoExported(v) { - check(v.Name, thing, w) + w.check(v.Name, thing) } - checkList(v.Type.Params, thing+" parameter", w) - checkList(v.Type.Results, thing+" result", w) + w.checkList(v.Type.Params, thing+" parameter") + w.checkList(v.Type.Results, thing+" result") case *ast.GenDecl: if v.Tok == token.IMPORT { return w } - var thing string - switch v.Tok { - case token.CONST: - thing = "const" - case token.TYPE: - thing = "type" - case token.VAR: - thing = "var" - } + + thing := v.Tok.String() for _, spec := range v.Specs { switch s := spec.(type) { case *ast.TypeSpec: - check(s.Name, thing, w) + w.check(s.Name, thing) case *ast.ValueSpec: for _, id := range s.Names { - check(id, thing, w) + w.check(id, thing) } } } @@ -217,23 +239,23 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { if !ok { // might be an embedded interface name continue } - checkList(ft.Params, "interface method parameter", w) - checkList(ft.Results, "interface method result", w) + w.checkList(ft.Params, "interface method parameter") + w.checkList(ft.Results, "interface method result") } case *ast.RangeStmt: if v.Tok == token.ASSIGN { return w } if id, ok := v.Key.(*ast.Ident); ok { - check(id, "range var", w) + w.check(id, "range var") } if id, ok := v.Value.(*ast.Ident); ok { - check(id, "range var", w) + w.check(id, "range var") } case *ast.StructType: for _, f := range v.Fields.List { for _, id := range f.Names { - check(id, "struct field", w) + w.check(id, "struct field") } } } diff --git a/test/var-naming_test.go b/test/var-naming_test.go index b6ccd411c..56108b056 100644 --- a/test/var-naming_test.go +++ b/test/var-naming_test.go @@ -13,4 +13,9 @@ func TestVarNaming(t *testing.T) { }) testRule(t, "var-naming_test", &rule.VarNamingRule{}, &lint.RuleConfig{}) + + testRule(t, "var-naming_upperCaseConst-false", &rule.VarNamingRule{}, &lint.RuleConfig{}) + testRule(t, "var-naming_upperCaseConst-true", &rule.VarNamingRule{}, &lint.RuleConfig{ + Arguments: []interface{}{[]interface{}{}, []interface{}{}, []interface{}{map[string]interface{}{"upperCaseConst": true}}}, + }) } diff --git a/testdata/var-naming_upperCaseConst-false.go b/testdata/var-naming_upperCaseConst-false.go new file mode 100644 index 000000000..3bb628e32 --- /dev/null +++ b/testdata/var-naming_upperCaseConst-false.go @@ -0,0 +1,9 @@ +// should fail if upperCaseConst = false (by default) #851 + +package fixtures + +const SOME_CONST_2 = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ + +const ( + SOME_CONST_3 = 3 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ +) diff --git a/testdata/var-naming_upperCaseConst-true.go b/testdata/var-naming_upperCaseConst-true.go new file mode 100644 index 000000000..272b708bc --- /dev/null +++ b/testdata/var-naming_upperCaseConst-true.go @@ -0,0 +1,10 @@ +// should pass if upperCaseConst = true + +package fixtures + +const SOME_CONST_2 = 2 + +const ( + SOME_CONST_3 = 3 + VER = 0 +)