Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Embedded Interfaces #9

Merged
merged 14 commits into from
Dec 1, 2017
Merged

Support Embedded Interfaces #9

merged 14 commits into from
Dec 1, 2017

Conversation

kevinbirch
Copy link
Contributor

@kevinbirch kevinbirch commented Nov 30, 2017

This PR inlines methods from embedded interfaces into the generated Fake. It does this by resolving embeded field identifers from the model cache and prepending the nested methods in source order. This recursive expansion happens just before the template is rendered so that the definition of embedded interfaces can occur after the definition of the embedder. To resolve embedded intefaces not defined in the current package, all intefaces in all imported packages are also scanned.

The -file flag has been removed because we always want to scan an entire package. This ensure that we can find all available interfaces that might be embedded.

Support for map and function parameter types has also been added.

@kevinbirch kevinbirch requested a review from solumos as a code owner November 30, 2017 03:29
@kevinbirch kevinbirch self-assigned this Nov 30, 2017
Name: obj.Name(),
}

for i := 0; i < ifType.NumMethods(); i++ {
Copy link
Contributor

@solumos solumos Dec 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this break for an embedded interface that's imported from another package that embeds another interface from another package? I think so, if main doesn't import bar directly.

e.g.

package main

import (
	"foo"
)

// Interface to mock
type Embedder interface {
	foo.DoubleEmbedder
	Other(string) string
}
package foo

import (
	"bar"
)

type DoubleEmbedder interface {
        bar.Breaker
}
package bar

type Breaker interface {
        Break()
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To resolve embedded intefaces not defined in the current package, all interfaces in all imported packages are also scanned.

More precisely - are packages imported by imported packages scanned if they're not imported in the current package?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it will break, very perceptive. I decided that the trouble required to fully recursively resolve a hierarchy like that wouldn't be worth the trouble (for now anyway).

are packages imported by imported packages scanned if they're not imported in the current package?

nope, they're not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems unlikely that such a case would come up very often. If it does then we can support it. Just the automatic resolution of imports puts this package far ahead of similar ones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the trouble required to fully recursively resolve a hierarchy like that wouldn't be worth the trouble (for now anyway)

Cool, I agree

@kevinbirch kevinbirch merged commit 56d4c3d into master Dec 1, 2017
@kevinbirch kevinbirch deleted the embedded_interfaces branch December 1, 2017 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants