Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal (ST1024): Add a check for redundant package aliases in import blocks #1497

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamroyjones
Copy link

@adamroyjones adamroyjones commented Feb 14, 2024

This pull request contains a (draft) working implementation of a proposal. There's a section ("Notes") that identifies an issue with this branch and raises a question about my implementation.

  • Description
    • A full working example
  • Notes

Description

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

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 branch adds a (proposed) 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.

A full working example

For example, the below triggers the proposed check.

// go.mod
module example.com

go 1.22.0
// main.go
package main

import (
	foo "example.com/foo"
)

func main() {
	foo.Foo("hello, world!")
}
// foo/foo.go
package foo

import (
	"fmt"
)

func Foo(s string) {
	fmt.Printf("foo.Foo: %s\n", s)
}

Notes

Firstly, this is adapted from mvdan/gofumpt#296, where the impact on performance and the change of scope ruled it out.

Secondly, I created a failing test on this branch. After scratching my head for a little while I'm not sure how to resolve it. The packages.Load call of example.com/samename returns a *package.Package with an Errors field that contains

no required module provides package example.com/samename; to add it:
	go get example.com/samename

I'm guessing it's because of

Env: append(os.Environ(), "GOPATH="+testdata, "GO111MODULE=off", "GOPROXY=off"),

but I'm not sure what to do. The check works in my local testing with modules.

Finally, I don't know if the package data are already somehow accessible (that is, whether these calls to packages.Load are wasteful). I'm a little at-sea in this codebase—at least for now.

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.
@adamroyjones adamroyjones changed the title proposal (ST1024): Add a check for redundant package aliases from import blocks proposal (ST1024): Add a check for redundant package aliases in import blocks Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant