Skip to content

Commit

Permalink
Add white & black lists for var-naming
Browse files Browse the repository at this point in the history
This PR introduces a white & black lists of initialisms for the
`var-naming` rule.

Now the rule can be configured with:

```toml
[rule.var-naming]
  arguments = [["ID"], ["VM", "BAR"]]
```

This way, the linter will ignore `customId` but will throw on `customVm` or `customBar`.

Fix #41
  • Loading branch information
mgechev committed Sep 15, 2018
1 parent 8d6642c commit 90f5153
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 50 deletions.
17 changes: 16 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| `exported` | n/a | Naming and commenting conventions on exported symbols. | yes | no |
| `if-return` | n/a | Redundant if when returning an error. | yes | no |
| `increment-decrement` | n/a | Use `i++` and `i--` instead of `i += 1` and `i -= 1`. | yes | no |
| `var-naming` | n/a | Naming rules. | yes | no |
| `var-naming` | whitelist & blacklist of initialisms | Naming rules. | yes | no |
| `package-comments` | n/a | Package commenting conventions. | yes | no |
| `range` | n/a | Prevents redundant variables when iterating over a collection. | yes | no |
| `receiver-naming` | n/a | Conventions around the naming of receivers. | yes | no |
Expand All @@ -270,6 +270,21 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| `bool-literal-in-expr`| n/a | Suggests removing Boolean literals from logic expressions | no | no |
| `redefines-builtin-id`| n/a | Warns on redefinitions of builtin identifiers | no | no |

## Configurable rules

Here you can find how you can configure some of the existing rules:

### `var-naming`

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

```toml
[rule.var-naming]
arguments = [["ID"], ["VM"]]
```

This way, revive will not warn for identifier called `customId` but will warn that `customVm` should be called `customVM`.

## Available Formatters

This section lists all the available formatters and provides a screenshot for each one.
Expand Down
2 changes: 1 addition & 1 deletion defaults.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ warningCode = 0
[rule.time-naming]
[rule.unexported-return]
[rule.indent-error-flow]
[rule.errorf]
[rule.errorf]
7 changes: 7 additions & 0 deletions fixtures/var-naming.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package fixtures

func foo() string {
customId := "result"
customVm := "result" // MATCH /var customVm should be customVM/
return customId
}
14 changes: 12 additions & 2 deletions lint/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

// Name returns a different name if it should be different.
func Name(name string) (should string) {
func Name(name string, whitelist, blacklist []string) (should string) {
// Fast path for simple cases: "_" and all lowercase.
if name == "_" {
return name
Expand Down Expand Up @@ -56,7 +56,17 @@ func Name(name string) (should string) {

// [w,i) is a word.
word := string(runes[w:i])
if u := strings.ToUpper(word); commonInitialisms[u] {
ignoreInitWarnings := map[string]bool{}
for _, i := range whitelist {
ignoreInitWarnings[i] = true
}

extraInits := map[string]bool{}
for _, i := range blacklist {
extraInits[i] = true
}

if u := strings.ToUpper(word); (commonInitialisms[u] || extraInits[u]) && !ignoreInitWarnings[u] {
// Keep consistent case, which is lowercase only at the start.
if w == 0 && unicode.IsLower(runes[w]) {
u = strings.ToLower(u)
Expand Down
3 changes: 2 additions & 1 deletion rule/bool-literal-in-expr.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package rule

import (
"github.com/mgechev/revive/lint"
"go/ast"
"go/token"

"github.com/mgechev/revive/lint"
)

// BoolLiteralRule warns when logic expressions contains Boolean literals.
Expand Down
41 changes: 0 additions & 41 deletions rule/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,47 +61,6 @@ func isCgoExported(f *ast.FuncDecl) bool {
return false
}

var commonInitialisms = map[string]bool{
"ACL": true,
"API": true,
"ASCII": true,
"CPU": true,
"CSS": true,
"DNS": true,
"EOF": true,
"GUID": true,
"HTML": true,
"HTTP": true,
"HTTPS": true,
"ID": true,
"IP": true,
"JSON": true,
"LHS": true,
"QPS": true,
"RAM": true,
"RHS": true,
"RPC": true,
"SLA": true,
"SMTP": true,
"SQL": true,
"SSH": true,
"TCP": true,
"TLS": true,
"TTL": true,
"UDP": true,
"UI": true,
"UID": true,
"UUID": true,
"URI": true,
"URL": true,
"UTF8": true,
"VM": true,
"XML": true,
"XMPP": true,
"XSRF": true,
"XSS": true,
}

var allCapsRE = regexp.MustCompile(`^[A-Z0-9_]+$`)

func isIdent(expr ast.Expr, ident string) bool {
Expand Down
37 changes: 34 additions & 3 deletions rule/var-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,23 @@ type VarNamingRule struct{}
func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
var failures []lint.Failure

var whitelist []string
var blacklist []string

if len(arguments) >= 1 {
whitelist = getList(arguments[0], "whitelist")
}

if len(arguments) >= 2 {
blacklist = getList(arguments[1], "blacklist")
}

fileAst := file.AST
walker := lintNames{
file: file,
fileAst: fileAst,
file: file,
fileAst: fileAst,
whitelist: whitelist,
blacklist: blacklist,
onFailure: func(failure lint.Failure) {
failures = append(failures, failure)
},
Expand Down Expand Up @@ -87,7 +100,7 @@ func check(id *ast.Ident, thing string, w *lintNames) {
})
}

should := lint.Name(id.Name)
should := lint.Name(id.Name, w.whitelist, w.blacklist)
if id.Name == should {
return
}
Expand Down Expand Up @@ -117,6 +130,8 @@ type lintNames struct {
lastGen *ast.GenDecl
genDeclMissingComments map[*ast.GenDecl]bool
onFailure func(lint.Failure)
whitelist []string
blacklist []string
}

func (w *lintNames) Visit(n ast.Node) ast.Visitor {
Expand Down Expand Up @@ -202,3 +217,19 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor {
}
return w
}

func getList(arg interface{}, argName string) []string {
temp, ok := arg.([]interface{})
if !ok {
panic(fmt.Sprintf("Invalid argument to the var-naming rule. Expecting a %s of type slice with initialisms, got %T", argName, arg))
}
var list []string
for _, v := range temp {
if val, ok := v.(string); ok {
list = append(list, val)
} else {
panic(fmt.Sprintf("Invalid %s values of the var-naming rule. Expecting slice of strings but got element of type %T", val, arg))
}
}
return list
}
2 changes: 1 addition & 1 deletion test/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func TestLintName(t *testing.T) {
{"IEEE802_16Bit", "IEEE802_16Bit"},
}
for _, test := range tests {
got := lint.Name(test.name)
got := lint.Name(test.name, nil, nil)
if got != test.want {
t.Errorf("lintName(%q) = %q, want %q", test.name, got, test.want)
}
Expand Down
14 changes: 14 additions & 0 deletions test/var-naming_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package test

import (
"testing"

"github.com/mgechev/revive/lint"
"github.com/mgechev/revive/rule"
)

func TestVarNaming(t *testing.T) {
testRule(t, "var-naming", &rule.VarNamingRule{}, &lint.RuleConfig{
Arguments: []interface{}{[]interface{}{"ID"}, []interface{}{"VM"}},
})
}

0 comments on commit 90f5153

Please sign in to comment.