feature: var-naming - add more bad package names and check for collisions with standard lib packages (#1540)

RULES_DESCRIPTIONS.md

@@ -1717,9 +1717,15 @@ of functions, variables, consts, and structs handle known initialisms (e.g., JSO
 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`)
-to forbid using the values from the list as package names additionally to the standard meaningless ones:
-"common", "interfaces", "misc", "types", "util", "utils".
+When `skipPackageNameChecks` is false (the default), you can configure
+`extraBadPackageNames` (`extrabadpackagenames`, `extra-bad-package-names`)
+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
+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)).
 
@@ -1765,6 +1771,11 @@ arguments = [[], [], [{ skip-package-name-checks = true }]]
 arguments = [[], [], [{ extra-bad-package-names = ["helpers", "models"] }]]
 ```
 
+```toml
+[rule.var-naming]
+arguments = [[], [], [{ skip-package-name-collision-with-go-std = true }]]
+```
+
 ## waitgroup-by-value
 
 _Description_: Function parameters that are passed by value, are in fact a copy of the original argument.

rule/var_naming.go

@@ -23,24 +23,50 @@ var knownNameExceptions = map[string]bool{
 // The rule warns about the usage of any package name in this list if skipPackageNameChecks is false.
 // Values in the list should be lowercased.
 var defaultBadPackageNames = map[string]struct{}{
-	"common":     {},
-	"interfaces": {},
-	"misc":       {},
-	"types":      {},
-	"util":       {},
-	"utils":      {},
+	"api":           {},
+	"common":        {},
+	"interface":     {},
+	"interfaces":    {},
+	"misc":          {},
+	"miscellaneous": {},
+	"shared":        {},
+	"type":          {},
+	"types":         {},
+	"util":          {},
+	"utilities":     {},
+	"utils":         {},
+}
+
+var stdLibPackageNames = map[string]struct{}{
+	"bytes":   {},
+	"context": {},
+	"crypto":  {},
+	"errors":  {},
+	"fmt":     {},
+	"hash":    {},
+	"http":    {},
+	"io":      {},
+	"json":    {},
+	"math":    {},
+	"net":     {},
+	"os":      {},
+	"sort":    {},
+	"string":  {},
+	"time":    {},
+	"xml":     {},
 }
 
 // VarNamingRule lints the name of a variable.
 type VarNamingRule struct {
-	allowList                []string
-	blockList                []string
-	skipInitialismNameChecks bool // if true disable enforcing capitals for common initialisms
+	allowList []string
+	blockList []string
 
-	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
-	pkgNameAlreadyChecked syncSet             // set of packages names already checked
+	allowUpperCaseConst               bool                // if true - allows to use UPPER_SOME_NAMES for constants
+	skipInitialismNameChecks          bool                // if true - disable enforcing capitals for common initialisms
+	skipPackageNameChecks             bool                // if true - disable check for meaningless and user-defined bad package names
+	skipPackageNameCollisionWithGoStd bool                // if true - disable checks for collisions with Go standard library package names
+	extraBadPackageNames              map[string]struct{} // inactive if skipPackageNameChecks is false
+	pkgNameAlreadyChecked             syncSet             // set of packages names already checked
 }
 
 // Configure validates the rule configuration, and configures the rule accordingly.
@@ -103,6 +129,9 @@ func (r *VarNamingRule) Configure(arguments lint.Arguments) error {
 					r.extraBadPackageNames[strings.ToLower(n)] = struct{}{}
 				}
 			}
+			if isRuleOption(k, "skipPackageNameCollisionWithGoStd") {
+				r.skipPackageNameCollisionWithGoStd = true
+			}
 		}
 	}
 	return nil
@@ -154,6 +183,13 @@ func (r *VarNamingRule) applyPackageCheckRules(file *lint.File, onFailure func(f
 	pkgNameNode := file.AST.Name
 	pkgName := pkgNameNode.Name
 	pkgNameLower := strings.ToLower(pkgName)
+
+	// Check if top level package
+	if pkgNameLower == "pkg" && filepath.Base(fileDir) != pkgName {
+		onFailure(r.pkgNameFailure(pkgNameNode, "should not have a root level package called pkg"))
+		return
+	}
+
 	if _, ok := r.extraBadPackageNames[pkgNameLower]; ok {
 		onFailure(r.pkgNameFailure(pkgNameNode, "avoid bad package names"))
 		return
@@ -164,6 +200,10 @@ func (r *VarNamingRule) applyPackageCheckRules(file *lint.File, onFailure func(f
 		return
 	}
 
+	if _, ok := stdLibPackageNames[pkgNameLower]; ok && !r.skipPackageNameCollisionWithGoStd {
+		onFailure(r.pkgNameFailure(pkgNameNode, "avoid package names that conflict with Go standard library package names"))
+	}
+
 	// Package names need slightly different handling than other names.
 	if strings.Contains(pkgName, "_") && !strings.HasSuffix(pkgName, "_test") {
 		onFailure(r.pkgNameFailure(pkgNameNode, "don't use an underscore in package name"))

test/var_naming_test.go

@@ -58,6 +58,13 @@ func TestVarNaming(t *testing.T) {
 			[]any{map[string]any{"skip-package-name-checks": false, "extra-bad-package-names": []any{"helpers"}}},
 		},
 	})
+	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_skip", &rule.VarNamingRule{}, &lint.RuleConfig{
+		Arguments: []any{[]any{}, []any{},
+			[]any{map[string]any{"skip-package-name-collision-with-go-std": true}},
+		},
+	})
 }
 
 func BenchmarkUpperCaseConstTrue(b *testing.B) {

testdata/if_return.go

@@ -1,7 +1,7 @@
 // Test of redundant if err != nil
 
-// Package pkg ...
-package pkg
+// Package fixtures ...
+package fixtures
 
 func f() error {
 	if err := f(); err != nil {

testdata/var_naming_std_lib_conflict.go

@@ -0,0 +1 @@
+package http // MATCH /avoid package names that conflict with Go standard library package names/

testdata/var_naming_std_lib_conflict_skip.go

@@ -0,0 +1 @@
+package http // will not trigger because of skip argument

testdata/var_naming_top_level_pkg.go

@@ -0,0 +1,6 @@
+package pkg // MATCH /should not have a root level package called pkg/
+
+const (
+	Foo = "bar"
+	Bar = "foo"
+)