Circular dependencies when including mocks generated by gomock in go_library

[ramenjosh]

What version of rules_go are you using?

0.34.0

What version of gazelle are you using?

0.26.0

What version of Bazel are you using?

5.1.1

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

MacOS x86_64

Any other potentially useful information about your toolchain?

N/A

What did you do?

I'm attempting to generate mocks using gomock and include those mocks as part of a go_library, such that other packages within my codebase can consume those mocks as part of their test cases. I'm using the source mode for gomock, which should only require passing in a single file to generate mocks from as opposed to a full library. Simplest reproducible case:

package client

import (
	"google.golang.org/genproto/googleapis/bytestream"
	"google.golang.org/grpc"
)

type Client interface {
	Connect(grpc.ClientConnInterface) *bytestream.ByteStreamClient
}

With BUILD file:

load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "gomock")

# For this test, the mock is included as part of the library
go_library(
    name = "client",
    srcs = [
        "client.go",
        "client_mock.go",
    ],
    importpath = "github.com/bazelbuild/rules_go/gomock/client",
    visibility = ["//visibility:public"],
    deps = [
        "@go_googleapis//google/bytestream:bytestream_go_proto",
        "@org_golang_google_grpc//:grpc",
        "@com_github_golang_mock//gomock",
    ],
)

gomock(
    name = "mocks",
    out = "client_mock.go",
    library = ":client",
    package = "client",
    source = "client.go",
    visibility = ["//visibility:public"],
)

What did you expect to see?

Build should pass.

What did you see instead?

A circular dependency occurs:

joshsmith@Joshs-MacBook-Pro rules_go % bazel test //tests/extras/gomock/...
ERROR: /Users/joshsmith/work/rules_go/tests/extras/gomock/source_with_importpath/BUILD.bazel:4:11: in go_library rule //tests/extras/gomock/source_with_importpath:client: cycle in dependency graph:
.-> //tests/extras/gomock/source_with_importpath:client (c75b52840eea6dcbe1b8559af2885f9d37de38614db15e782e3b011d421f690b)
|   //tests/extras/gomock/source_with_importpath:client_mock.go (c75b52840eea6dcbe1b8559af2885f9d37de38614db15e782e3b011d421f690b)
|   //tests/extras/gomock/source_with_importpath:mocks (c75b52840eea6dcbe1b8559af2885f9d37de38614db15e782e3b011d421f690b)
`-- //tests/extras/gomock/source_with_importpath:client (c75b52840eea6dcbe1b8559af2885f9d37de38614db15e782e3b011d421f690b)
ERROR: Analysis of target '//tests/extras/gomock/source_with_importpath:client' failed; build aborted
INFO: Elapsed time: 0.386s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded, 2 targets configured)

I've had a look through the source code and can see that the only reason a go_library is required is for it's importpath:
https://github.com/bazelbuild/rules_go/blob/2113d18ca1817e8529d7fa44e40af2742f6b2633/extras/gomock.bzl#L38

Patching the rule to allow passing an importpath directly bypasses this isssue, and also seems more in line with how source mode is intended to be used. This looks like the following:

go_library(
    name = "client",
    srcs = [
        "client.go",
        "client_mock.go",
    ],
    importpath = "github.com/bazelbuild/rules_go/gomock/client",
    visibility = ["//visibility:public"],
    deps = [
        "@go_googleapis//google/bytestream:bytestream_go_proto",
        "@org_golang_google_grpc//:grpc",
        "@com_github_golang_mock//gomock",
    ],
)

gomock(
    name = "mocks",
    out = "client_mock.go",
    importpath = "github.com/bazelbuild/rules_go/gomock/client",
    package = "client",
    source = "client.go",
    visibility = ["//visibility:public"],
)

[fmeum]

@linzhp I have also wondered about why gomock depends on a library before. Is it really just for the import path?

[linzhp]

Yeah. It was designed that way so the reflective mode and source mode can share the same argument.

[jmhodges]

I think this is a change from when gomock was a separate project. When I moved to using the contrib one, I've had to add a self_package parameter to the gomock call with the import path of the library. Def seems like a bug