go/revive
1
0
Fork 0
mirror of https://github.com/mgechev/revive.git synced 2025-10-30 23:37:49 +02:00
Code Issues Releases Activity

var-naming: option to skip initialism name checks (#1415)

Browse Source 
Co-authored-by: akrenzler <akrenzler@paloaltonetworks@com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
This commit is contained in:
Asaf Krenzler
2025-07-11 14:45:38 +03:00
committed by GitHub
parent 594e56503b
commit cc180e221e
11 changed files with 473 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
RULES_DESCRIPTIONS.md
View File

@@ -1363,6 +1363,9 @@ It ignores functions starting with `Example`, `Test`, `Benchmark`, and `Fuzz` in
_Configuration_: This rule accepts two slices of strings and one optional slice containing a single map with named parameters.
(This is because TOML does not support "slice of any," and we maintain backward compatibility with the previous configuration version).
The first slice is an allowlist, and the second one is a blocklist of initialisms.
You can add a boolean parameter `skipInitialismNameChecks` (`skipinitialismnamechecks` or `skip-initialism-name-checks`) to control how names
of functions, variables, consts, and structs handle known initialisms (e.g., JSON, HTTP, etc.) when written in `camelCase`.
When `skipInitialismNameChecks` is set to true, the rule allows names like `readJson`, `HttpMethod` etc.
In the map, you can add a boolean `upperCaseConst` (`uppercaseconst`, `upper-case-const`) parameter to allow `UPPER_CASE` for `const`.
You can also add a boolean `skipPackageNameChecks` (`skippackagenamechecks`, `skip-package-name-checks`) to skip package name checks.
When `skipPackageNameChecks` is false (the default), you can configure `extraBadPackageNames` (`extrabadpackagenames`, `extra-bad-package-names`)
@@ -1373,6 +1376,11 @@ By default, the rule behaves exactly as the alternative in `golint` but optional
Examples:
```toml
[rule.var-naming]
arguments = [[], [], [{ skipInitialismNameChecks = true }]]
```
```toml
[rule.var-naming]
arguments = [["ID"], ["VM"], [{ upperCaseConst = true }]]
@@ -1388,6 +1396,11 @@ arguments = [[], [], [{ skipPackageNameChecks = true }]]
arguments = [[], [], [{ extraBadPackageNames = ["helpers", "models"] }]]
```
```toml
[rule.var-naming]
arguments = [[], [], [{ skip-initialism-name-checks = true }]]
```
```toml
[rule.var-naming]
arguments = [["ID"], ["VM"], [{ upper-case-const = true }]]

135
internal/rule/name.go Normal file
View File

@@ -0,0 +1,135 @@
// Package rule defines utility functions some revive rules can share.
package rule
import (
"strings"
"unicode"
)
// commonInitialisms is a set of common initialisms.
// Only add entries that are highly unlikely to be non-initialisms.
// For instance, "ID" is fine (Freudian code is rare), but "AND" is not.
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,
"IDS": 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,
}
// Name returns a different name of struct, var, const, or function if it should be different.
func Name(name string, allowlist, blocklist []string, skipInitialismNameChecks bool) (should string) {
// Fast path for simple cases: "_" and all lowercase.
if name == "_" {
return name
}
allLower := true
for _, r := range name {
if !unicode.IsLower(r) {
allLower = false
break
}
}
if allLower {
return name
}
// Split camelCase at any lower->upper transition, and split on underscores.
// Check each word for common initialisms.
runes := []rune(name)
w, i := 0, 0 // index of start of word, scan
for i+1 <= len(runes) {
eow := false // whether we hit the end of a word
switch {
case i+1 == len(runes):
eow = true
case runes[i+1] == '_':
// underscore; shift the remainder forward over any run of underscores
eow = true
n := 1
for i+n+1 < len(runes) && runes[i+n+1] == '_' {
n++
}
// Leave at most one underscore if the underscore is between two digits
if i+n+1 < len(runes) && unicode.IsDigit(runes[i]) && unicode.IsDigit(runes[i+n+1]) {
n--
}
copy(runes[i+1:], runes[i+n+1:])
runes = runes[:len(runes)-n]
case unicode.IsLower(runes[i]) && !unicode.IsLower(runes[i+1]):
// lower->non-lower
eow = true
}
i++
if !eow {
continue
}
// [w,i) is a word.
word := string(runes[w:i])
ignoreInitWarnings := map[string]bool{}
for _, i := range allowlist {
ignoreInitWarnings[i] = true
}
extraInits := map[string]bool{}
for _, i := range blocklist {
extraInits[i] = true
}
if u := strings.ToUpper(word); !skipInitialismNameChecks && (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)
}
// Keep lowercase s for IDs
if u == "IDS" {
u = "IDs"
}
// All the common initialisms are ASCII,
// so we can replace the bytes exactly.
copy(runes[w:], []rune(u))
} else if w > 0 && strings.ToLower(word) == word {
// already all lowercase, and not the first word, so uppercase the first character.
runes[w] = unicode.ToUpper(runes[w])
}
w = i
}
return string(runes)
}

195
internal/rule/name_test.go Normal file
View File

@@ -0,0 +1,195 @@
package rule_test
import (
"testing"
"github.com/mgechev/revive/internal/rule"
)
func TestName(t *testing.T) {
tests := []struct {
name string
allowlist []string
blocklist []string
skipInitialismNameChecks bool
want string
}{
{
name: "foo_bar",
want: "fooBar",
},
{
name: "foo_bar_baz",
want: "fooBarBaz",
},
{
name: "Foo_bar",
want: "FooBar",
},
{
name: "foo_WiFi",
want: "fooWiFi",
},
{
name: "id",
want: "id",
},
{
name: "Id",
want: "ID",
},
{
name: "foo_id",
want: "fooID",
},
{
name: "fooId",
want: "fooID",
},
{
name: "fooUid",
want: "fooUID",
},
{
name: "idFoo",
want: "idFoo",
},
{
name: "uidFoo",
want: "uidFoo",
},
{
name: "midIdDle",
want: "midIDDle",
},
{
name: "APIProxy",
want: "APIProxy",
},
{
name: "ApiProxy",
want: "APIProxy",
},
{
name: "apiProxy",
want: "apiProxy",
},
{
name: "_Leading",
want: "_Leading",
},
{
name: "___Leading",
want: "_Leading",
},
{
name: "trailing_",
want: "trailing",
},
{
name: "trailing___",
want: "trailing",
},
{
name: "a_b",
want: "aB",
},
{
name: "a__b",
want: "aB",
},
{
name: "a___b",
want: "aB",
},
{
name: "Rpc1150",
want: "RPC1150",
},
{
name: "case3_1",
want: "case3_1",
},
{
name: "case3__1",
want: "case3_1",
},
{
name: "IEEE802_16bit",
want: "IEEE802_16bit",
},
{
name: "IEEE802_16Bit",
want: "IEEE802_16Bit",
},
{
name: "IDS",
want: "IDs",
},
// Test skipInitialismChecks functionality
{
name: "getJson",
skipInitialismNameChecks: true,
want: "getJson",
},
{
name: "userId",
skipInitialismNameChecks: true,
want: "userId",
},
{
name: "myHttpClient",
skipInitialismNameChecks: true,
want: "myHttpClient",
},
// Test allowlist functionality
{
name: "fooId",
allowlist: []string{"ID"},
want: "fooId",
},
{
name: "fooApi",
allowlist: []string{"API"},
want: "fooApi",
},
{
name: "fooHttp",
allowlist: []string{"HTTP"},
want: "fooHttp",
},
// Test blocklist functionality
{
name: "fooCustom",
blocklist: []string{"CUSTOM"},
want: "fooCUSTOM",
},
{
name: "mySpecial",
blocklist: []string{"SPECIAL"},
want: "mySPECIAL",
},
// Test combination of allowlist and blocklist
{
name: "fooIdCustom",
allowlist: []string{"ID"},
blocklist: []string{"CUSTOM"},
want: "fooIdCUSTOM",
},
// Test combination of allowlist, blocklist and skipInitialismChecks
{
name: "fooIdCustomHttpJson",
allowlist: []string{"ID"},
blocklist: []string{"CUSTOM"},
skipInitialismNameChecks: true,
want: "fooIdCustomHttpJson",
},
}
for _, test := range tests {
got := rule.Name(test.name, test.allowlist, test.blocklist, test.skipInitialismNameChecks)
if got != test.want {
t.Errorf("name(%q, allowlist=%v, blocklist=%v, skipInitialismNameChecks=%v) = %q, want %q",
test.name, test.allowlist, test.blocklist, test.skipInitialismNameChecks, got, test.want)
}
}
}

134
lint/name.go
View File

@@ -1,134 +1,10 @@
package lint
import (
"strings"
"unicode"
)
import "github.com/mgechev/revive/internal/rule"
// Name returns a different name if it should be different.
func Name(name string, allowlist, blocklist []string) (should string) {
// Fast path for simple cases: "_" and all lowercase.
if name == "_" {
return name
}
allLower := true
for _, r := range name {
if !unicode.IsLower(r) {
allLower = false
break
}
}
if allLower {
return name
}
// Split camelCase at any lower->upper transition, and split on underscores.
// Check each word for common initialisms.
runes := []rune(name)
w, i := 0, 0 // index of start of word, scan
for i+1 <= len(runes) {
eow := false // whether we hit the end of a word
switch {
case i+1 == len(runes):
eow = true
case runes[i+1] == '_':
// underscore; shift the remainder forward over any run of underscores
eow = true
n := 1
for i+n+1 < len(runes) && runes[i+n+1] == '_' {
n++
}
// Leave at most one underscore if the underscore is between two digits
if i+n+1 < len(runes) && unicode.IsDigit(runes[i]) && unicode.IsDigit(runes[i+n+1]) {
n--
}
copy(runes[i+1:], runes[i+n+1:])
runes = runes[:len(runes)-n]
case unicode.IsLower(runes[i]) && !unicode.IsLower(runes[i+1]):
// lower->non-lower
eow = true
}
i++
if !eow {
continue
}
// [w,i) is a word.
word := string(runes[w:i])
ignoreInitWarnings := map[string]bool{}
for _, i := range allowlist {
ignoreInitWarnings[i] = true
}
extraInits := map[string]bool{}
for _, i := range blocklist {
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)
}
// Keep lowercase s for IDs
if u == "IDS" {
u = "IDs"
}
// All the common initialisms are ASCII,
// so we can replace the bytes exactly.
copy(runes[w:], []rune(u))
} else if w > 0 && strings.ToLower(word) == word {
// already all lowercase, and not the first word, so uppercase the first character.
runes[w] = unicode.ToUpper(runes[w])
}
w = i
}
return string(runes)
}
// commonInitialisms is a set of common initialisms.
// Only add entries that are highly unlikely to be non-initialisms.
// For instance, "ID" is fine (Freudian code is rare), but "AND" is not.
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,
"IDS": 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,
//
// Deprecated: Do not use this function, it will be removed in the next major release.
func Name(name string, allowlist, blocklist []string) string {
return rule.Name(name, allowlist, blocklist, false)
}

44
lint/name_test.go
View File

@@ -1,44 +0,0 @@
package lint
import "testing"
// TestName tests Name function.
func TestName(t *testing.T) {
tests := []struct {
name, want string
}{
{"foo_bar", "fooBar"},
{"foo_bar_baz", "fooBarBaz"},
{"Foo_bar", "FooBar"},
{"foo_WiFi", "fooWiFi"},
{"id", "id"},
{"Id", "ID"},
{"foo_id", "fooID"},
{"fooId", "fooID"},
{"fooUid", "fooUID"},
{"idFoo", "idFoo"},
{"uidFoo", "uidFoo"},
{"midIdDle", "midIDDle"},
{"APIProxy", "APIProxy"},
{"ApiProxy", "APIProxy"},
{"apiProxy", "apiProxy"},
{"_Leading", "_Leading"},
{"___Leading", "_Leading"},
{"trailing_", "trailing"},
{"trailing___", "trailing"},
{"a_b", "aB"},
{"a__b", "aB"},
{"a___b", "aB"},
{"Rpc1150", "RPC1150"},
{"case3_1", "case3_1"},
{"case3__1", "case3_1"},
{"IEEE802_16bit", "IEEE802_16bit"},
{"IEEE802_16Bit", "IEEE802_16Bit"},
}
for _, test := range tests {
got := Name(test.name, nil, nil)
if got != test.want {
t.Errorf("Name(%q) = %q, want %q", test.name, got, test.want)
}
}
}

37
rule/var_naming.go
View File

@@ -9,6 +9,7 @@ import (
"sync"
"github.com/mgechev/revive/internal/astutils"
"github.com/mgechev/revive/internal/rule"
"github.com/mgechev/revive/lint"
)
@@ -32,8 +33,10 @@ var defaultBadPackageNames = map[string]struct{}{
// VarNamingRule lints the name of a variable.
type VarNamingRule struct {
allowList []string
blockList []string
allowList []string
blockList []string
skipInitialismNameChecks bool // if true disable enforcing capitals for common initialisms
allowUpperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants
skipPackageNameChecks bool // check for meaningless and user-defined bad package names
extraBadPackageNames map[string]struct{} // inactive if skipPackageNameChecks is false
@@ -78,6 +81,8 @@ func (r *VarNamingRule) Configure(arguments lint.Arguments) error {
}
for k, v := range args {
switch {
case isRuleOption(k, "skipInitialismNameChecks"):
r.skipInitialismNameChecks = fmt.Sprint(v) == "true"
case isRuleOption(k, "upperCaseConst"):
r.allowUpperCaseConst = fmt.Sprint(v) == "true"
case isRuleOption(k, "skipPackageNameChecks"):
@@ -116,12 +121,13 @@ func (r *VarNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure
fileAst := file.AST
walker := lintNames{
file: file,
fileAst: fileAst,
allowList: r.allowList,
blockList: r.blockList,
onFailure: onFailure,
upperCaseConst: r.allowUpperCaseConst,
file: file,
fileAst: fileAst,
onFailure: onFailure,
allowList: r.allowList,
blockList: r.blockList,
skipInitialismChecks: r.skipInitialismNameChecks,
upperCaseConst: r.allowUpperCaseConst,
}
ast.Walk(&walker, fileAst)
@@ -177,12 +183,13 @@ func (*VarNamingRule) pkgNameFailure(node ast.Node, msg string, args ...any) lin
}
type lintNames struct {
file *lint.File
fileAst *ast.File
onFailure func(lint.Failure)
allowList []string
blockList []string
upperCaseConst bool
file *lint.File
fileAst *ast.File
onFailure func(lint.Failure)
allowList []string
blockList []string
skipInitialismChecks bool
upperCaseConst bool
}
func (w *lintNames) checkList(fl *ast.FieldList, thing string) {
@@ -221,7 +228,7 @@ func (w *lintNames) check(id *ast.Ident, thing string) {
return
}
should := lint.Name(id.Name, w.allowList, w.blockList)
should := rule.Name(id.Name, w.allowList, w.blockList, w.skipInitialismChecks)
if id.Name == should {
return
}

95
rule/var_naming_test.go
View File

@@ -10,23 +10,25 @@ import (
func TestVarNamingRule_Configure(t *testing.T) {
tests := []struct {
name string
arguments lint.Arguments
wantErr error
wantAllowList []string
wantBlockList []string
wantAllowUpperCaseConst bool
wantSkipPackageNameChecks bool
wantBadPackageNames map[string]struct{}
name string
arguments lint.Arguments
wantErr error
wantAllowList []string
wantBlockList []string
wantSkipInitialismNameChecks bool
wantAllowUpperCaseConst bool
wantSkipPackageNameChecks bool
wantBadPackageNames map[string]struct{}
}{
{
name: "no arguments",
arguments: lint.Arguments{},
wantErr: nil,
wantAllowList: nil,
wantBlockList: nil,
wantAllowUpperCaseConst: false,
wantSkipPackageNameChecks: false,
name: "no arguments",
arguments: lint.Arguments{},
wantErr: nil,
wantAllowList: nil,
wantBlockList: nil,
wantSkipInitialismNameChecks: false,
wantAllowUpperCaseConst: false,
wantSkipPackageNameChecks: false,
},
{
name: "valid arguments",
@@ -34,17 +36,19 @@ func TestVarNamingRule_Configure(t *testing.T) {
[]any{"ID"},
[]any{"VM"},
[]any{map[string]any{
"upperCaseConst": true,
"skipPackageNameChecks": true,
"extraBadPackageNames": []any{"helpers", "models"},
"skipInitialismNameChecks": true,
"upperCaseConst": true,
"skipPackageNameChecks": true,
"extraBadPackageNames": []any{"helpers", "models"},
}},
},
wantErr: nil,
wantAllowList: []string{"ID"},
wantBlockList: []string{"VM"},
wantAllowUpperCaseConst: true,
wantSkipPackageNameChecks: true,
wantBadPackageNames: map[string]struct{}{"helpers": {}, "models": {}},
wantErr: nil,
wantAllowList: []string{"ID"},
wantBlockList: []string{"VM"},
wantSkipInitialismNameChecks: true,
wantAllowUpperCaseConst: true,
wantSkipPackageNameChecks: true,
wantBadPackageNames: map[string]struct{}{"helpers": {}, "models": {}},
},
{
name: "valid lowercased arguments",
@@ -52,17 +56,19 @@ func TestVarNamingRule_Configure(t *testing.T) {
[]any{"ID"},
[]any{"VM"},
[]any{map[string]any{
"uppercaseconst": true,
"skippackagenamechecks": true,
"extrabadpackagenames": []any{"helpers", "models"},
"skipinitialismnamechecks": true,
"uppercaseconst": true,
"skippackagenamechecks": true,
"extrabadpackagenames": []any{"helpers", "models"},
}},
},
wantErr: nil,
wantAllowList: []string{"ID"},
wantBlockList: []string{"VM"},
wantAllowUpperCaseConst: true,
wantSkipPackageNameChecks: true,
wantBadPackageNames: map[string]struct{}{"helpers": {}, "models": {}},
wantErr: nil,
wantAllowList: []string{"ID"},
wantBlockList: []string{"VM"},
wantSkipInitialismNameChecks: true,
wantAllowUpperCaseConst: true,
wantSkipPackageNameChecks: true,
wantBadPackageNames: map[string]struct{}{"helpers": {}, "models": {}},
},
{
name: "valid kebab-cased arguments",
@@ -70,17 +76,19 @@ func TestVarNamingRule_Configure(t *testing.T) {
[]any{"ID"},
[]any{"VM"},
[]any{map[string]any{
"upper-case-const": true,
"skip-package-name-checks": true,
"extra-bad-package-names": []any{"helpers", "models"},
"skip-initialism-name-checks": true,
"upper-case-const": true,
"skip-package-name-checks": true,
"extra-bad-package-names": []any{"helpers", "models"},
}},
},
wantErr: nil,
wantAllowList: []string{"ID"},
wantBlockList: []string{"VM"},
wantAllowUpperCaseConst: true,
wantSkipPackageNameChecks: true,
wantBadPackageNames: map[string]struct{}{"helpers": {}, "models": {}},
wantErr: nil,
wantAllowList: []string{"ID"},
wantBlockList: []string{"VM"},
wantSkipInitialismNameChecks: true,
wantAllowUpperCaseConst: true,
wantSkipPackageNameChecks: true,
wantBadPackageNames: map[string]struct{}{"helpers": {}, "models": {}},
},
{
name: "invalid allowlist type",
@@ -149,6 +157,9 @@ func TestVarNamingRule_Configure(t *testing.T) {
if rule.skipPackageNameChecks != tt.wantSkipPackageNameChecks {
t.Errorf("unexpected skipPackageNameChecks: got = %v, want %v", rule.skipPackageNameChecks, tt.wantSkipPackageNameChecks)
}
if rule.skipInitialismNameChecks != tt.wantSkipInitialismNameChecks {
t.Errorf("unexpected skipInitialismNameChecks: got = %v, want %v", rule.skipInitialismNameChecks, tt.wantSkipInitialismNameChecks)
}
if !reflect.DeepEqual(rule.extraBadPackageNames, tt.wantBadPackageNames) {
t.Errorf("unexpected extraBadPackageNames: got = %v, want %v", rule.extraBadPackageNames, tt.wantBadPackageNames)
}

18
test/var_naming_test.go
View File

@@ -11,6 +11,24 @@ func TestVarNaming(t *testing.T) {
testRule(t, "var_naming", &rule.VarNamingRule{}, &lint.RuleConfig{
Arguments: []any{[]any{"ID"}, []any{"VM"}},
})
testRule(t, "var_naming_skip_initialism_name_checks_true", &rule.VarNamingRule{}, &lint.RuleConfig{
Arguments: []any{
[]any{},
[]any{},
[]any{map[string]any{"skip-initialism-name-checks": true}},
}})
testRule(t, "var_naming_skip_initialism_name_checks_false", &rule.VarNamingRule{}, &lint.RuleConfig{
Arguments: []any{
[]any{},
[]any{},
[]any{map[string]any{"skip-initialism-name-checks": false}},
}})
testRule(t, "var_naming_allowlist_blocklist_skip_initialism_name_checks", &rule.VarNamingRule{}, &lint.RuleConfig{
Arguments: []any{
[]any{"ID"},
[]any{"VM"},
[]any{map[string]any{"skip-initialism-name-checks": true}},
}})
testRule(t, "var_naming_test", &rule.VarNamingRule{}, &lint.RuleConfig{})

10
testdata/var_naming_allowlist_blocklist_skip_initialism_name_checks.go vendored Normal file
View File

@@ -0,0 +1,10 @@
package fixtures
func varNamingAllowListBlocklistSkipInitialismNameChecks() string {
customId := "result"
customVm := "result"
_ = customVm
customIds := "result"
_ = customIds
return customId
}

11
testdata/var_naming_skip_initialism_name_checks_false.go vendored Normal file
View File

@@ -0,0 +1,11 @@
package fixtures
const HTTPRes = 200
func readJSON() {
return
}
type SSHConfig struct {
keyPath string
}

11
testdata/var_naming_skip_initialism_name_checks_true.go vendored Normal file
View File

@@ -0,0 +1,11 @@
package fixtures
const HttpRes = 200
func readJson() {
return
}
type SshConfig struct {
keyPath string
}