Open candiduslynx opened 8 months ago
Hi @candiduslynx, thank you for reporting the issue.
The only viable solution for that seems to be using
import "golang.org/x/tools/go/packages"
// ...
// Load the package information
pkgs, err := packages.Load(cfg, importPath)
if err != nil {
return "", fmt.Errorf("Error loading package: %w", err)
}
// Ensure that the package information is available
if len(pkgs) == 0 || pkgs[0].Name == "" {
return "", fmt.Errorf("Package information not found for import path: %s", importPath)
}
return pkgs[0].Name, nil // <-- here we'll return `run` for https://pkg.go.dev/cloud.google.com/go/run/apiv2
The problem of this solution is that package.Load uses exec.Command
internally. And thus it's very slow on big code bases. For such cases I'd use importas
linter, where you can strictly define an alias (or an absence of such). I'm not sure if revive
has a built-in alternative.
Another possible solution would be to load only package name when traversing the vendored imports, but this will also require an additional effort:
There are two ways of having the deps: vendor
dir or in $GOPATH/pkg/mod
. This would mean that the tool will have to understand, which mode is used in the project. While technically possible (first check for vendor
dir, then - if not found - look in $GOPATH/pkg/mod
), that would require the rule to go way beyond what other rules do. It will also mean that the rule will have to parse the first found non-test file using "go/parser"
(this will slow down the processing as well, I guess). I'm not sure if it's a good idea.
IMO this check just needs to be retired, as it's not usable with some major SDKs.
Or, at the very least, the package name should be determined by an updated process (it will still be a hack and won't work for the example in the issue description, where the package name doesn't match the directory name):
v%d
v%d
, go to the next oneI'm not entirely sure what to do with
import apiv2 "cloud.google.com/go/run/apiv2"
Note that the actual package name can be v1: https://pkg.go.dev/k8s.io/api/core/v1.
Note that this linter directly conflicts with what goimports does for packages like the one denisvmedia posted above.
if the package name is literally v1, then goimports will change
import "github.com/foo/bar/v1"
to
import v1 "gitub.com/foo/bar/v1"
...and then revive will tell you that's redundant.
Now, I don't 100% agree with goimports aliasing a thing to the same name... and I think it's generally dumb to call a package literally v1. BUT, if I have to pick who has to comply with who, I gotta say that revive should not call anything that goimports does "incorrect".
Describe the bug
When checking
redundant-import-alias
the code will only look at the import path, whereas the proper way would've been to check the imported package name. I've seen this withimport apiv2 "cloud.google.com/go/run/apiv2"
(the code is underapiv2
directory, but the package name isrun
: https://pkg.go.dev/cloud.google.com/go/run/apiv2)To Reproduce Steps to reproduce the behavior:
Create a package in a dir named
redundant
with the following content (note that the package name must be different):import redundant "path/to/redundant"
Expected behavior No issue is presented, as the alias is used to override the package name, not the path.
Logs
This is printed when importing: