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

feature: new rule package-directory-mismatch (#1429)

This commit is contained in:
Baranov Victor
2025-08-16 20:16:20 +03:00
committed by GitHub
parent f07a47a1bf
commit 15aded98f6
42 changed files with 340 additions and 0 deletions

View File

@@ -570,6 +570,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| [`nested-structs`](./RULES_DESCRIPTIONS.md#nested-structs) | n/a | Warns on structs within structs | no | no |
| [`optimize-operands-order`](./RULES_DESCRIPTIONS.md#optimize-operands-order) | n/a | Checks inefficient conditional expressions | no | no |
| [`package-comments`](./RULES_DESCRIPTIONS.md#package-comments) | n/a | Package commenting conventions. | yes | no |
| [`package-directory-mismatch`](./RULES_DESCRIPTIONS.md#package-directory-mismatch) | string | Checks that package name matches containing directory name | no | no |
| [`range-val-address`](./RULES_DESCRIPTIONS.md#range-val-address)| n/a | Warns if address of range value is used dangerously | no | yes |
| [`range-val-in-closure`](./RULES_DESCRIPTIONS.md#range-val-in-closure)| n/a | Warns if range value is used in a closure dispatched as goroutine| no | no |
| [`range`](./RULES_DESCRIPTIONS.md#range) | n/a | Prevents redundant variables when iterating over a collection. | yes | no |

View File

@@ -64,6 +64,7 @@ List of all available rules.
- [nested-structs](#nested-structs)
- [optimize-operands-order](#optimize-operands-order)
- [package-comments](#package-comments)
- [package-directory-mismatch](#package-directory-mismatch)
- [range-val-address](#range-val-address)
- [range-val-in-closure](#range-val-in-closure)
- [range](#range)
@@ -963,6 +964,48 @@ More [information here](https://go.dev/wiki/CodeReviewComments#package-comments)
_Configuration_: N/A
## package-directory-mismatch
_Description_: It is considered a good practice to name a package after the directory containing it.
This rule warns when the package name declared in the file does not match the name of the directory containing the file.
The following cases are excluded from this check:
- Package `main` (executable packages)
- Files in `testdata` directories (at any level) - by default
- Files directly in `internal` directories (but files in subdirectories of `internal` are checked)
For test files (files with `_test` suffix), package name additionally checked if it matches directory name with `_test` suffix appended.
The rule normalizes both directory and package names before comparison by removing hyphens (`-`),
underscores (`_`), and dots (`.`). This allows package `foo_barbuz` be equal with directory `foo-bar.buz`.
For files in version directories (`v1`, `v2`, etc.), package name is checked if it matches either the version directory or its parent directory.
_Configuration_: Named arguments for directory exclusions.
Examples:
Default behavior excludes paths containing `testdata`
```toml
[rule.package-directory-mismatch]
```
Ignore specific directories with `ignore-directories`
```toml
[rule.package-directory-mismatch]
arguments = [{ ignore-directories = ["testcases", "testinfo"] }]
```
Include all directories (`testdata` also)
```toml
[rule.package-directory-mismatch]
arguments = [{ ignoreDirectories = [] }]
```
## range-val-address
_Description_: Range variables in a loop are reused at each iteration.

View File

@@ -111,6 +111,7 @@ var allRules = append([]lint.Rule{
&rule.IdenticalIfElseIfBranchesRule{},
&rule.IdenticalSwitchBranchesRule{},
&rule.UselessFallthroughRule{},
&rule.PackageDirectoryMismatchRule{},
}, defaultRules...)
// allFormatters is a list of all available formatters to output the linting results.

View File

@@ -0,0 +1,156 @@
package rule
import (
"fmt"
"path/filepath"
"regexp"
"strings"
"github.com/mgechev/revive/lint"
)
// PackageDirectoryMismatchRule detects when package name doesn't match directory name.
type PackageDirectoryMismatchRule struct {
ignoredDirs *regexp.Regexp
}
const defaultIgnoredDirs = "testdata"
// Configure the rule to exclude certain directories.
func (r *PackageDirectoryMismatchRule) Configure(arguments lint.Arguments) error {
if len(arguments) < 1 {
var err error
r.ignoredDirs, err = r.buildIgnoreRegex([]string{defaultIgnoredDirs})
return err
}
args, ok := arguments[0].(map[string]any)
if !ok {
return fmt.Errorf("invalid argument type: expected map[string]any, got %T", arguments[0])
}
for k, v := range args {
if !isRuleOption(k, "ignoreDirectories") {
return fmt.Errorf("unknown argument %s for %s rule", k, r.Name())
}
ignoredAny, ok := v.([]any)
if !ok {
return fmt.Errorf("invalid value %v for argument %s of rule %s, expected []string got %T", v, k, r.Name(), v)
}
ignoredDirs := make([]string, len(ignoredAny))
for i, item := range ignoredAny {
str, ok := item.(string)
if !ok {
return fmt.Errorf("invalid value in %s argument of rule %s: expected string, got %T", k, r.Name(), item)
}
ignoredDirs[i] = str
}
var err error
r.ignoredDirs, err = r.buildIgnoreRegex(ignoredDirs)
return err
}
return nil
}
func (*PackageDirectoryMismatchRule) buildIgnoreRegex(ignoredDirs []string) (*regexp.Regexp, error) {
if len(ignoredDirs) == 0 {
return nil, nil
}
patterns := make([]string, len(ignoredDirs))
for i, dir := range ignoredDirs {
patterns[i] = regexp.QuoteMeta(dir)
}
pattern := strings.Join(patterns, "|")
regex, err := regexp.Compile(pattern)
if err != nil {
return nil, fmt.Errorf("failed to compile regex for ignored directories: %w", err)
}
return regex, nil
}
// skipDirs contains directory names that should be unconditionally ignored when checking.
// These entries handle edge cases where filepath.Base might return these values.
var skipDirs = map[string]struct{}{
".": {}, // Current directory
"/": {}, // Root directory
"": {}, // Empty path
}
// semanticallyEqual checks if package and directory names are semantically equal to each other.
func (PackageDirectoryMismatchRule) semanticallyEqual(packageName, dirName string) bool {
normDir := normalizePath(dirName)
normPkg := normalizePath(packageName)
return normDir == normPkg || normDir == "go"+normPkg
}
// Apply applies the rule to the given file.
func (r *PackageDirectoryMismatchRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
if file.Pkg.IsMain() {
return nil
}
dirPath := filepath.Dir(file.Name)
dirName := filepath.Base(dirPath)
if r.ignoredDirs != nil && r.ignoredDirs.MatchString(dirPath) {
return nil
}
// Check if we got an invalid directory.
if _, skipDir := skipDirs[dirName]; skipDir {
return nil
}
// Files directly in 'internal/' (like 'internal/abcd.go') should not be checked.
// But files in subdirectories of 'internal/' (like 'internal/foo/abcd.go') should be checked.
if dirName == "internal" {
return nil
}
packageName := file.AST.Name.Name
if r.semanticallyEqual(packageName, dirName) {
return nil
}
if file.IsTest() {
// External test package (directory + '_test' suffix)
if r.semanticallyEqual(packageName, dirName+"_test") {
return nil
}
}
// define a default failure message
failure := fmt.Sprintf("package name %q does not match directory name %q", packageName, dirName)
// For version directories (v1, v2, etc.), we need to check also the parent directory
if isVersionPath(dirName) {
parentDirName := filepath.Base(filepath.Dir(dirPath))
if r.semanticallyEqual(packageName, parentDirName) {
return nil
}
failure = fmt.Sprintf("package name %q does not match directory name %q or parent directory name %q", packageName, dirName, parentDirName)
}
return []lint.Failure{
{
Failure: failure,
Confidence: 1,
Node: file.AST.Name,
Category: lint.FailureCategoryNaming,
},
}
}
// Name returns the rule name.
func (*PackageDirectoryMismatchRule) Name() string {
return "package-directory-mismatch"
}

View File

@@ -55,6 +55,30 @@ func normalizeRuleOption(arg string) string {
return strings.ToLower(strings.ReplaceAll(arg, "-", ""))
}
var normalizePathReplacer = strings.NewReplacer("-", "", "_", "", ".", "")
// normalizePath removes hyphens, underscores, and dots from the name
//
// Example: normalizePath("foo.bar-_buz") -> "foobarbuz".
func normalizePath(name string) string {
return normalizePathReplacer.Replace(name)
}
// isVersionPath checks if a directory name is a version directory (v1, V2, etc.)
func isVersionPath(name string) bool {
if len(name) < 2 || (name[0] != 'v' && name[0] != 'V') {
return false
}
for i := 1; i < len(name); i++ {
if name[i] < '0' || name[i] > '9' {
return false
}
}
return true
}
var directiveCommentRE = regexp.MustCompile("^//(line |extern |export |[a-z0-9]+:[a-z0-9])") // see https://go-review.googlesource.com/c/website/+/442516/1..2/_content/doc/comment.md#494
func isDirectiveComment(line string) bool {

View File

@@ -0,0 +1,77 @@
package test
import (
"testing"
"github.com/mgechev/revive/lint"
"github.com/mgechev/revive/rule"
)
func TestPackageDirectoryMismatch(t *testing.T) {
// Configure rule to ignore no directories so our tests in 'testdata/' can run
config := &lint.RuleConfig{Arguments: []any{map[string]any{"ignore-directories": []any{}}}}
testRule(t, "package_directory_mismatch/good/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/bad/bad", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/maincmd/main", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/mixed/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/mixed/bad", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/go-good/good1", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/go-good/good2", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/go-good/bad", &rule.PackageDirectoryMismatchRule{}, config)
// Test normalization cases
testRule(t, "package_directory_mismatch/normalization/fo-ob_ar/foobar", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/foo_bar/foobar", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/foo.b_ar/foobar", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/go-foo_bar/foo_bar", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/go-foo_bar/foobar", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/go-foo-bar/foo_bar", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/go-foo-bar/foobar", &rule.PackageDirectoryMismatchRule{}, config)
// Edge cases that are not logic, but we decided to ignore to have a simpler implementation
testRule(t, "package_directory_mismatch/normalization/go-od/go_od", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/go-od/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/normalization/go-od/od", &rule.PackageDirectoryMismatchRule{}, config)
// Test version directories (v1, v2, etc.)
testRule(t, "package_directory_mismatch/api/v1/api", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/api/V1/api", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/api/v1v/api", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/api/v2/v2", &rule.PackageDirectoryMismatchRule{}, config)
// Test internal directory variations
testRule(t, "package_directory_mismatch/internal/good/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/internal/bad/bad", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/internal/any", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/internal/api/v1/api", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/internal/api/v2/api", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/internal/v1/api", &rule.PackageDirectoryMismatchRule{}, config)
// Test handling of testfiles
testRule(t, "package_directory_mismatch/test/good_test", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/test/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/test/bad_test", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/test/bad", &rule.PackageDirectoryMismatchRule{}, config)
}
func TestPackageDirectoryMismatchWithDefaultConfig(t *testing.T) {
// Test with default configuration (should ignore testdata directories by default)
testRule(t, "package_directory_mismatch/testdata/ignored", &rule.PackageDirectoryMismatchRule{})
}
func TestPackageDirectoryMismatchWithTestDirectories(t *testing.T) {
// Test with new format that excludes specific test directories
config := &lint.RuleConfig{Arguments: []any{map[string]any{"ignore-directories": []any{"testinfo", "testutils"}}}}
testRule(t, "package_directory_mismatch/testinfo/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/testutils/good", &rule.PackageDirectoryMismatchRule{}, config)
}
func TestPackageDirectoryMismatchWithMultipleDirectories(t *testing.T) {
// Test with new named argument format that excludes both "testutils" and "testinfo"
config := &lint.RuleConfig{Arguments: []any{map[string]any{"ignoreDirectories": []any{"testutils", "testinfo"}}}}
testRule(t, "package_directory_mismatch/testutils/good", &rule.PackageDirectoryMismatchRule{}, config)
testRule(t, "package_directory_mismatch/testinfo/good", &rule.PackageDirectoryMismatchRule{}, config)
}

View File

@@ -0,0 +1 @@
package api

View File

@@ -0,0 +1 @@
package api

View File

@@ -0,0 +1 @@
package api // MATCH /package name "api" does not match directory name "v1v"/

View File

@@ -0,0 +1 @@
package v2

View File

@@ -0,0 +1 @@
package ptr // MATCH /package name "ptr" does not match directory name "bad"/

View File

@@ -0,0 +1 @@
package bad // MATCH /package name "bad" does not match directory name "go-good"/

View File

@@ -0,0 +1 @@
package go_od

View File

@@ -0,0 +1 @@
package go_good

View File

@@ -0,0 +1 @@
package good

View File

@@ -0,0 +1 @@
package any

View File

@@ -0,0 +1 @@
package bad // MATCH /package name "bad" does not match directory name "v1" or parent directory name "api"/

View File

@@ -0,0 +1 @@
package api

View File

@@ -0,0 +1 @@
package wrong // MATCH /package name "wrong" does not match directory name "bad"/

View File

@@ -0,0 +1 @@
package good

View File

@@ -0,0 +1 @@
package api // MATCH /package name "api" does not match directory name "v1" or parent directory name "internal"/

View File

@@ -0,0 +1,2 @@
// main packages can be in any directory
package main

View File

@@ -0,0 +1 @@
package wrong // MATCH /package name "wrong" does not match directory name "mixed"/

View File

@@ -0,0 +1 @@
package mixed

View File

@@ -0,0 +1 @@
package foobar

View File

@@ -0,0 +1 @@
package foobar

View File

@@ -0,0 +1 @@
package foobar

View File

@@ -0,0 +1 @@
package foo_bar

View File

@@ -0,0 +1 @@
package foobar

View File

@@ -0,0 +1 @@
package foo_bar

View File

@@ -0,0 +1 @@
package foobar

View File

@@ -0,0 +1 @@
package go_od

View File

@@ -0,0 +1 @@
package good

View File

@@ -0,0 +1 @@
package od

View File

@@ -0,0 +1 @@
package bad // MATCH /package name "bad" does not match directory name "test"/

View File

@@ -0,0 +1 @@
package bad_test // MATCH /package name "bad_test" does not match directory name "test"/

View File

@@ -0,0 +1 @@
package test

View File

@@ -0,0 +1 @@
package test_test

View File

@@ -0,0 +1 @@
package bad // MATCH /package name "bad" does not match directory name "testdata"/

View File

@@ -0,0 +1 @@
package ignored

View File

@@ -0,0 +1,2 @@
// This would normally cause a violation, but should be excluded by regex
package good

View File

@@ -0,0 +1 @@
package good