Extend gomock to allow passing an source_importpath instead of librar…

…y when operating in source mode (#3822)


Co-authored-by: Josh Smith <[email protected]>

README.rst

@@ -54,6 +54,7 @@ Go rules for Bazel_
 .. _go_cross_binary: docs/go/core/rules.md#go_cross_binary
 .. _go_toolchain: go/toolchains.rst#go_toolchain
 .. _go_wrap_sdk: go/toolchains.rst#go_wrap_sdk
+.. _gomock: docs/go/extras/extras.md#gomock
 
 .. External rules
 .. _git_repository: https://docs.bazel.build/versions/master/repo/git.html
@@ -209,6 +210,7 @@ Documentation
   * `go_context`_
 
 * `Extra rules <docs/go/extras/extras.md>`_
+  * `gomock`_
 
 * `nogo build-time static analysis`_
 * `Build modes <go/modes.rst>`_

docs/go/extras/extras.md

@@ -33,8 +33,8 @@ This rule has moved. See [gazelle rule] in the Gazelle repository.
 ## gomock
 
 <pre>
-gomock(<a href="#gomock-name">name</a>, <a href="#gomock-library">library</a>, <a href="#gomock-out">out</a>, <a href="#gomock-source">source</a>, <a href="#gomock-interfaces">interfaces</a>, <a href="#gomock-package">package</a>, <a href="#gomock-self_package">self_package</a>, <a href="#gomock-aux_files">aux_files</a>, <a href="#gomock-mockgen_tool">mockgen_tool</a>,
-       <a href="#gomock-imports">imports</a>, <a href="#gomock-copyright_file">copyright_file</a>, <a href="#gomock-mock_names">mock_names</a>, <a href="#gomock-kwargs">kwargs</a>)
+gomock(<a href="#gomock-name">name</a>, <a href="#gomock-out">out</a>, <a href="#gomock-library">library</a>, <a href="#gomock-source_importpath">source_importpath</a>, <a href="#gomock-source">source</a>, <a href="#gomock-interfaces">interfaces</a>, <a href="#gomock-package">package</a>, <a href="#gomock-self_package">self_package</a>, <a href="#gomock-aux_files">aux_files</a>,
+       <a href="#gomock-mockgen_tool">mockgen_tool</a>, <a href="#gomock-imports">imports</a>, <a href="#gomock-copyright_file">copyright_file</a>, <a href="#gomock-mock_names">mock_names</a>, <a href="#gomock-kwargs">kwargs</a>)
 </pre>
 
 Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library.
@@ -48,8 +48,9 @@ If `source` is given, the mocks are generated in source mode; otherwise in refle
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="gomock-name"></a>name |  the target name.   |  none |
-| <a id="gomock-library"></a>library |  the Go library to took for the interfaces (reflecitve mode) or source (source mode).   |  none |
 | <a id="gomock-out"></a>out |  the output Go file name.   |  none |
+| <a id="gomock-library"></a>library |  the Go library to look into for the interfaces (reflective mode) or source (source mode). If running in source mode, you can specify source_importpath instead of this parameter.   |  <code>None</code> |
+| <a id="gomock-source_importpath"></a>source_importpath |  the importpath for the source file. Alternative to passing library, which can lead to circular dependencies between mock and library targets. Only valid for source mode.   |  <code>""</code> |
 | <a id="gomock-source"></a>source |  a Go file in the given <code>library</code>. If this is given, <code>gomock</code> will call mockgen in source mode to mock all interfaces in the file.   |  <code>None</code> |
 | <a id="gomock-interfaces"></a>interfaces |  a list of interfaces in the given <code>library</code> to be mocked in reflective mode.   |  <code>[]</code> |
 | <a id="gomock-package"></a>package |  the name of the package the generated mocks should be in. If not specified, uses mockgen's default. See [mockgen's -package](https://github.com/golang/mock#flags) for more information.   |  <code>""</code> |

extras/gomock.bzl

@@ -34,10 +34,17 @@ _MOCKGEN_MODEL_LIB = Label("//extras/gomock:mockgen_model")
 def _gomock_source_impl(ctx):
     go_ctx = go_context(ctx)
 
+    # In Source mode, it's not necessary to pass through a library, as the only thing we use it for is setting up
+    # the relative file locations. Forcing users to pass a library makes it difficult in the case where a mock should
+    # be included as part of that same library, as it results in a dependency loop (GoMock -> GoLibrary -> GoMock).
+    # Allowing users to pass an importpath directly bypasses this issue.
+    # See the test case in //tests/extras/gomock/source_with_importpath for an example.
+    importpath = ctx.attr.source_importpath if ctx.attr.source_importpath else ctx.attr.library[GoLibrary].importmap
+
     # create GOPATH and copy source into GOPATH
     go_path_prefix = "gopath"
-    source_relative_path = paths.join("src", ctx.attr.library[GoLibrary].importmap, ctx.file.source.basename)
-    source = ctx.actions.declare_file(paths.join(go_path_prefix, source_relative_path))
+    source_relative_path = paths.join("src", importpath, ctx.file.source.basename)
+    source = ctx.actions.declare_file(paths.join("gopath", source_relative_path))
 
     # trim the relative path of source to get GOPATH
     gopath = source.path[:-len(source_relative_path)]
@@ -107,7 +114,11 @@ _gomock_source = rule(
         "library": attr.label(
             doc = "The target the Go library where this source file belongs",
             providers = [GoLibrary],
-            mandatory = True,
+            mandatory = False,
+        ),
+        "source_importpath": attr.string(
+            doc = "The importpath for the source file. Alternative to passing library, which can lead to circular dependencies between mock and library targets.",
+            mandatory = False,
         ),
         "source": attr.label(
             doc = "A Go source file to find all the interfaces to generate mocks for. See also the docs for library.",
@@ -156,15 +167,16 @@ _gomock_source = rule(
     toolchains = [GO_TOOLCHAIN],
 )
 
-def gomock(name, library, out, source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, **kwargs):
+def gomock(name, out, library = None, source_importpath = "", source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, **kwargs):
     """Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library.
 
     If `source` is given, the mocks are generated in source mode; otherwise in reflective mode.
 
     Args:
         name: the target name.
-        library: the Go library to took for the interfaces (reflecitve mode) or source (source mode).
         out: the output Go file name.
+        library: the Go library to look into for the interfaces (reflective mode) or source (source mode). If running in source mode, you can specify source_importpath instead of this parameter.
+        source_importpath: the importpath for the source file. Alternative to passing library, which can lead to circular dependencies between mock and library targets. Only valid for source mode.
         source: a Go file in the given `library`. If this is given, `gomock` will call mockgen in source mode to mock all interfaces in the file.
         interfaces: a list of interfaces in the given `library` to be mocked in reflective mode.
         package: the name of the package the generated mocks should be in. If not specified, uses mockgen's default. See [mockgen's -package](https://github.com/golang/mock#flags) for more information.
@@ -179,8 +191,9 @@ def gomock(name, library, out, source = None, interfaces = [], package = "", sel
     if source:
         _gomock_source(
             name = name,
-            library = library,
             out = out,
+            library = library,
+            source_importpath = source_importpath,
             source = source,
             package = package,
             self_package = self_package,
@@ -194,8 +207,8 @@ def gomock(name, library, out, source = None, interfaces = [], package = "", sel
     else:
         _gomock_reflect(
             name = name,
-            library = library,
             out = out,
+            library = library,
             interfaces = interfaces,
             package = package,
             self_package = self_package,

tests/extras/gomock/README.rst

@@ -0,0 +1,17 @@
+gomock
+=====================
+
+Tests that ensure the gomock rules can be called correctly under different input permutations.
+
+reflective
+------------------------
+Checks that gomock can be run in "reflective" mode when passed a `GoLibrary` and `interfaces`.
+
+source
+------------------------
+Checks that gomock can be run in "source" mode when passed a `GoLibrary` and `source`.
+
+source_with_importpath
+------------------------
+Checks that gomock can be run in "source" mode when passed an `importpath` and `source`.
+This test case also demonstrates the circumstance in which `importpath` is necessary to prevent a circular dependency.

tests/extras/gomock/reflective/BUILD.bazel

@@ -0,0 +1,34 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "gomock")
+
+go_library(
+    name = "client",
+    srcs = [
+        "client.go",
+    ],
+    importpath = "github.com/bazelbuild/rules_go/gomock/client",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_google_genproto//googleapis/bytestream",
+        "@org_golang_google_grpc//:grpc",
+    ],
+)
+
+# Build the mocks using reflective mode (i.e. without passing source)
+gomock(
+    name = "mocks",
+    out = "client_mock.go",
+    library = ":client",
+    package = "client",
+    interfaces = ["Client"],
+    visibility = ["//visibility:public"],
+)
+
+go_test(
+    name = "client_test",
+    srcs = [
+        "client_mock.go",
+        "client_test.go",
+    ],
+    embed = [":client"],
+    deps = ["@com_github_golang_mock//gomock"],
+)

tests/extras/gomock/client_test.go → tests/extras/gomock/reflective/client_test.go

@@ -1,4 +1,3 @@
 package client
 
 var _ Client = (*MockClient)(nil)
-var _ ClientWrapper = (*MockClientWrapper)(nil)

tests/extras/gomock/source/client.go

@@ -0,0 +1,10 @@
+package client
+
+import (
+	"google.golang.org/genproto/googleapis/bytestream"
+	"google.golang.org/grpc"
+)
+
+type Client interface {
+	Connect(grpc.ClientConnInterface) *bytestream.ByteStreamClient
+}

tests/extras/gomock/source/client_test.go

@@ -0,0 +1,3 @@
+package client
+
+var _ Client = (*MockClient)(nil)

tests/extras/gomock/source_with_importpath/BUILD.bazel

@@ -0,0 +1,37 @@
+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 = [
+        "@org_golang_google_genproto//googleapis/bytestream",
+        "@org_golang_google_grpc//:grpc",
+        "@com_github_golang_mock//gomock",
+    ],
+)
+
+# Pass importpath instead of library to the generation step
+# Passing library instead of importpath here will cause a circular dependency
+gomock(
+    name = "mocks",
+    out = "client_mock.go",
+    source_importpath = "github.com/bazelbuild/rules_go/gomock/client",
+    package = "client",
+    source = "client.go",
+    visibility = ["//visibility:public"],
+)
+
+# Don't include client_mock.go as a source file, instead use it from the library
+go_test(
+    name = "client_test",
+    srcs = [
+        "client_test.go",
+    ],
+    embed = [":client"],
+)

tests/extras/gomock/source_with_importpath/client.go

@@ -0,0 +1,10 @@
+package client
+
+import (
+	"google.golang.org/genproto/googleapis/bytestream"
+	"google.golang.org/grpc"
+)
+
+type Client interface {
+	Connect(grpc.ClientConnInterface) *bytestream.ByteStreamClient
+}

tests/extras/gomock/source_with_importpath/client_test.go

@@ -0,0 +1,3 @@
+package client
+
+var _ Client = (*MockClient)(nil)