Repository: golang/mock
Pull request #443 (2020-06-05 19:37:22)

gomock/controller: use skip additional frame
<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

**Description**
In 832173191abead7f0878d8bb24f32f1a5fdf6d79, the callerInfo call
gets nested within an additional function call, but the frame skip
isn't updated. When used with mockgen, this causes the location frame
to unhelpfully point to the generated mock instead of the callsite in
the user's test.

Before:
```
... at .../mocks/mock_ecriface/mock_ECRAPI.go:612 because: there are no expected calls of...
```

After:
```
... at .../example_test.go:29 because: there are no expected calls of...
```

**Submitter Checklist**

These are the criteria that every PR should meet, please check them off as you
review them:

- [ ] Includes tests

**Reviewer Notes**

- [ ] The code flow looks good.
- [ ] Tests added.


**Release Notes**

Describe any changes here so maintainer can include it in the release notes, or delete this block.

```
- Fixes blame location for unexpected calls to mock
```


googlebot: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgolang%2Fmock%2Fpull%2F443) for more info**.

<!-- need_author_cla -->
googlebot: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgolang%2Fmock%2Fpull%2F443) for more info**.

<!-- need_author_cla -->
googlebot: CLAs look good, thanks!

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgolang%2Fmock%2Fpull%2F443) for more info**.

<!-- ok -->
googlebot: CLAs look good, thanks!

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgolang%2Fmock%2Fpull%2F443) for more info**.

<!-- ok -->

Patch:
diff --git a/gomock/call.go b/gomock/call.go
index 7345f65..49d3584 100644
--- a/gomock/call.go
+++ b/gomock/call.go
@@ -63,6 +63,9 @@ func newCall(t TestHelper, receiver interface{}, method string, methodType refle
 		}
 	}
 
+	// callerInfo's skip should be updated if the number of calls between the user's test
+	// and this line changes, i.e. this code is wrapped in another anonymous function.
+	// 0 is us, 1 is RecordCallWithMethodType(), 2 is the generated recorder, and 3 is the user's test.
 	origin := callerInfo(3)
 	actions := []func([]interface{}) []interface{}{func([]interface{}) []interface{} {
 		// Synthesize the zero value for each of the return args' types.
diff --git a/gomock/controller.go b/gomock/controller.go
index 0a8366b..ee175cd 100644
--- a/gomock/controller.go
+++ b/gomock/controller.go
@@ -224,7 +224,10 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf
 
 		expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args)
 		if err != nil {
-			origin := callerInfo(2)
+			// callerInfo's skip should be updated if the number of calls between the user's test
+			// and this line changes, i.e. this code is wrapped in another anonymous function.
+			// 0 is us, 1 is controller.Call(), 2 is the generated mock, and 3 is the user's test.
+			origin := callerInfo(3)
 			ctrl.T.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err)
 		}
 
@@ -291,6 +294,8 @@ func (ctrl *Controller) Finish() {
 	}
 }
 
+// callerInfo returns the file:line of the call site. skip is the number
+// of stack frames to skip when reporting. 0 is callerInfo's call site.
 func callerInfo(skip int) string {
 	if _, file, line, ok := runtime.Caller(skip + 1); ok {
 		return fmt.Sprintf("%s:%d", file, line)
diff --git a/mockgen/parse.go b/mockgen/parse.go
index 8ceed12..2bc503b 100644
--- a/mockgen/parse.go
+++ b/mockgen/parse.go
@@ -641,13 +641,16 @@ func parsePackageImport(srcDir string) (string, error) {
 		}
 	}
 	// fall back to GOPATH mode
-	goPath := os.Getenv("GOPATH")
-	if goPath == "" {
+	goPaths := os.Getenv("GOPATH")
+	if goPaths == "" {
 		return "", fmt.Errorf("GOPATH is not set")
 	}
-	sourceRoot := filepath.Join(goPath, "src") + string(os.PathSeparator)
-	if !strings.HasPrefix(srcDir, sourceRoot) {
-		return "", errOutsideGoPath
+	goPathList := strings.Split(goPaths, string(os.PathListSeparator))
+	for _, goPath := range goPathList {
+		sourceRoot := filepath.Join(goPath, "src") + string(os.PathSeparator)
+		if strings.HasPrefix(srcDir, sourceRoot) {
+			return filepath.ToSlash(strings.TrimPrefix(srcDir, sourceRoot)), nil
+		}
 	}
-	return filepath.ToSlash(strings.TrimPrefix(srcDir, sourceRoot)), nil
+	return "", errOutsideGoPath
 }


Test patch:
diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go
index a274d8e..28fb5d5 100644
--- a/mockgen/parse_test.go
+++ b/mockgen/parse_test.go
@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -202,3 +203,45 @@ func TestParsePackageImport_FallbackGoPath(t *testing.T) {
 		t.Errorf("expect %s, got %s", expected, pkgPath)
 	}
 }
+
+func TestParsePackageImport_FallbackMultiGoPath(t *testing.T) {
+	var goPathList []string
+
+	// first gopath
+	goPath, err := ioutil.TempDir("", "gopath1")
+	if err != nil {
+		t.Error(err)
+	}
+	goPathList = append(goPathList, goPath)
+	defer func() {
+		if err = os.RemoveAll(goPath); err != nil {
+			t.Error(err)
+		}
+	}()
+	srcDir := filepath.Join(goPath, "src/example.com/foo")
+	err = os.MkdirAll(srcDir, 0755)
+	if err != nil {
+		t.Error(err)
+	}
+
+	// second gopath
+	goPath, err = ioutil.TempDir("", "gopath2")
+	if err != nil {
+		t.Error(err)
+	}
+	goPathList = append(goPathList, goPath)
+	defer func() {
+		if err = os.RemoveAll(goPath); err != nil {
+			t.Error(err)
+		}
+	}()
+
+	goPaths := strings.Join(goPathList, string(os.PathListSeparator))
+	os.Setenv("GOPATH", goPaths)
+	os.Setenv("GO111MODULE", "on")
+	pkgPath, err := parsePackageImport(srcDir)
+	expected := "example.com/foo"
+	if pkgPath != expected {
+		t.Errorf("expect %s, got %s", expected, pkgPath)
+	}
+}