Repository: mgechev/revive
Pull request #1322 (2025-04-19 00:20:12)

var-naming: remove regex from code
related with #1313 

replaced regexp with functions eqivalent, and add some tests and benchmarks

```
go test -bench=.
goos: linux
goarch: amd64
pkg: github.com/mgechev/revive/rule
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
BenchmarkAnyCapsRE-8            35753906                34.16 ns/op
BenchmarkAnyCaps-8              1000000000               0.8155 ns/op
BenchmarkAllCapsRE-8             4175530               311.3 ns/op
BenchmarkAllCaps-8              89929506                13.28 ns/op
BenchmarkUpperCaseConstRE-8      9525739               133.2 ns/op
BenchmarkIsUpperCaseConst-8     211988328                5.434 ns/op
PASS
ok      github.com/mgechev/revive/rule  9.191s
```
as we can see above, results of benchmarks using regexp are much slower (benchmarks with RE suffix)

ps. sorry for previous versions of that PR, but I had old not synced master on my fork :( 

ccoVeille: > The standard package unicode provides utility functions like IsDigit, IsLower, IsUpper ... 

I'm fine with this. But then all characters from non-latin languages (Greek,  Russian, CJK…) would be allowed.

I'm fine with this, as I said by opening:
- https://github.com/mgechev/revive/issues/1315


mfederowicz: > 1. As benchmarks show, and it was expected, RE are slower than the new functions, that is a fact. How much faster is the rule when it uses the new functions?

maybe using ```time``` for measuring execution time is not the best, but i notice something interesting (increase number of warnings), when i switch revive version from 1.9.0 to 1.9.0-2-g17527c9 (builded from git source: master)

results from test executions on kubernetes master branch (https://github.com/kubernetes/kubernetes)

test command: ```time revive --formatter friendly ./...```

version 1.8.0: installed from go install -x github.com/mgechev/revive@v1.8.0
upperCaseConst=false
Warnings:1928  var-naming  

real    0m2.311s
user    0m6.862s
sys     0m0.757s

upperCaseConst=true
Warnings:1923  var-naming  

real    0m2.376s
user    0m6.930s
sys     0m0.788s

version:1.9.0: installed from go install -x github.com/mgechev/revive@latest
upperCaseConst=false
Warnings:1928  var-naming  
real    0m2.459s
user    0m6.971s
sys     0m0.837s

upperCaseConst=true
Warnings:1923  var-naming  

real    0m2.360s
user    0m6.877s
sys     0m0.761s


version dev:v1.9.0-2-g17527c9 (build from git:master)
upperCaseConst=false
Warnings: 2131  var-naming  

real    0m2.382s
user    0m6.998s
sys     0m0.765s

upperCaseConst=true
Warnings: 2126  var-naming  

real    0m2.328s
user    0m6.861s
sys     0m0.715s

version dev:v1.9.0-4-g5f63cc2 (build from git branch without regexp)
upperCaseConst=false
Warnings: 2131  var-naming  

real    0m2.196s
user    0m6.726s
sys     0m0.732s

upperCaseConst=true
Warnings: 2126  var-naming  

real    0m2.412s
user    0m6.836s
sys     0m0.723s



chavacava: > maybe using time for measuring execution time is not the best

[hyperfine](https://github.com/sharkdp/hyperfine) might be the right tool for this kind of benchmarks.

>  i notice something interesting (increase number of warnings), 
> when i switch revive version from 1.9.0 to 1.9.0-2-g17527c9

PR #1312 (merged after 1.9.0) adds new failures
ccoVeille: @mfederowicz if you agree, I would appreciate a Co-Authored-By tag.


ccoVeille: > > maybe using time for measuring execution time is not the best
> 
> [hyperfine](https://github.com/sharkdp/hyperfine) might be the right tool for this kind of benchmarks.

Also, did you have a chance to use tools like hyperfine like what @chavacava suggested here?





mfederowicz: @ccoVeille yes what we want to measure (what scenario)

maybe 

run ```hyperfine revive --formatter friendly ./...``` on https://github.com/kubernetes/kubernetes with flag: upperCaseConst=true and false

with which versions?
ccoVeille: I would compare the code from this branch ( 2fa9077146104a1731749994cc9a2afacea1b507 ) and the code from master ( 17527c928baeac86c96a6b87909ef9a5539dd8e3 )

Also, I would limit the rules to var-naming only via a bench.toml config file, then use -- config=bench.toml 
mfederowicz: @ccoVeille iam confused, below results of tests executed on master: https://github.com/kubernetes/kubernetes

branch/revive - v1.9.0-12-g2fa9077
master/revive - v1.9.0-2-g17527c9

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `branch/revive --config=bench.false.toml --formatter friendly ./...` | 2.376 ± 0.064 | 2.286 | 2.477 | 1.00 |
| `branch/revive --config=bench.true.toml --formatter friendly ./...` | 2.355 ± 0.058 | 2.248 | 2.443 | 1.00 |
| `master/revive --config=bench.false.toml --formatter friendly ./...` | 2.387 ± 0.042 | 2.328 | 2.470 | 1.00 |
| `master/revive --config=bench.true.toml --formatter friendly ./...` | 2.329 ± 0.069 | 2.206 | 2.414 | 1.00 |
mfederowicz: ok @alexandear, @ccoVeille, @chavacava  i resolved conflict from master, so we can merge this PR :P
ccoVeille: > @mfederowicz if you agree, I would appreciate a Co-Authored-By tag.
> 

Could you ?

```
Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
```


ccoVeille: @denisvmedia if you feel there is a need for allowing "extended Unicode character" in the same way Go allows us to use via the spec.

You can take a look at #1315

-#1315



mfederowicz: ok @ccoVeille I added you as Co-authored in last commit, as some one said: last but not least :)
mfederowicz: @ccoVeille interesting because in my mailbox i have email with suggested by you changes, but suggested change is the same code (or maybe it is time to sleep :))
ccoVeille: > @ccoVeille interesting because in my mailbox i have email with suggested by you changes, but suggested change is the same code (or maybe it is time to sleep :))

I simply send too fast, so I deleted,and now I  managed to provide what I initially meant

Patch:
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 {


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