feat(var-naming): restore golint behavior by default, backwards-compatible rename and invert skipPackageNameCollisionWithGoStd

RULES_DESCRIPTIONS.md

@@ -1766,9 +1766,9 @@ When `skipPackageNameChecks` is false (the default), you can configure
 to forbid using the values from the list as package names additionally
 to the standard meaningless ones: "common", "interfaces", "misc",
 "types", "util", "utils".
-When `skipPackageNameCollisionWithGoStd`
-(`skippackagenamecollisionwithgostd`, `skip-package-name-collision-with-go-std`)
-is set to true, the rule disables checks on package names that collide
+When `checkPackageNameCollisionWithGoStd`
+(`checkpackagenamecollisionwithgostd`, `check-package-name-collision-with-go-std`)
+is set to true, the rule checks for package names that collide
 with Go standard library packages.
 
 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)).
@@ -1817,7 +1817,7 @@ arguments = [[], [], [{ extra-bad-package-names = ["helpers", "models"] }]]
 
 ```toml
 [rule.var-naming]
-arguments = [[], [], [{ skip-package-name-collision-with-go-std = true }]]
+arguments = [[], [], [{ check-package-name-collision-with-go-std = true }]]
 ```
 
 ## waitgroup-by-value

logging/logger.go

@@ -21,19 +21,30 @@ func GetLogger() (*slog.Logger, error) {
 		return logger, nil
 	}
 
-	debugModeEnabled := os.Getenv("DEBUG") != ""
-	if !debugModeEnabled {
-		// by default, suppress all logging output
-		return slog.New(slog.DiscardHandler), nil
-	}
+	// By default, suppress all logging output below level WARN,
+	// and only log to stderr.
+	leveler := new(slog.LevelVar)
+	leveler.Set(slog.LevelWarn)
+	opts := &slog.HandlerOptions{Level: leveler}
 
-	var err error
-	loggerFile, err = os.Create(logFile)
-	if err != nil {
-		return nil, err
+	var out io.Writer = os.Stderr
+
+	debugModeEnabled := os.Getenv("DEBUG") != ""
+	if debugModeEnabled {
+		// In DEBUG mode, log all levels of output at level DEBUG and higher,
+		// to both stderr and the logFile.
+		leveler.Set(slog.LevelDebug)
+
+		var err error
+		loggerFile, err = os.Create(logFile)
+		if err != nil {
+			return nil, err
+		}
+
+		out = io.MultiWriter(os.Stderr, loggerFile)
 	}
 
-	logger = slog.New(slog.NewTextHandler(io.MultiWriter(os.Stderr, loggerFile), nil))
+	logger = slog.New(slog.NewTextHandler(out, opts))
 
 	logger.Info("Logger initialized", "logFile", logFile)
 
@@ -42,6 +53,7 @@ func GetLogger() (*slog.Logger, error) {
 
 // Close closes the logger file if it was opened.
 func Close() error {
+	logger = nil
 	if loggerFile == nil {
 		return nil
 	}

logging/logger_test.go

@@ -10,6 +10,11 @@ import (
 func TestGetLogger(t *testing.T) {
 	t.Run("no debug", func(t *testing.T) {
 		t.Setenv("DEBUG", "")
+		t.Cleanup(func() {
+			if err := logging.Close(); err != nil {
+				t.Error(err)
+			}
+		})
 
 		logger, err := logging.GetLogger()
 		if err != nil {

rule/var_naming.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mgechev/revive/internal/astutils"
 	"github.com/mgechev/revive/internal/rule"
 	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/logging"
 )
 
 var knownNameExceptions = map[string]bool{
@@ -50,9 +51,9 @@ type VarNamingRule struct {
 	extraBadPackageNames     map[string]struct{} // inactive if skipPackageNameChecks is false
 	pkgNameAlreadyChecked    syncSet             // set of packages names already checked
 
-	skipPackageNameCollisionWithGoStd bool // if true - disable checks for collisions with Go standard library package names
+	checkPackageNameCollisionWithGoStd bool // if true - checks for collisions with Go standard library package names
 	// stdPackageNames holds the names of standard library packages excluding internal and vendor.
-	// populated only if skipPackageNameCollisionWithGoStd is false.
+	// populated only if checkPackageNameCollisionWithGoStd is true.
 	// E.g., `net/http` stored as `http`, `math/rand/v2` - `rand` etc.
 	stdPackageNames map[string]struct{}
 }
@@ -117,11 +118,25 @@ func (r *VarNamingRule) Configure(arguments lint.Arguments) error {
 					r.extraBadPackageNames[strings.ToLower(n)] = struct{}{}
 				}
 			case isRuleOption(k, "skipPackageNameCollisionWithGoStd"):
-				r.skipPackageNameCollisionWithGoStd = fmt.Sprint(v) == "true"
+				r.checkPackageNameCollisionWithGoStd = fmt.Sprint(v) != "true"
+
+				slogger, err := logging.GetLogger()
+				if err == nil {
+					slogger.Warn(
+						"an option in the configuration is deprecated, attempting to interpret it and continue",
+						"deprecated_option", k,
+						"suggestion", "instead, use its inverse, checkPackageNameCollisionWithGoStd",
+						"interpreted_as", map[string]any{
+							"checkPackageNameCollisionWithGoStd": r.checkPackageNameCollisionWithGoStd,
+						},
+					)
+				}
+			case isRuleOption(k, "checkPackageNameCollisionWithGoStd"):
+				r.checkPackageNameCollisionWithGoStd = fmt.Sprint(v) == "true"
 			}
 		}
 	}
-	if !r.skipPackageNameCollisionWithGoStd && r.stdPackageNames == nil {
+	if r.checkPackageNameCollisionWithGoStd && r.stdPackageNames == nil {
 		pkgs, err := gopackages.Load(nil, "std")
 		if err != nil {
 			return fmt.Errorf("load std packages: %w", err)
@@ -215,7 +230,7 @@ func (r *VarNamingRule) applyPackageCheckRules(file *lint.File, onFailure func(f
 		return
 	}
 
-	if !r.skipPackageNameCollisionWithGoStd {
+	if r.checkPackageNameCollisionWithGoStd {
 		if _, ok := r.stdPackageNames[pkgNameLower]; ok {
 			onFailure(r.pkgNameFailure(pkgNameNode, "avoid package names that conflict with Go standard library package names"))
 		}

rule/var_naming_test.go

@@ -10,16 +10,16 @@ import (
 
 func TestVarNamingRule_Configure(t *testing.T) {
 	tests := []struct {
-		name                                  string
-		arguments                             lint.Arguments
-		wantErr                               error
-		wantAllowList                         []string
-		wantBlockList                         []string
-		wantSkipInitialismNameChecks          bool
-		wantAllowUpperCaseConst               bool
-		wantSkipPackageNameChecks             bool
-		wantBadPackageNames                   map[string]struct{}
-		wantSkipPackageNameCollisionWithGoStd bool
+		name                                   string
+		arguments                              lint.Arguments
+		wantErr                                error
+		wantAllowList                          []string
+		wantBlockList                          []string
+		wantSkipInitialismNameChecks           bool
+		wantAllowUpperCaseConst                bool
+		wantSkipPackageNameChecks              bool
+		wantBadPackageNames                    map[string]struct{}
+		wantCheckPackageNameCollisionWithGoStd bool
 	}{
 		{
 			name:                         "no arguments",
@@ -37,65 +37,65 @@ func TestVarNamingRule_Configure(t *testing.T) {
 				[]any{"ID"},
 				[]any{"VM"},
 				[]any{map[string]any{
-					"skipInitialismNameChecks":          true,
-					"upperCaseConst":                    true,
-					"skipPackageNameChecks":             true,
-					"extraBadPackageNames":              []any{"helpers", "models"},
-					"skipPackageNameCollisionWithGoStd": true,
+					"skipInitialismNameChecks":           true,
+					"upperCaseConst":                     true,
+					"skipPackageNameChecks":              true,
+					"extraBadPackageNames":               []any{"helpers", "models"},
+					"checkPackageNameCollisionWithGoStd": true,
 				}},
 			},
-			wantErr:                               nil,
-			wantAllowList:                         []string{"ID"},
-			wantBlockList:                         []string{"VM"},
-			wantSkipInitialismNameChecks:          true,
-			wantAllowUpperCaseConst:               true,
-			wantSkipPackageNameChecks:             true,
-			wantBadPackageNames:                   map[string]struct{}{"helpers": {}, "models": {}},
-			wantSkipPackageNameCollisionWithGoStd: true,
+			wantErr:                                nil,
+			wantAllowList:                          []string{"ID"},
+			wantBlockList:                          []string{"VM"},
+			wantSkipInitialismNameChecks:           true,
+			wantAllowUpperCaseConst:                true,
+			wantSkipPackageNameChecks:              true,
+			wantBadPackageNames:                    map[string]struct{}{"helpers": {}, "models": {}},
+			wantCheckPackageNameCollisionWithGoStd: true,
 		},
 		{
 			name: "valid lowercased arguments",
 			arguments: lint.Arguments{
 				[]any{"ID"},
 				[]any{"VM"},
 				[]any{map[string]any{
-					"skipinitialismnamechecks":          true,
-					"uppercaseconst":                    true,
-					"skippackagenamechecks":             true,
-					"extrabadpackagenames":              []any{"helpers", "models"},
-					"skippackagenamecollisionwithgostd": true,
+					"skipinitialismnamechecks":           true,
+					"uppercaseconst":                     true,
+					"skippackagenamechecks":              true,
+					"extrabadpackagenames":               []any{"helpers", "models"},
+					"checkpackagenamecollisionwithgostd": true,
 				}},
 			},
-			wantErr:                               nil,
-			wantAllowList:                         []string{"ID"},
-			wantBlockList:                         []string{"VM"},
-			wantSkipInitialismNameChecks:          true,
-			wantAllowUpperCaseConst:               true,
-			wantSkipPackageNameChecks:             true,
-			wantBadPackageNames:                   map[string]struct{}{"helpers": {}, "models": {}},
-			wantSkipPackageNameCollisionWithGoStd: true,
+			wantErr:                                nil,
+			wantAllowList:                          []string{"ID"},
+			wantBlockList:                          []string{"VM"},
+			wantSkipInitialismNameChecks:           true,
+			wantAllowUpperCaseConst:                true,
+			wantSkipPackageNameChecks:              true,
+			wantBadPackageNames:                    map[string]struct{}{"helpers": {}, "models": {}},
+			wantCheckPackageNameCollisionWithGoStd: true,
 		},
 		{
 			name: "valid kebab-cased arguments",
 			arguments: lint.Arguments{
 				[]any{"ID"},
 				[]any{"VM"},
 				[]any{map[string]any{
-					"skip-initialism-name-checks":             true,
-					"upper-case-const":                        true,
-					"skip-package-name-checks":                true,
-					"extra-bad-package-names":                 []any{"helpers", "models"},
-					"skip-package-name-collision-with-go-std": true,
+					"skip-initialism-name-checks":              true,
+					"upper-case-const":                         true,
+					"skip-package-name-checks":                 true,
+					"extra-bad-package-names":                  []any{"helpers", "models"},
+					"check-package-name-collision-with-go-std": true,
 				}},
 			},
-			wantErr:                               nil,
-			wantAllowList:                         []string{"ID"},
-			wantBlockList:                         []string{"VM"},
-			wantSkipInitialismNameChecks:          true,
-			wantAllowUpperCaseConst:               true,
-			wantSkipPackageNameChecks:             true,
-			wantBadPackageNames:                   map[string]struct{}{"helpers": {}, "models": {}},
-			wantSkipPackageNameCollisionWithGoStd: true,
+			wantErr:                                nil,
+			wantAllowList:                          []string{"ID"},
+			wantBlockList:                          []string{"VM"},
+			wantSkipInitialismNameChecks:           true,
+			wantAllowUpperCaseConst:                true,
+			wantSkipPackageNameChecks:              true,
+			wantBadPackageNames:                    map[string]struct{}{"helpers": {}, "models": {}},
+			wantCheckPackageNameCollisionWithGoStd: true,
 		},
 		{
 			name:      "invalid allowlist type",
@@ -170,22 +170,51 @@ func TestVarNamingRule_Configure(t *testing.T) {
 			if !reflect.DeepEqual(rule.extraBadPackageNames, tt.wantBadPackageNames) {
 				t.Errorf("unexpected extraBadPackageNames: got = %v, want %v", rule.extraBadPackageNames, tt.wantBadPackageNames)
 			}
-			if rule.skipPackageNameCollisionWithGoStd != tt.wantSkipPackageNameCollisionWithGoStd {
-				t.Errorf("unexpected skipPackageNameCollisionWithGoStd: got = %v, want %v", rule.skipPackageNameCollisionWithGoStd, tt.wantSkipPackageNameCollisionWithGoStd)
+			if rule.checkPackageNameCollisionWithGoStd != tt.wantCheckPackageNameCollisionWithGoStd {
+				t.Errorf("unexpected checkPackageNameCollisionWithGoStd: got = %v, want %v", rule.checkPackageNameCollisionWithGoStd, tt.wantCheckPackageNameCollisionWithGoStd)
 			}
 		})
 	}
 }
 
 func TestVarNamingRule_Configure_LoadStdPackages(t *testing.T) {
-	t.Run("loads std packages", func(t *testing.T) {
+	checkGoStdConfig := lint.Arguments{
+		[]any(nil),
+		[]any(nil),
+		[]any{map[string]any{
+			"checkPackageNameCollisionWithGoStd": true,
+		}},
+	}
+
+	t.Run("nil config defaults to golint behavior and skips std packages", func(t *testing.T) {
 		var rule VarNamingRule
 
 		err := rule.Configure(nil)
 		if err != nil {
 			t.Fatalf("unexpected error: got = %v, want = nil", err)
 		}
 
+		for _, pkg := range []string{"fmt", "http", "json", "rand", "io"} {
+			if _, ok := rule.stdPackageNames[pkg]; ok {
+				t.Errorf("expected package %q to be not loaded, but got loaded", pkg)
+			}
+		}
+
+		for _, pkg := range []string{"v2", "internal", "vendor", "std", "text", "go"} {
+			if _, ok := rule.stdPackageNames[pkg]; ok {
+				t.Errorf("expected package %q to be not loaded, but got loaded", pkg)
+			}
+		}
+	})
+
+	t.Run("loads std packages", func(t *testing.T) {
+		var rule VarNamingRule
+
+		err := rule.Configure(checkGoStdConfig)
+		if err != nil {
+			t.Fatalf("unexpected error: got = %v, want = nil", err)
+		}
+
 		for _, pkg := range []string{"fmt", "http", "json", "rand", "io"} {
 			if _, ok := rule.stdPackageNames[pkg]; !ok {
 				t.Errorf("expected package %q to be loaded, but got empty", pkg)
@@ -202,12 +231,12 @@ func TestVarNamingRule_Configure_LoadStdPackages(t *testing.T) {
 	t.Run("does not reload std packages on subsequent Configure calls", func(t *testing.T) {
 		var rule VarNamingRule
 
-		err := rule.Configure(nil)
+		err := rule.Configure(checkGoStdConfig)
 		if err != nil {
 			t.Fatalf("unexpected error: got = %v, want = nil", err)
 		}
 		firstLoadLen := len(rule.stdPackageNames)
-		err = rule.Configure(nil)
+		err = rule.Configure(checkGoStdConfig)
 		if err != nil {
 			t.Fatalf("unexpected error: got = %v, want = nil", err)
 		}
@@ -223,7 +252,7 @@ func TestVarNamingRule_Configure_LoadStdPackages(t *testing.T) {
 			[]any{},
 			[]any{},
 			[]any{map[string]any{
-				"skip-package-name-collision-with-go-std": true,
+				"check-package-name-collision-with-go-std": false,
 			}},
 		})
 		if err != nil {

test/var_naming_test.go

@@ -66,12 +66,18 @@ func TestVarNaming(t *testing.T) {
 		},
 	})
 	testRule(t, "var_naming_top_level_pkg", &rule.VarNamingRule{}, &lint.RuleConfig{})
-	testRule(t, "var_naming_std_lib_conflict", &rule.VarNamingRule{}, &lint.RuleConfig{})
+	testRule(t, "var_naming_std_lib_conflict", &rule.VarNamingRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			[]any{},
+			[]any{},
+			[]any{map[string]any{"check-package-name-collision-with-go-std": true}},
+		},
+	})
 	testRule(t, "var_naming_std_lib_conflict_skip", &rule.VarNamingRule{}, &lint.RuleConfig{
 		Arguments: []any{
 			[]any{},
 			[]any{},
-			[]any{map[string]any{"skip-package-name-collision-with-go-std": true}},
+			[]any{map[string]any{"check-package-name-collision-with-go-std": false}},
 		},
 	})
 }