Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Type-safe return values #622

Open
trim21 opened this issue Feb 14, 2022 · 6 comments
Open

Type-safe return values #622

trim21 opened this issue Feb 14, 2022 · 6 comments
Labels
status: needs more info This issue need more information from the author. type: feature request

Comments

@trim21
Copy link

trim21 commented Feb 14, 2022

Requested feature

This issue proposal generating a typed .Return( for *gomock.Call.

it's a re-open for closed issue #427 .

Why the feature is needed

currently .Return( has a sigurate of func (rets ...interface{}) can accept any return value in compile time.

But wrong return values' type result in always failing at runtime, which can be avoided at compile time.

Proposed solution

User Code

type Auth interface {
	// GetByToken return an authorized user by a valid access token.
	GetByToken(ctx context.Context, token string) (model.User, error)
}

Generated mocks

// Code generated by MockGen. DO NOT EDIT.

type MockAuthMockRecorder struct {
	...
}

// GetByToken indicates an expected call of GetByToken.
func (mr *MockAuthMockRecorder) GetByToken(ctx, token interface{}) *TypedCall {
	mr.mock.ctrl.T.Helper()
	return &TypedCall{mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetByToken",
		reflect.TypeOf((*MockAuth)(nil).GetByToken), ctx, token)}
}


type TypedCall struct {
	*gomock.Call
}

func (t *TypedCall) Return(typedReturn1 model.User, typedReturn2 error) *gomock.Call {
	return t.Call.Return(typedReturn1, typedReturn2)
}

User Test Code

func TestTypeSafeReturn(t *testing.T) {
	m := NewMockAuth(gomock.NewController(t))
	m.GetByToken(gomock.Any(), gomock.Any()).Return() <== Now we get compile time error

	// These should works fine at compile time and runtime.
	m.GetByToken(gomock.Any(), gomock.Any()).Return(model.User{}, nil)
	m.GetByToken(gomock.Any(), gomock.Any()).Return(model.User{}, errors.New("Not Found")) 
}
@codyoss
Copy link
Member

codyoss commented Apr 24, 2022

The reason this suggestion was closed in the path is it sounded like it would be a breaking change. Looking at this prop that is also true. I think if we were going to do something like this in the future we would want to do it in a way that is backwards compatible. Do you have a suggested API that is not a breaking change?

@codyoss codyoss added the status: needs more info This issue need more information from the author. label Apr 24, 2022
@trim21
Copy link
Author

trim21 commented Apr 24, 2022

The code will break with this change it already broken at runtime (always panic). This change only make then break early to compile time.

non-breaking-change Suggestions:

  1. Add a flag in cli that generate new typed call.
  2. Add a .TypedReturn( , .Typed().Return( or new struct NewTypedMockAuth(gomock.NewController(t)) these methods are type-safe.

Personally I prefer first one.

@trim21
Copy link
Author

trim21 commented May 21, 2022

@codyoss any thoughts on these suggestions?

@trim21
Copy link
Author

trim21 commented Jul 29, 2022

Another mock generator support typed-return

https://github.com/vektra/mockery

And it's kinda funny because it doesn't support type safe matcher...

@codyoss
Copy link
Member

codyoss commented Aug 12, 2022

I don't disagree that overall type-safe returns would be a good idea, but there are potentially people exploiting the current state that may be broken. Let me think about this some more and see if this issue gets more traction. The flag suggestion seems like an okay idea though.

@alvarotuso
Copy link

This would also be very interesting for me 👍 . Not having typed returns makes test maintenance harder.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: needs more info This issue need more information from the author. type: feature request
Projects
None yet
Development

No branches or pull requests

3 participants