(How do I run the repro? I'm slightly familiar with the test script language, but this thing where it's split into three files is new to me.)

Even without running the repro I think I understand what's happening. When doing a name= query, go/packages makes a temporary module to run go list in, and adds the things it finds in the module cache as dependencies of that temporary module. If the thing it found is the target of a replace directive, then it can't be used directly as a dependency.

Nasty bug. I'm not sure how to fix it. In this particular case, goimports could look at the module it's running in, notice that banana is mentioned in a replace directive, and skip it. But we won't always be running in the right module to see that. The only thing I can think of is to manually parse the module file and skip mismatches. Yuck.

@bcmills, @jayconrod, is there any chance that with -e this could be in the JSON rather than a fatal error?

How do I run the repro?

I'm just writing a simple rig for running arbitrary testscript tests (with goproxytest mod files).

Even without running the repro I think I understand what's happening

Isn't the logic you want here to only add a directory (for which the directory path does not equal the module path) if that directory is "in use" as the target of a replace, else don't add such mismatched directories?

PS this bug is now killing me because my module cache contains a number of such replace targets where the directory path does not match the module path.

for which the directory path does not equal the module path

Yeah. That's what I meant by manually parsing the module file and skipping mismatches. Sigh. I guess I can copy cmd/go/internal/modfile and its deps into x/tools.

I'll try to get to this early next week after I've talked with @ianthehat. If it's convenient, pushing the repro to GitHub would save me a little time -- I need something go mod download-able to build the test case.

@heschik - apologies, haven't had a chance to look at this yet.

Just to also flag, goimports also appears to do the wrong thing with directory paths that have been encoded. I'll try and add a repro for that too.

I need something go mod download-able to build the test case.

It should be possible to get what we need from these testscript repros, i.e. set GOPATH to something known so that we can then use the pkg/mod/cache/download directory afterwards; it's just much easier not to have to create separate GitHub repos for all this stuff.

Good point on encoding.

I'm afraid plans have changed and I've got some other goimports work to do so this fix will be delayed. But I'll get to it as part of that work or ASAP afterward.

The only thing I can think of is to manually parse the module file and skip mismatches.

Note that there is an existing function in cmd/go/internal/modfile that parses only the module line.

@bcmills, @jayconrod, is there any chance that with -e this could be in the JSON rather than a fatal error?

Yes, that's arguably what it ought to do. Might not be feasible until 1.13, though.

I updated @subash-a's description of this issue to include a fully reproducible script.

@heschik - as promised, here's a repro of the related path encoding bug:

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

# test goimports
env GOPROXY=$GOMODPROXY
cd mod
go mod download github.com/Fruit/apple@v1.0.0
exec goimports -l main.go
stdout main.go

-- go.mod --
module goimports

require golang.org/x/tools v0.0.0-20181221235234-d00ac6d27372

-- mod/go.mod --
module mod

-- mod/main.go --
package main

import (
    "fmt"

    // we are missing an import to satisfy core
)

func main() {
    fmt.Println(core.Orange)
}
-- mod/core/core.go --
package core

type T struct{}
var Orange T

-- .gomodproxy/github.com_!fruit_apple_v1.0.0/.mod --
module fruit.com

-- .gomodproxy/github.com_!fruit_apple_v1.0.0/.info --
{"Version":"v1.0.0","Time":"2018-10-22T18:45:39Z"}

-- .gomodproxy/github.com_!fruit_apple_v1.0.0/go.mod --
module fruit.com

-- .gomodproxy/github.com_!fruit_apple_v1.0.0/fruit/fruit.go --
package fruit

const Apple = "apple"
-- .gomodproxy/github.com_!fruit_apple_v1.0.0/coretest/coretest.go --
// package coretest becomes a candidate for the missing
// core import in main above
package coretest

const Mandarin = "mandarin"

which should pass but gives:

$ testscript goimports_encoded_paths_walking.txt

# install goimports (4.603s)
# test goimports (0.222s)
> env GOPROXY=$GOMODPROXY
> cd mod
$WORK/mod
> go mod download github.com/Fruit/apple@v1.0.0
[stderr]
go: finding github.com/Fruit/apple v1.0.0

> exec goimports -l main.go
[stderr]
querying module cache matches: go [list -e -json -compiled -test=false -export=false -deps=false -find=true -- github.com/!fruit/apple/coretest golang.org/x/tools/cmd/heapview/internal/core]: exit status 1: go: github.com/!fruit/apple@v1.0.0: malformed module path "github.com/!fruit/apple": invalid char '!'
go: error loading module requirements

[exit status 2]
FAIL: /tmp/testscript247052820/0/script.txt:10: unexpected command failure
error running goimports_encoded_paths_walking.txt in /tmp/testscript247052820/0

Thanks for the repros. Both of them pass with https://golang.org/cl/158097, and I believe the test cases I copied from cmd/go cover these cases.

That CL is submitted, so this should be fixed. Let me know if not.