x/tools/cmd/goimports: prefer main module requirements

myitcv commented 5 years ago

What version of Go are you using (`go version`)?

$ go version
go version go1.12.1 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build898038110=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Ran testscript on the following:

# install goimports
env GOMODPROXY=$GOPROXY
env GOPROXY=
go install golang.org/x/tools/cmd/goimports

# switch back to our local proxy
env GOPROXY=$GOMODPROXY

# "warm" the module (download) cache
go get gopkg.in/tomb.v1
go get gopkg.in/tomb.v2

# test goimports
cd mod
go mod tidy
exec goimports main.go
stdout '\Q"gopkg.in/tomb.v2"\E'

-- go.mod --
module goimports

require golang.org/x/tools v0.0.0-20190322203728-c1a832b0ad89

-- mod/go.mod --
module mod

require gopkg.in/tomb.v2 v2.0.0

-- mod/main.go --
package main

import (
    "fmt"
)

func main() {
    fmt.Println(tomb.Tomb)
}

-- .gomodproxy/gopkg.in_tomb.v1_v1.0.0/.mod --
module gopkg.in/tomb.v1

-- .gomodproxy/gopkg.in_tomb.v1_v1.0.0/.info --
{"Version":"v1.0.0","Time":"2018-10-22T18:45:39Z"}

-- .gomodproxy/gopkg.in_tomb.v1_v1.0.0/go.mod --
module gopkg.in/tomb.v1

-- .gomodproxy/gopkg.in_tomb.v1_v1.0.0/tomb.go --
package tomb

const Tomb = "A great package v1"

-- .gomodproxy/gopkg.in_tomb.v2_v2.0.0/.mod --
module gopkg.in/tomb.v2

-- .gomodproxy/gopkg.in_tomb.v2_v2.0.0/.info --
{"Version":"v2.0.0","Time":"2018-10-22T18:45:39Z"}

-- .gomodproxy/gopkg.in_tomb.v2_v2.0.0/go.mod --
module gopkg.in/tomb.v2

-- .gomodproxy/gopkg.in_tomb.v2_v2.0.0/tomb.go --
package tomb

const Tomb = "A great package v2"

What did you expect to see?

A passing run.

What did you see instead?

A failed run.

goimports should be "consulting" the main module for matches before dropping down to a module cache-based search. Here that would have resulted in gopkg.in/tomb.v2 being correctly resolved, instead of gopkg.in/tomb.v1.

This will, I suspect, also massively improve the speed of goimports in a large majority of cases.

cc @heschik

heschi commented 5 years ago

@ianthehat

This would be fairly easy to do as a separate pass. I don't think it would make a huge performance difference for most people, but maybe if you have a gigantic module cache or are looking for a very generic package like "util" or something. For me, the stronger argument is just that we should be biased against adding new deps when an existing dep suffices.

DeedleFake commented 2 years ago

Any progress on this? It's a constant thorn in my side when dealing with any dependency that's v2 or up.

I don't think it would make a huge performance difference for most people

It might actually be quite a bit faster, as I think that 99% of the time you're looking for a package that's in one of the dependencies in go.mod.

golang / go