Support go:mock annotations to mock all interfaces in one call #6

jonathansharman · 2025-04-13T14:24:50Z

Description

This PR enables mock to generate more than one mock in a single invocation and to generate more than one mock into the same file. Running without the formerly-required positional argument will search the input directory (-d) for any interface annotated with a //go:mock directive in its godoc comments. (I chose go:mock in analogy to go:generate, but I'm open to other ideas for how to mark interfaces for mocking, if anyone has qualms about a made-up go directive!) By default, an interface defined in example.go will be mocked into example_mock.go, or the output filename can be specified as part of the directive, e.g. //go:mock example_mock.go.

This new execution mode has three benefits over the single-interface/go:generate approach:

In this mode, mock only has to parse and analyze packages once, not once per mocked interface. This can provide massive performance gains in very large repos - see my benchmarks below.
Compared to go:generate, go:mock directives are appreciably DRYer, especially in the common case where the mock file's name corresponds to the interface file's name. Compare:
```
//go:generate go tool mock -o myInterface_mock.go MyInterface
type MyInterface any
```
```
//go:mock
type MyInterface2 any
```
Multiple mocks can now be generated into the same file. This would have been possible but significantly more complicated to implement when generating mocks one a time, requiring mock to know how to locate and update one mock without interfering with existing mocks in the same file.

Backward Compatibility

I don't think the new mode of execution has any major drawbacks vs. the old one. It does lose the ability to generate one mock at a time, but

we at Niche have never used mock in that way as far as I know (the prescribed way is to run go generate ./...),
the new mode is so fast that it's hard to imagine wanting to single out one mock at a time for performance reasons, and
we could add a new CLI option for filtering by interface if we wanted to.

The only other use case for the single-interface mode I can think of is wanting to generate a mock without marking up the source code with go:mock (or go:generate). That's also not something we've found useful at Niche so far since generally the person/code defining the mock is also the one defining the interface. (For that matter, mock's import generation logic relies on mocks being generated into the same package as the mocked interface.)

Regardless, I decided to put some effort into ensuring the old mode continues to work as-is. If we didn't care about backward compatibility, I would drop the -o option and probably turn the -d option into a positional argument so that mock could be invoked as mock ./... or mock -w ./..., which would be more in line with similar tools like gofmt, gofumpt, and modernize. However, I think it's worth keeping old scripts working as-is.

Testing Considerations

I've split the example directory into two packages: examples/generate and examples/directive. The generate package contains the old examples, using go:generate directives. The directive package is just copied from the existing examples, except that it uses go:mock directives instead of go:generate. It also has one extra test file, customOutputFile.go for testing out custom output filenames and generation of two mocks into the same file.

I've also tested this out in https://github.com/nicheinc/lead/pull/926. Since that repo is private to nicheinc, I'll summarize the most interesting results here as well. Using go tool mock instead of go run github.com/nicheinc/mock resulted in a 2.8x speedup by itself. Using go tool mock -w -d ./... resulted in an additional speedup of 18x, for a total speedup of 51x!

Versioning

Minor. Changes are backward-compatible.

… directory

jonathansharman · 2025-04-13T19:13:54Z

iface/get.go

+					if outputFile == "" {
+						outputPath = strings.TrimSuffix(inputPath, ".go") + "_mock.go"
+					} else {
+						outputPath = filepath.Join(filepath.Dir(inputPath), outputFile)


This actually does support relative paths, e.g. ../example_mock.go, but since the mock will be generated into a different directory than the source interface, its package name and imports may be broken. This is an issue with the existing functionality too though, and fixing it in general could be very complicated. Furthermore, generating mocks into different directories from their interfaces doesn't seem all that useful anyway.

jonathansharman · 2025-04-13T19:15:15Z

iface/get.go

-	for _, t := range *visited {
-		if t == typ {
-			return true
-		}
+func validateType(typ types.Type, visited map[types.Type]bool) bool {
+	if visited[typ] {
+		return true
 	}
-	*visited = append(*visited, typ)


My linter pointed out this loop could be a slices.Contains call, which made me realize it could be a map lookup.

Or a collections.Set 🤷

That was my first thought, but mock is GPL 3.0, and nicheinc/collections is currently private. 🤫

jonathansharman · 2025-04-13T19:25:57Z

iface/get.go

 	// Find the interface by name
 	ifaceObj := pkg.Types.Scope().Lookup(ifaceName)
 	if ifaceObj == nil {
-		return Interface{}, errors.Errorf("interface not found in package: %s", ifaceName)


I didn't know there used to be a third-party (?) errors package before errors and fmt.Errorf were invented!

jonathansharman · 2025-04-13T19:29:18Z

main.go

+//go:embed template.tmpl
+var tmpl string


I renamed template.go to template.tmpl and embedded it in this tmpl variable, which makes the template file a little neater (no giant raw string literal) and allows some IDEs to provide template validation.

jonathansharman · 2025-04-13T19:30:25Z

main.go

+	// Load package info.
+	pkgs, packageErr := packages.Load(&packages.Config{Mode: packages.LoadSyntax}, config.dir)
+	if packageErr != nil {
+		log.Fatalf(`Error loading package information: %s`, packageErr)
 	}
-	ifaceName := args[0]
-
-	// Parse the package and get info about the interface
-	iface, err := iface.GetInterface(*dir, ifaceName)
-	if err != nil {
-		log.Fatalf("Error getting interface information: %s", err)
+	if len(pkgs) < 1 {
+		log.Fatalf(`No packages found in %s`, config.dir)
 	}


I pulled this out of GetInterface since it's now the common first step of both single-interface and multi-interface mode.

nathanjcochran · 2025-04-16T20:50:02Z

😮

wmarshall · 2025-04-24T20:14:44Z

examples/directive/myInterface.go

+// TODO: chan types
+// TODO: map types


Are these still todos?

I guess not? 1e410f8

@nathanjcochran Do you remember if map and chan types as params/returns didn't use to generate correctly, or why these TODOs are here? Seven years later they seem to work no problem, but maybe I don't understand what those TODOs mean. 🤔

Looks like I added those TODOs while I was in the process of creating that test interface and working through some errors I was encountering at the time: nathanjcochran@3332ebb.

So I think it was just a reminder to myself to add those types to test interface, which I apparently never got around to doing. So I guess it was actually a reminder to you 😅.

I think it was right around the time I discovered this Go bug: golang/go#25262 (hence the removal of this and this), and I probably got sidetracked filing that issue.

wmarshall · 2025-05-01T18:46:33Z

iface/get.go

+			ast.Inspect(fileNode, func(node ast.Node) bool {
+				// Only consider declarations with godoc comments.
+				decl, isGenDecl := node.(*ast.GenDecl)
+				if !isGenDecl || decl.Doc == nil {


Is it worth putting an explicit check for a node = nil here? I think you get saved by is isGenDecl being false in that scenario, but my understanding is nil is the base-case parameter

Certainly doesn't hurt: b37e654

iface/interface.go

jonathansharman · 2025-05-01T21:29:20Z

@bclarkx2 suggested a step beyond one mock file per input file: one mock file per package. That was already possible on this branch by setting each go:mock directive in a given package to the same output file, but I've made that easier to do in 8c9316a, which allows the use of the -o flag in multi-interface mode. When -o is passed, it will override the usual default output file, so the default example_mock.go is only used if both the go:mock directive and the -o flag are empty. This allows one to run mock -w -o mock.go -d ./... to get one mock.go file per package.

Active Niche employees can see the results in https://github.com/nicheinc/lead/pull/926. 😄

Jonathan Sharman added 3 commits April 13, 2025 10:07

Support go:mock annotations to mock all interfaces in one call

ef67134

Qualify go:mock output filepaths to avoid dumping mocks into the root…

b5d7332

… directory

Remove hyphen

8e7f7a5

jonathansharman commented Apr 13, 2025

View reviewed changes

Wording tweak: we aren't parsing the package here

6ed78bd

jonathansharman marked this pull request as ready for review April 13, 2025 21:01

Jonathan Sharman added 3 commits April 14, 2025 09:44

Allow multiple mocks to be generated into the same file

8a7be25

Add an example of generating two mocks into the same file

05fc546

Fix import logic, add tests for blank imports

9a39a33

wmarshall self-requested a review April 23, 2025 17:36

wmarshall reviewed Apr 24, 2025

View reviewed changes

Add map/chan params/returns to test interfaces

1e410f8

wmarshall reviewed May 1, 2025

View reviewed changes

Handle nil node in ast.Inspect callback

b37e654

wmarshall reviewed May 1, 2025

View reviewed changes

iface/interface.go Show resolved Hide resolved

wmarshall approved these changes May 1, 2025

View reviewed changes

Jonathan Sharman added 2 commits May 1, 2025 17:08

Allow -o to override the default output file

8c9316a

Update README > Usage

9e80fbe

jonathansharman merged commit 0ea12e7 into main May 1, 2025

jonathansharman deleted the feature/one-call-mocks-all branch May 1, 2025 21:34

jonathansharman mentioned this pull request May 1, 2025

Support go:mock annotations to mock all interfaces in one call nathanjcochran/mock#8

Open

Support go:mock annotations to mock all interfaces in one call #6

Support go:mock annotations to mock all interfaces in one call #6

Uh oh!

Conversation

jonathansharman commented Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Backward Compatibility

Testing Considerations

Versioning

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathansharman Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nathanjcochran commented Apr 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nathanjcochran Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathansharman commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jonathansharman commented Apr 13, 2025 •

edited

Loading

jonathansharman Apr 13, 2025 •

edited

Loading

nathanjcochran Apr 25, 2025 •

edited

Loading