code review 5080043: Avoid generating mocks for interfaces with embedded int...

Avoid generating mocks for interfaces with embedded interfaces.

Fixes issue 2.

[dsymonds, 2011-09-20 04:27:39]

Hello dsymonds,

I'd like you to review this change to
https://gomock.googlecode.com/hg/

[dsymonds, 2011-09-20 04:27:46]

*** Submitted as http://code.google.com/p/gomock/source/detail?r=aca9ae018061
***

Avoid generating mocks for interfaces with embedded interfaces.

Fixes issue 2.

TBR=dsymonds
http://codereview.appspot.com/5080043

[awreece, 2011-09-20 04:56:18]

I apologize for the unsolicited code review, but I would appreciate it if you
would take a look.

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go
File mockgen/mockgen.go (right):

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode149
mockgen/mockgen.go:149: // because there is not yet a way to find out the method
set of Y,
Can we add a TODO(dsymonds) next to the "not yet" so we don't ignore the issue?

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode235
mockgen/mockgen.go:235: if is.Name.Name == "_" {
What is this for?

[dsymonds, 2011-09-20 04:58:30]

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go
File mockgen/mockgen.go (right):

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode149
mockgen/mockgen.go:149: // because there is not yet a way to find out the method
set of Y,
On 2011/09/20 04:56:18, awreece wrote:
> Can we add a TODO(dsymonds) next to the "not yet" so we don't ignore the
issue?

Sure. I'll add something next time I'm in here.

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode235
mockgen/mockgen.go:235: if is.Name.Name == "_" {
On 2011/09/20 04:56:18, awreece wrote:
> What is this for?

If you have an import renamed to "_" then you cannot possibly be using its
symbols in any interface that we are mocking.

[awreece, 2011-09-20 05:00:47]

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go
File mockgen/mockgen.go (right):

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode149
mockgen/mockgen.go:149: // because there is not yet a way to find out the method
set of Y,
SGTM
On 2011/09/20 04:58:30, dsymonds wrote:
> On 2011/09/20 04:56:18, awreece wrote:
> > Can we add a TODO(dsymonds) next to the "not yet" so we don't ignore the
> issue?
> 
> Sure. I'll add something next time I'm in here.

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode235
mockgen/mockgen.go:235: if is.Name.Name == "_" {
I saw that in the test, but would have appreciated seeing it as a comment. But
anyways, thanks for clarifying.

On 2011/09/20 04:58:30, dsymonds wrote:
> On 2011/09/20 04:56:18, awreece wrote:
> > What is this for?
> 
> If you have an import renamed to "_" then you cannot possibly be using its
> symbols in any interface that we are mocking.

[dsymonds, 2011-09-20 05:02:31]

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go
File mockgen/mockgen.go (right):

http://codereview.appspot.com/5080043/diff/4001/mockgen/mockgen.go#newcode235
mockgen/mockgen.go:235: if is.Name.Name == "_" {
On 2011/09/20 05:00:47, awreece wrote:
> I saw that in the test, but would have appreciated seeing it as a comment. But
> anyways, thanks for clarifying.

Yeah, sorry. This CL accidentally glommed together a handful of changes that had
been reviewed internally. I'll take more care to break them up in the future.