From 8625f9e7553bf5252ce768a08c9f85ec6712b86f Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Wed, 14 Feb 2024 17:25:25 +0000 Subject: [PATCH] Add a check for redundant package aliases from import blocks Suppose that: - there were a package example.com/foo/bar whose package name were bar, and - that this package contained an export `Bar` of type `func(string)`. Then ```go package main import ( bar "example.com/foo/bar" ) func main() { bar.Bar("Hello, world!") } ``` contains some unhelpful redundancy: the package alias, bar, adds nothing. This commit adds a stylistic check, ST1024, that should catch these kinds of cases. An exception is carved out for those cases where the last component of the package path differs from the package name. An example of this is github.com/google/gofuzz whose package name is fuzz, not gofuzz. --- stylecheck/analysis.go | 2 + stylecheck/st1024/st1024.go | 83 +++++++++++++++++++ stylecheck/st1024/st1024_test.go | 13 +++ .../CheckRedundantAliases.go | 17 ++++ .../src/example.com/diffname/diffname.go | 4 + .../src/example.com/samename/samename.go | 4 + 6 files changed, 123 insertions(+) create mode 100644 stylecheck/st1024/st1024.go create mode 100644 stylecheck/st1024/st1024_test.go create mode 100644 stylecheck/st1024/testdata/src/example.com/CheckRedundantAliases/CheckRedundantAliases.go create mode 100644 stylecheck/st1024/testdata/src/example.com/diffname/diffname.go create mode 100644 stylecheck/st1024/testdata/src/example.com/samename/samename.go diff --git a/stylecheck/analysis.go b/stylecheck/analysis.go index 7171befe2..b613e0a31 100644 --- a/stylecheck/analysis.go +++ b/stylecheck/analysis.go @@ -22,6 +22,7 @@ import ( "honnef.co/go/tools/stylecheck/st1021" "honnef.co/go/tools/stylecheck/st1022" "honnef.co/go/tools/stylecheck/st1023" + "honnef.co/go/tools/stylecheck/st1024" ) var Analyzers = []*lint.Analyzer{ @@ -43,4 +44,5 @@ var Analyzers = []*lint.Analyzer{ st1021.SCAnalyzer, st1022.SCAnalyzer, st1023.SCAnalyzer, + st1024.SCAnalyzer, } diff --git a/stylecheck/st1024/st1024.go b/stylecheck/st1024/st1024.go new file mode 100644 index 000000000..21b17b2a0 --- /dev/null +++ b/stylecheck/st1024/st1024.go @@ -0,0 +1,83 @@ +package st1024 + +import ( + "fmt" + "strconv" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/packages" + + "honnef.co/go/tools/analysis/facts/generated" + "honnef.co/go/tools/analysis/lint" + "honnef.co/go/tools/analysis/report" +) + +var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{ + Analyzer: &analysis.Analyzer{ + Name: "ST1024", + Run: run, + Requires: []*analysis.Analyzer{generated.Analyzer}, + }, + Doc: &lint.Documentation{ + Title: "Package aliases should not be redundant", + Text: `An alias should be a non-trivial renaming of what it aliases. + +For example, there's no benefit from using "bar" as an alias for a package +named "bar" whose package path is "example.com/foo/bar". We can just use +its name.`, + MergeIf: lint.MergeIfAny, + }, +}) + +var Analyzer = SCAnalyzer.Analyzer + +func run(pass *analysis.Pass) (interface{}, error) { + // To avoid loading the same packages repeatedly, we construct a map of the + // package path to the package pointer outside of the loops. + pathToPackage := make(map[string]*packages.Package) + + for _, f := range pass.Files { + for _, imp := range f.Imports { + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + return nil, fmt.Errorf("unquoting the package path %q: %w", imp.Path.Value, err) + } + + var pkgName string + pkg, ok := pathToPackage[path] + if !ok { + pkgsLoaded, err := packages.Load(&packages.Config{Mode: packages.NeedName}, path) + if err != nil || len(pkgsLoaded) != 1 { + return nil, fmt.Errorf("loading packages: %w", err) + } + pkg = pkgsLoaded[0] + pathToPackage[path] = pkg + } + pkgName = pkg.Name + + // Is there a package alias? Is it different from the package name? + if imp.Name == nil || imp.Name.Name != pkgName { + continue + } + + // We compare against the last component of the package as it's + // stylistically reasonable to use a package alias when the package name + // and the last component of its path differ. For example, the package + // name of github.com/google/gofuzz is "fuzz"; explicitly importing it as + // "fuzz" is reasonable. + lastPkgComponent := path + if idx := strings.LastIndexByte(lastPkgComponent, '/'); idx != -1 { + lastPkgComponent = lastPkgComponent[idx+1:] + } + if pkgName != lastPkgComponent { + continue + } + + opts := []report.Option{report.FilterGenerated()} + report.Report(pass, imp, fmt.Sprintf("package %q is imported with a redundant alias", path), opts...) + } + } + + return nil, nil +} diff --git a/stylecheck/st1024/st1024_test.go b/stylecheck/st1024/st1024_test.go new file mode 100644 index 000000000..25948e38a --- /dev/null +++ b/stylecheck/st1024/st1024_test.go @@ -0,0 +1,13 @@ +// Code generated by generate.go. DO NOT EDIT. + +package st1024 + +import ( + "testing" + + "honnef.co/go/tools/analysis/lint/testutil" +) + +func TestTestdata(t *testing.T) { + testutil.Run(t, SCAnalyzer) +} diff --git a/stylecheck/st1024/testdata/src/example.com/CheckRedundantAliases/CheckRedundantAliases.go b/stylecheck/st1024/testdata/src/example.com/CheckRedundantAliases/CheckRedundantAliases.go new file mode 100644 index 000000000..e94831edc --- /dev/null +++ b/stylecheck/st1024/testdata/src/example.com/CheckRedundantAliases/CheckRedundantAliases.go @@ -0,0 +1,17 @@ +// Package pkg ... +package pkg + +import ( + fmt "fmt" //@ diag(`package "fmt" is imported with a redundant alias`) + "math" + + adiffname "example.com/diffname" + samename "example.com/samename" //@ diag(`package "example.com/samename" is imported with a redundant alias`) +) + +var ( + _ = fmt.Println + _ = math.E + _ = adiffname.I + _ = samename.I +) diff --git a/stylecheck/st1024/testdata/src/example.com/diffname/diffname.go b/stylecheck/st1024/testdata/src/example.com/diffname/diffname.go new file mode 100644 index 000000000..f48de5588 --- /dev/null +++ b/stylecheck/st1024/testdata/src/example.com/diffname/diffname.go @@ -0,0 +1,4 @@ +// Package adiffname ... +package adiffname + +var I int = 0 diff --git a/stylecheck/st1024/testdata/src/example.com/samename/samename.go b/stylecheck/st1024/testdata/src/example.com/samename/samename.go new file mode 100644 index 000000000..a567b365f --- /dev/null +++ b/stylecheck/st1024/testdata/src/example.com/samename/samename.go @@ -0,0 +1,4 @@ +// Package samename ... +package samename + +var I int = 0