imporve var-naming - add upperCaseConst option to allow UPPER_CASED constants #851

RULES_DESCRIPTIONS.md

@@ -669,13 +669,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

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,25 +104,31 @@ 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
 	}
 	if knownNameExceptions[id.Name] {
 		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")
 			}
 		}
 	}

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}}},
+	})
 }

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/
+)

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
+)