-
Notifications
You must be signed in to change notification settings - Fork 118
Commit
A deadlock related to controller calling Stringer on mocks themselves was revealed in #116. #144 solved this deadlock by adding a generated `ISGOMOCK()` method to all generated mocks, and then checking for it before calling `.String()` on arguments. This reveals an exported method on each generated mock that: * Bloats the generated code * Can be taken dependency on in strange ways via Hyrum's Law * Technically opens up another route for naming collision. This PR attempts to clean up this type of check by instead generating an unexported field in generated mock structs instead, and checks for it using reflect. This hides this implementation detail of gomock/mockgen better, and produces less generated bloat. Note that, importantly, the regression test added in #144 still passes with this PR. Ref: #193
- Loading branch information
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,36 @@ | ||
package gomock | ||
|
||
import "fmt" | ||
|
||
type mockInstance interface { | ||
ISGOMOCK() struct{} | ||
} | ||
type mockedStringer interface { | ||
fmt.Stringer | ||
mockInstance | ||
} | ||
import ( | ||
"fmt" | ||
"reflect" | ||
) | ||
|
||
// getString is a safe way to convert a value to a string for printing results | ||
// If the value is a a mock, getString avoids calling the mocked String() method, | ||
// which avoids potential deadlocks | ||
func getString(x any) string { | ||
switch v := x.(type) { | ||
case mockedStringer: | ||
return fmt.Sprintf("%T", v) | ||
case fmt.Stringer: | ||
return v.String() | ||
default: | ||
return fmt.Sprintf("%v", v) | ||
if isGeneratedMock(x) { | ||
return fmt.Sprintf("%T", x) | ||
} | ||
if s, ok := x.(fmt.Stringer); ok { | ||
return s.String() | ||
} | ||
return fmt.Sprintf("%v", x) | ||
} | ||
|
||
// isGeneratedMock checks if the given type has a "isgomock" field, | ||
// indicating it is a generated mock. | ||
func isGeneratedMock(x any) bool { | ||
typ := reflect.TypeOf(x) | ||
if typ == nil { | ||
return false | ||
} | ||
if typ.Kind() == reflect.Ptr { | ||
typ = typ.Elem() | ||
} | ||
if typ.Kind() != reflect.Struct { | ||
return false | ||
} | ||
_, isgomock := typ.FieldByName("isgomock") | ||
return isgomock | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.