Skip to content

Fix issue with nested anon structs in source code#253

Closed
chrsmith wants to merge 1 commit intomasterfrom
chrsmith/fix-mockgen-fix
Closed

Fix issue with nested anon structs in source code#253
chrsmith wants to merge 1 commit intomasterfrom
chrsmith/fix-mockgen-fix

Conversation

@chrsmith
Copy link
Copy Markdown

What changed?

  • Fix an bug where mockgen-fix would fail to format updated source code if the mock struct contained a field with an anonymous struct type (struct{}).
  • Wire up a new --verbose flag to print out the code being modified, to make it easier to debug errors during the formatting step.

Why?

I was unable to run make proto because the mockgen-fix step was failing with the following error:

2026/04/24 13:55:32 failed formatting: 292:1: expected '}', found 'var'
exit status 1

As it turns out, there is a minor bug in the implementation. The version of mockgen I have on my machine (v0.6.0) apparently emits the expected struct with a new field that causes the logic for finding the "end of the struct }".

  285: // MockOperatorServiceServer is a mock of OperatorServiceServer interface.
  286: type MockOperatorServiceServer struct {
  287: 	ctrl     *gomock.Controller
  288: 	recorder *MockOperatorServiceServerMockRecorder
+ 289: 	isgomock struct{}
  290: }

So the actual fix is just changing this line, ensuring that we find the first } character at the start of the line. (And not the last character of isgomock struct{}.)

- endBrace := structIndex + strings.Index(source[structIndex:], "}\n") + 2
+ endBrace := structIndex + strings.Index(source[structIndex:], "\n}\n") + 2

But this PR includes the additional refactorings I used to look into this.

How did you test it?

Just ran it locally. Now things work as expected.

Potential risks

If the proto compiler emits Go code where structs don't end with their last } at the beginning of the line, then mockgen-fix will start failing in CI.

BONUS: When did this break?!?

I'm a little confused why I am running into problems here. After some quick searching:

The isgomock struct{} field was added to the mockgen output in October 2024 as part of the v0.5.0 release of the uber-go/mock fork.
This change was specifically introduced in Pull Request #204 to resolve a common issue where mocking the fmt.Stringer interface could lead to deadlocks or infinite recursion during test failures.

So... could it be the case that everyone using mockgen is on a version from 10/2024? And I'm only hitting this because I just ran go install go.uber.org/mock/mockgen@latest this afternoon?

That is plausible, but doesn't sound right...

@chrsmith chrsmith requested review from a team as code owners April 24, 2026 21:16
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 24, 2026

CLA assistant check
All committers have signed the CLA.

@chrsmith
Copy link
Copy Markdown
Author

@long-nt-tran pointed out that the issue here is that the repo is geared around using a fixed version of the tool. (And not just keeping up with the latest.)

The key piece is using the make mockgen-install and make goimports-install targets.

mockgen-install:
	printf $(COLOR) "Install/update mockgen..."
	go install -modfile=build/go.mod github.com/golang/mock/mockgen

goimports-install:
	printf $(COLOR) "Install/update goimports..."
	go install golang.org/x/tools/cmd/goimports@latest

@chrsmith chrsmith closed this Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants