Skip to content

Commit

Permalink
imporve var-naming - add upperCaseConst option to allow UPPER_CASED…
Browse files Browse the repository at this point in the history
… constants #851 (#852)

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

Co-authored-by: chavacava <[email protected]>
  • Loading branch information
comdiv and chavacava authored Jul 31, 2023
1 parent 7bd6668 commit 8941d19
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 33 deletions.
8 changes: 6 additions & 2 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -670,13 +670,17 @@ _Configuration_: N/A

_Description_: This rule warns when [initialism](https:/golang/go/wiki/CodeReviewComments#initialisms), [variable](https:/golang/go/wiki/CodeReviewComments#variable-names) or [package](https:/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:/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:/golang/lint/issues/89))

Example:

```toml
[rule.var-naming]
arguments = [["ID"], ["VM"]]
arguments = [["ID"], ["VM"], [{upperCaseConst=true}]]
```

## var-declaration
Expand Down
84 changes: 53 additions & 31 deletions rule/var-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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()
Expand All @@ -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.
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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:
Expand All @@ -181,31 +210,24 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor {
// not exported in the Go API.
// See https:/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)
}
}
}
Expand All @@ -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")
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/var-naming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}}},
})
}
9 changes: 9 additions & 0 deletions testdata/var-naming_upperCaseConst-false.go
Original file line number Diff line number Diff line change
@@ -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/
)
10 changes: 10 additions & 0 deletions testdata/var-naming_upperCaseConst-true.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// should pass if upperCaseConst = true

package fixtures

const SOME_CONST_2 = 2

const (
SOME_CONST_3 = 3
VER = 0
)

0 comments on commit 8941d19

Please sign in to comment.