diff --git a/rule/var_naming.go b/rule/var_naming.go index 818797d..710d141 100644 --- a/rule/var_naming.go +++ b/rule/var_naming.go @@ -5,20 +5,12 @@ import ( "go/ast" "go/token" "path/filepath" - "regexp" "strings" "sync" "github.com/mgechev/revive/lint" ) -var anyCapsRE = regexp.MustCompile(`[A-Z]`) - -var allCapsRE = regexp.MustCompile(`^[A-Z0-9_]+$`) - -// regexp for constant names like `SOME_CONST`, `SOME_CONST_2`, `X123_3`, `_SOME_PRIVATE_CONST` (#851, #865) -var upperCaseConstRE = regexp.MustCompile(`^_?[A-Z][A-Z\d]*(_[A-Z\d]+)*$`) - var knownNameExceptions = map[string]bool{ "LastInsertId": true, // must match database/sql "kWh": true, @@ -165,7 +157,7 @@ func (r *VarNamingRule) applyPackageCheckRules(file *lint.File, onFailure func(f if strings.Contains(pkgName, "_") && !strings.HasSuffix(pkgName, "_test") { onFailure(r.pkgNameFailure(pkgNameNode, "don't use an underscore in package name")) } - if anyCapsRE.MatchString(pkgName) { + if hasUpperCaseLetter(pkgName) { onFailure(r.pkgNameFailure(pkgNameNode, "don't use MixedCaps in package names; %s should be %s", pkgName, pkgNameLower)) } } @@ -200,12 +192,12 @@ func (w *lintNames) check(id *ast.Ident, thing string) { // #851 upperCaseConst support // if it's const - if thing == token.CONST.String() && w.upperCaseConst && upperCaseConstRE.MatchString(id.Name) { + if thing == token.CONST.String() && w.upperCaseConst && isUpperCaseConst(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, "_") { + if isUpperUnderscore(id.Name) { w.onFailure(lint.Failure{ Failure: "don't use ALL_CAPS in Go names; use CamelCase", Confidence: 0.8, @@ -328,6 +320,86 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { return w } +// isUpperCaseConst checks if a string is in constant name format like `SOME_CONST`, `SOME_CONST_2`, `X123_3`, `_SOME_PRIVATE_CONST`. +// See #851, #865. +func isUpperCaseConst(s string) bool { + if len(s) == 0 { + return false + } + r := []rune(s) + c := r[0] + if len(r) == 1 { + return isUpper(c) + } + if c != '_' && !isUpper(c) { // Must start with an uppercase letter or underscore + return false + } + for i, c := range r { + switch { + case isUpperOrDigit(c): + continue + case c == '_': + // Underscore must be followed by at least one uppercase letter or digit + if i+1 >= len(s) || !isUpperOrDigit(r[i+1]) { + return false + } + + default: + return false + } + } + return true +} + +// hasUpperCaseLetter checks if a string contains at least one upper case letter. +func hasUpperCaseLetter(s string) bool { + for _, r := range s { + if isUpper(r) { + return true + } + } + return false +} + +// isUpperOrDigit checks if a rune is an uppercase letter or digit. +func isUpperOrDigit(r rune) bool { + return isUpper(r) || isDigit(r) +} + +// isUpper checks if rune is a simple digit. +// +// We don't use unicode.IsDigit as it returns true for a large variety of digits that are not 0-9. +func isDigit(r rune) bool { + return r >= '0' && r <= '9' +} + +// isUpper checks if rune is ASCII upper case letter +// +// We restrict to A-Z because unicode.IsUpper returns true for a large variety of letters. +func isUpper(r rune) bool { + return r >= 'A' && r <= 'Z' +} + +// isUpperUnderscore detects variable that are made from upper case letters, underscore, or digits. +// +// Short variable names are considered OK. +func isUpperUnderscore(s string) bool { + if !strings.Contains(s, "_") { + return false + } + if len(s) <= 5 { + // avoid false positives + return false + } + for _, r := range s { + if r == '_' || isUpperOrDigit(r) { + continue + } + return false + } + return true +} + func getList(arg any, argName string) ([]string, error) { args, ok := arg.([]any) if !ok { diff --git a/rule/var_naming_test.go b/rule/var_naming_test.go index 7d901f2..21f0294 100644 --- a/rule/var_naming_test.go +++ b/rule/var_naming_test.go @@ -150,3 +150,204 @@ func TestVarNamingRule_Configure(t *testing.T) { }) } } + +func TestHasUpperCaseLetter(t *testing.T) { + tests := []struct { + varName string + expected bool + }{ + {"Exit", true}, + {"fmt", false}, + {"_SOME_PRIVATE_CONST_2", true}, + {"", false}, + // Unicode uppercase (non-ASCII) + {"Ä", false}, // Latin capital letter A with diaeresis + {"Ω", false}, // Greek capital letter Omega + {"Д", false}, // Cyrillic capital letter De + + // Unicode lowercase/symbols + {"ß", false}, // German sharp s + {"π", false}, // Greek small letter pi + {"💡", false}, // Emoji + {"你", false}, // Chinese character + } + + for _, tt := range tests { + t.Run(tt.varName, func(t *testing.T) { + if got := hasUpperCaseLetter(tt.varName); got != tt.expected { + t.Errorf("hasCaps(%s) = %v; want %v", tt.varName, got, tt.expected) + } + }) + } +} + +func TestIsUpperCaseConst(t *testing.T) { + tests := []struct { + varName string + expected bool + }{ + {"SOME_CONST_2", true}, + {"__FOO", false}, + {"__", false}, + {"X509B", true}, + {"FOO", true}, + {"1FOO", false}, + {"_FOO123_BAR456", true}, + {"A1_B2_C3", true}, + {"A1_b2", false}, + {"FOO_", false}, + {"foo", false}, + {"_", false}, + {"", false}, + {"FOOBAR", true}, + {"FO", true}, + {"F_O", true}, + {"FOO123", true}, + } + + for _, tt := range tests { + t.Run(tt.varName, func(t *testing.T) { + if got := isUpperCaseConst(tt.varName); got != tt.expected { + t.Errorf("isUpperCaseConst(%s) = %v; want %v", tt.varName, got, tt.expected) + } + }) + } +} + +func TestIsUpperUnderscore(t *testing.T) { + tests := []struct { + varName string + expected bool + }{ + {"_", false}, + {"", false}, + {"empty string", false}, + {"_404_404", true}, + {"FOO_BAR", true}, + {"FOOBAR", false}, + {"FO", false}, + {"F_O", false}, + {"_FOOBAR", true}, + {"FOOBAR_", true}, + {"FOO123", false}, + {"FOO_123", true}, + } + + for _, tt := range tests { + t.Run(tt.varName, func(t *testing.T) { + if got := isUpperUnderscore(tt.varName); got != tt.expected { + t.Errorf("isUpperUnderScore(%s) = %v; want %v", tt.varName, got, tt.expected) + } + }) + } +} + +func TestIsDigit(t *testing.T) { + tests := []struct { + input rune + expected bool + }{ + {'0', true}, + {'1', true}, + {'2', true}, + {'3', true}, + {'4', true}, + {'5', true}, + {'6', true}, + {'7', true}, + {'8', true}, + {'9', true}, + {'a', false}, + {'Z', false}, + {' ', false}, + {'!', false}, + {'🙂', false}, // Emoji to test unicode + {'٠', false}, // Arabic-Indic 0 + {'١', false}, // Arabic-Indic 1 + {'२', false}, // Devanagari 2 + {'৩', false}, // Bengali 3 + {'४', false}, // Devanagari 4 + {'௫', false}, // Tamil 5 + {'๖', false}, // Thai 6 + {'৭', false}, // Bengali 7 + {'८', false}, // Devanagari 8 + {'९', false}, // Devanagari 9 + } + + for _, tt := range tests { + result := isDigit(tt.input) + if result != tt.expected { + t.Errorf("isDigit(%q) = %v; want %v", tt.input, result, tt.expected) + } + } +} + +func TestIsUpper(t *testing.T) { + t.Run("non letter", func(t *testing.T) { + tests := []rune{ + '0', + '5', + ' ', + '_', + '!', + '🙂', // Emoji to test unicode + } + for _, r := range tests { + result := isUpper(r) + if result { + t.Errorf("isUpper(%q) = %v; want false", r, result) + } + } + }) + + t.Run("non ASCII letter", func(t *testing.T) { + tests := []rune{ + 'Ą', + 'Ć', + '你', + '日', + '本', + '語', + '韓', + '中', + '文', + 'あ', + 'ア', + '한', + } + for _, r := range tests { + result := isUpper(r) + if result { + t.Errorf("isUpper(%q) = %v; want false", r, result) + } + } + }) + + t.Run("lowercase ASCII letter", func(t *testing.T) { + tests := []rune{ + 'a', + 'b', + } + for _, r := range tests { + result := isUpper(r) + if result { + t.Errorf("isUpper(%q) = %v; want false", r, result) + } + } + }) + + t.Run("uppercase ASCII letter", func(t *testing.T) { + tests := []rune{ + 'A', + 'B', + 'C', + 'Z', + } + for _, r := range tests { + result := isUpper(r) + if !result { + t.Errorf("isUpper(%q) = %v; want true", r, result) + } + } + }) +} diff --git a/test/var_naming_test.go b/test/var_naming_test.go index 639773f..8c3cc01 100644 --- a/test/var_naming_test.go +++ b/test/var_naming_test.go @@ -41,3 +41,12 @@ func TestVarNaming(t *testing.T) { }, }) } + +func BenchmarkUpperCaseConstTrue(b *testing.B) { + var t *testing.T + for i := 0; i < b.N; i++ { + testRule(t, "var_naming_upper_case_const_true", &rule.VarNamingRule{}, &lint.RuleConfig{ + Arguments: []any{[]any{}, []any{}, []any{map[string]any{"upperCaseConst": true}}}, + }) + } +}