Comment 1:

Labels changed: added priority-later, go1.2.

Status changed to Accepted.

Comment 2:

Yes, this would be marvellous for GoMock.

Comment 3:

While we're listing use cases, I'd also be able to take an arbitrary http.ResponseWriter
value from a client (which may implement optional http.Hijacker and/or io.WriterTo
and/or http.Flusher) and change a method or two on it, while still forwarding all other
methods on it.
Currently whenever I want to implement an http.ResponseWriter wrapper (which I continue
to do regularly), I have to defensively implement all 3 optional interfaces, with the
implementation bodies checking at runtime whether the wrapper ResponseWrapper implements
those, otherwise I break people's flushing or sendfile or hijacking, when perhaps all I
want to do is record Writes that go by, or change the underlying transport, or watch for
errors, etc.
Doing something like:
type myWrapper struct {
    io.Writer
    http.ResponseWriter
}
... is an error, due to Write ambiguity, and something like:
type myWrapper struct {
    http.ResponseWriter
}
func (myWrapper) Write([]byte) (int, error) { ... }
... hides Flush, WriteTo, Hijack, etc.
I could see where this road leads to abuse and performance problems, though.
But it's painful either way.

Comment 4:

It is unclear to me why this would be enough to eliminate Go file
generation from GoMock. The implementations of preexisting interfaces
could be generated this way but not the mocking stubs that people call
to describe the expected call sequences.
Russ

Comment 5:

It wouldn't eliminate the code generation, but it would halve it. That would be nice.
Eliminating the rest would require run-time type construction, which is outside the
scope of this issue.

Comment 6:

Eliminating half the generation for gomock may be compelling for you
as the author but it doesn't change at all the way people interact
with gomock. It's still a separate preprocessor. I'd be much more
excited if we could get rid of all the Go file generation entirely.
That's a qualitative difference.
However, even with runtime type construction I don't believe you can
eliminate the other half. You need something at compile time for the
mock user to import.
Russ

Comment 7:

You don't. The existing API for GoMock generated code only exports the
mock implementation of the interface; the recorder is returned by an
EXPECT method on that. I don't see any reason why something needs to
be imported. Instead of
  foo := mock_foo.NewMockFoo(ctrl)
you could write
  foo := gomock.NewMock(ctrl, (*Foo)(nil)) // passing a pointer to the interface
and get back a dynamically created mock.

Comment 8:

Yes but keep going. What does the code using foo look like? Suppose it
says foo.EXPECT(). The compiler must see a definition of a struct or
interface with an EXPECT method at compile time in order to build
that. Where did that come from? Here's a line from
sample/user_test.go:
        mockIndex.EXPECT().Put("b", gomock.Eq(2)) // matchers work too
I don't see how the compiler can compile that line without a
gomock-generated import defining something with a Put method that will
accept gomock.Eq(2) as an argument.

Comment 9:

Aah, I see where you're going. I was imagining that those methods
could be changed to take (...interface{}), and do type checks at run
time, but that still doesn't solve the very existence of EXPECT, and
nor does the compiler know about the existence of its return type's
Put method.

Comment 10:

That 'qualitative difference' can also be achieved by other means, such as build tool
support for custom pre-compilation phases, possibly specified by build directives (e.g.
a // +prebuild comment in GoMock could specify that the GoMock preprocessor be run on
anything that imports the mocking package).

Comment 11:

Not going to make the Go 1.2 cut. Not clear there's a use case anyway.

Labels changed: added go1.3maybe, removed go1.2.

Comment 12:

Labels changed: removed go1.3maybe.

Comment 13:

Labels changed: added go1.3maybe.

Comment 14:

Labels changed: added release-none, removed go1.3maybe.

Comment 15:

Labels changed: added repo-main.

Comment 16 by martin@probst.io:

Wouldn't it be possible to write something like Mockito's API:
  ctrl := ...
  defer ctrl.Finish()
  mockFoo := gomock.NewMock(ctrl, (*Foo)(nil))
  gomock.When(mockFoo.Put("b", gomock.Eq(2))).
    ThenReturn(PutResponse{}, nil)
  myTestCode()
I.e. NewMock creates a mock that returns zero values for all calls. When() returns an
Expectation interface that allows users to set expectations, e.g. using functions like
"ThenReturn(retVals ...interface{})". NewMock internally tracks the last call to any
method, calls on the returned Expectation interface set up the return values for any
subsequent calls.
You lose type safety on the values, i.e. ThenReturn is essentially untyped, but that
might be acceptable in return for no code generation. In Java-land, that is solved using
generics - When() could be parameterized as "func template <T> When(argument
<T>) Expectation<T>" (pseudo syntax), so that "ThenReturn()" calls could
require the correct types.

@bradfitz You mention having to implement wrappers for optional interfaces of http.ResponseWriter, are there any examples in http standard library? I've come across this problem as you've described it: https://groups.google.com/d/topic/golang-nuts/HKY3mI7Q2jY/discussion

@crawshaw, any interest? :)

@bradfitz I don't have time for this in the 1.7 window.

@jimmyfrasche One option is that StructOf left open the possibility of promoting methods from fields. You could create a struct type using StructOf which embeds a concrete type, and then add methods using NewTypeOf (or whatever it's called). The problem with this is that the new "methods" being closures means that the type would essentially have static members shared by all instances. The InterfaceOf approach at least makes it so that the underlying value is the collection of values captured by the method closures.

Based on the idea that you should be able to cache pairs of types, I tried writing out a more complete implementation using NewTypeOf.

I wanted to use reflect to do the equivalent of:

type createdAtRuntime struct {
  s S
  t T
}

func (c createdAtRuntime) SomeMethodNotOnT() {
  c.s.SomeMethodNotOnT()
}

// other methods of S not on T

func (c createdAtRuntime) SomeMethodOnSandT() {
  c.t.SomeMethodOnSandT()
}

// other methods of T

That way I could cache createdAtRuntime and create new instances just by setting the fields.

The code was mostly straightforward.

Create a struct with two fields a and b of the appropriate types.

Collect the relevant methods and create proxies that call them on a or b appropriately.

I ran into a problem though.

In order to create the proxies I needed the result of NewTypeOf but of course the proxies are the methods that I want to pass to NewTypeOf. I'm not sure of a way around that. (The code for the proxies is here https://play.golang.org/p/xUSfI92a7O if it's of interest but the rest was certainly uninteresting).

Also, I'm not sure how unexported methods would work with this. If I have a "Wrap" func defined in package A and in package B I create a value from a type defined in package B and one in C, then can I assert for interfaces containing unexported methods in package B? What if I pass the value to package C?

In order to create the proxies I needed the result of NewTypeOf but of course the proxies are the methods that I want to pass to NewTypeOf.

Right, we'd need an explicit carve-out to allow the receivers of those methods to be the underlying type rather than the actual receiver type. (In my comment, I described that as 'the "underlying" type (or a pointer to that type) must be assignable to the function's first argument'.)

Also, I'm not sure how unexported methods would work with this.

I think you'd have to use embedding, which implies that NewTypeOf (and by extension StructOf) would have to handle embedded fields. (StructOf currently does not, but explicitly documents that it may in the future.)

I think we need MakeValue(methods []Method) reflect.Value with the following semantics:

• the returned Value's type has an arbitrary, unique, illegal name (say "1"), an invalid PkgPath, and the specified method set
• the underlying type is struct{}
• when methods are called on the value they are not passed a receiver—they're treated like ordinary function calls
• type assertions on unexported methods respect the PkgPath of the method not the type

That let's us create a value with an arbitrary method set and type from regular functions.

Let's also say there was a way to create a method value† from a given reflect.Value (there may be one hidden in reflect but I couldn't spot it). Just to have some notation let's say it's reflect.MethodValue(v reflect.Value, methodName string) reflect.Value where the returned Value is the function.

Then given two arbitrary Value's to combine, we just need to

• collect and compute the method set,
• use MethodValue to turn the methods we're keeping into functions,
• create a Method for each copying the Name, PkgPath from the original Method, and setting Func to the result of MethodValue,
• and pass those to MakeValue.

† I don't see this used much in the wild so if anyone reading is unfamiliar, given a type T with a method func (t T) M(a int, b string) float64 and a value of that type v then v.M is equivalent to calling the below

func methodValueOfM(t T) func(int, string) float64 {
  return func(a int, b string) float64 {
    return t.M(a, b)
  }
}

@bcmills yes, sorry—missed the point about the underlying type to break the loop. That does have the disadvantage that you couldn't create a method which invokes another method using reflection as the receiver doesn't have the method set yet. Nothing to do with the use-case under discussion, but a limitation nonetheless.

Maybe it needs to be a builder pattern where NewTypeOf(underlying Type) returns some type that has an AddMethod(reflect.Method) method and a Build() reflect.Type method? Not especially idiomatic but without separating the steps in someway I think you're always going to have some edge to hit. The piecemeal construction avoids the issue.

Back to the use-case, as far as embedding, since we're, by definition, trying to combine two types with intersecting method sets we could only embed one of the two types so you'd have the same problems just with one less type to worry about except you'd also need to worry about a method colliding the embedded types name.

But that made me realize that in order for any of this to work we'd potentially need to create the equivalent of

package x //where the CombineValues func is defined
struct {
  a y.typeName 
  b z.typeName
}

since the types we're combining come in interfaces so the underlying type could very well be unexported and is likely from another package. For that to work the constructed type would have to operate by different rules than ordinary Go types, which I'm sure would cause all kinds of subtle problems.

The MakeValue approach suffers from the same issue but in a way that seems intuitively safer to me as the methods are siloed. You could do some weird stuff with MakeFunc. I think it would be easier to reason about the interactions there. But I'm just speculating.

Field names conflicting with methods is potentially unfortunate. However, the solution already exists, since Anonymous bool can be set separately from Name string (the type aliases proposal describes this as a solved problem).

As for what the receiver type would need to be for NewTypeOf methods, if it can be Value or an interface type, that would work. It is unfortunate though.

As for unexported methods, the behaviour of promotion in the language is that unexported methods are promoted, so you should be able to assert to an interface containing those methods (provided the method name in the interface and the method name on the type originate in the same package).

MakeValue could also work. An interesting potential is that your "two Values" could be

val1 := reflect.ValueOf(Wrap{ inner: err })
val2 := val1.Field(0).Elem()

@stevenblenkinsop good point about Anonymous—with that it's just a matter of generating a name longer than any methods, aaaaaaaaaa instead of just a, and so on.

The issue with the receiver type is that instead of

func (f Foo) Bar() {...}

you have to create the methods like

func (f struct{ aaaaa S; bbbbb T}) Bar()

but that should actually be fine if NewTypeOf goes through and fixes up the signatures (and it would have to) since when it's invoked the reflect.Value passed in as the receiver would be of the correct type, so my previous concern was simply due to insufficient blood-coffee levels.

The interesting thing about this wrt to unexported methods is that if you create a type from two types defined in separate packages, you could have two unexported methods on the created type with the same Name but different PkgPaths so it could, say, assert to interface{foo()} in both packages but call different methods. That is how it should behave, but it is a bit weird.

Having thought about this more I think @bcmills solution could work. I'll write up a version using it and my MakeValue later tonight to compare them and see if I run into any more issues.

Still playing with this, but I think NewTypeOf is the only way forward.

All of the other alternatives (including first class support in the language) become O(n) when composed.

If you had a function for combining two values in some way, say Combine, there's going to be cases where, for example, an error is returned, wrapped, returned, wrapped, and so on, several times resulting in something equivalent to Combine(Combine(Combine(err1, err2), err3), err4). Each layer adds another onion layer of indirection. For errors this isn't a big deal but for other uses like an http.ResponseWriter it could be a deal breaker.

However, with NewTypeOf, an implementation of Combine could detect types it created and optimize it so that there was never more than one level of indirection. That makes construction of the type more involved but that can be easily cached.

I've created a gist of a rough implementation. Warning: a little over 300 LOC.

While it's an optimizing implementation, the implementation is far from optimized. I pessimized it fairly extensively to make it easier to understand what it's doing since I can't actually run it to verify. It should be relatively straightforward to reduce allocations and the number of passes over the data, though.

To avoid creating a tree of combined values that has to proxy and re-proxy method invocations up to the depth of the tree, it adds a marker field as the 0th field of the struct it creates. When it's called on a value whose type has that marker field, like Combine(Combine(a, b), Combine(c, d)), it "flattens" everything out and computes the minimal method set as if it had been called like a hypothetical CombineVariadic(a, b, c, d). It also discards any fields, aside from the marker field, that do not contribute to the method set, allowing them to be garbage collected when they do not contribute anything.

A simpler, non-optimizing implementation would work, too, but it would have unfortunate performance implications in all but the simplest of cases making it less likely to be used.

If something like that was added to the reflect package itself, then fmt.Errorf could be rewritten to something like:

// Errorf formats according to a format specifier and returns the string
// as a value that satisfies error.
//
// The methods of the first error in the format args are still there
// [editor's note: I shouldn't write this docstring]
func Errorf(format string, a ...interface{}) error {
  msg := errors.New(Sprintf(format, a...))
  if err := firstError(a); err != nil {
    return reflect.Combine(err, msg)
  }
  return msg
}
func firstError(a []interface{}) error {
  for _, i := range a {
    if e, ok := i.(error); ok {
      return e
    }
  }
  return nil
}

That's not a light change to consider, it could have some weird consequences, but it would mean that a lot of code that hides optional methods on errors would stop doing so with no modification.

Another issue: (should have tested this earlier) StructOf can't create an unexported field, though maybe that's not an issue, since you could only get to the fields with reflect anyway and they're fairly arbitrary. If you use it to created exported fields with an unexported type from another package you can set that field, which is mildly surprising, but that is what we want to do here: https://play.golang.org/p/wQ8dC45dmI

Let's say for the moment that reflect can create new types with methods, something equivalent to what I wrote above is added to the stdlib, and that it becomes the blessed way, by convention, of composing errors.

This approach works well for errors with additional methods but not so well for errors with additional fields or sentinel errors, like io.EOF. IsX style funcs, like os.IsPermission, that dispatch on the identity of the type don't play well with this scheme.

Errors with additional fields can be made to work with this approach by adding an unexported method and an exported helper. For example, let's examine os.PathError. If it adds a method like

func (p *PathError) pathError() *PathError {
  return p
}

and the OS package adds a func like

func IsPathError(err error) *PathError { // result == nil implies false
  if p, ok := err.(interface{pathError() *PathError}); ok {
    return p.pathError()
  }
  return nil
}

then an error, under any number of combinations (with non-PathErrors), is still fully inspectable. Helpers like IsPermission could use similar tactics behind the scenes to extract the correct underlying error for their tests.

There's a small increase in API and implementation but everything works and is Go1 compatible.

Sentinel errors are a bit more annoying. They could follow the same basic approach, but each one would need its own type, like

type eof struct{}
func (e eof) Error() string { return "EOF" }
func (e eof) isEOF() {}
var EOF = eof{}
func IsEOF(err error) bool {
  _, eof := err.(interface{isEOF()})
  return eof
}

I don't think that's strictly Go1 compatible, but the only code I can think of that that would break is something using reflect. Perhaps it would be permissible as part of the transition to Go2?

It looks like the specific approach taken in the gist I posted relied on a bug ( #22073 ).

If it wants to expose unexported methods, it would need to use embedding to handle the method promotion (once that is in place) and only create proxy methods as tie-breakers for exported methods.

However that has several unhappy implications:

1. it cannot remove any "useless" values as it cannot detect when they do not contribute to the resultant method set
2. it cannot create tie-breakers for unexported methods which implicitly removes the method from the method set due to the ambiguity

1 is unfortunate but non-essential.

However, 2 means it just cannot handle unexported methods correctly. It needs to stick to only exported methods and always hide all unexported methods.

So the approach in the gist is correct and it can discard values that don't add to the method set, but it's less useful than I'd hoped because it can't handle unexported methods.

In order to handle unexported methods in the same manner as exported methods, it would need to either be a builtin or a function defined in the reflect package that circumvents the usual protections.

I hit a use case for this while working on google/go-cmp#88. The gist of the problem is that TypeOf and ValueOf can never directly return a type that is an interface. I understand why it is the case, but it goes against user expectation:

b := new(bytes.Reader)
reflect.TypeOf(b)            // *bytes.Reader
reflect.TypeOf(io.Reader(b)) // io.Reader expected, but got *bytes.Reader

This causes an issue when doing something like:

equateNotExist := cmp.Comparer(func(x, y error) bool { ... })
var e1, e2 error = syscall.ENOENT, os.ErrNotExist
cmp.Equal(e1, e2, equateNotExist) // false

The user expected this to be true since there is a custom comparer that knows how to compare error interfaces. However, this returns false because under the hood, cmp.Equal uses reflect.ValueOf and loses the fact that the user intended to pass in two error interfaces. cmp.Equal see two different types syscall.Errno and *errors.errorString and immediately returns false without even giving the custom comparer a chance to work.

If you so much as pass even a pointer to the interface, then it works as expected:

cmp.Equal(&e1, &e2, equateNotExist) // true

However, doing so is not intuitive at all.

In an effort to make this more user-friendly, my approach was to try to promote the initial top-level value into an interface with the common set of methods to both values:

// Determine the set of common methods.
type method struct {
	Name string
	Type reflect.Type
}
mm := make(map[method]int)
for i := 0; i < vx.NumMethod(); i++ {
	mm[method{vx.Type().Method(i).Name, vx.Method(i).Type()}]++
}
for i := 0; i < vy.NumMethod(); i++ {
	mm[method{vy.Type().Method(i).Name, vy.Method(i).Type()}]++
}
var ms []reflect.Method
for m, n := range mm {
	if n == 2 {
		ms = append(ms, reflect.Method{Name: m.Name, Type: m.Type})
	}
}

// Promote to interface type with common method set.
iface := reflect.InterfaceOf(ms)
return vx.Convert(iface), vy.Convert(iface)

but this does not work since there is no reflect.InterfaceOf method.

@dsnet I might misread this, but the documentation says that

The comparer f must be a function "func(T, T) bool" and is implicitly filtered to input values assignable to T. If T is an interface, it is possible that f is called with two values of different concrete types that both implement T.

That would seem to cover the case you mention? In particular this works.

@Merovius, so as to keep this thread about InterfaceOf, I'll comment on google/go-cmp#88 regarding your comment. My point in my previous comment is that I had use-case for InterfaceOf to create a dynamic interface that contains the common set of methods for 2 concrete types.

I don't want to be a party pooper, but adding a feature like reflect.MakeInterface — or any other way of creating arbitrary types to fulfill interfaces at runtime — pushes the Go complexity clock quite close to midnight. My fear hearkens from building, maintaining, and diagnosing production Java servers that pushed the JRE through things like cglib and similar machinations. I am really not trying to sound like reactionary, but these systems were an absolute nightmare and enabled bounds of complexity in Inversion of Control harnesses and other obtuse stacks of indirection. The only thing that could — barely — tame JRE dynamic code generation is Java's superb runtime debugger ecosystem, which we don't have.

I skimmed over the issue's feedback and wondered: is there value in having a discussion 1. whether we should have this feature at all and 2. whether the benefits disproportionately outweigh the likely costs.

@matttproud I assume you are talking about the case of adding methods to a type created by using the reflect package. E.g., adding methods to a type created via reflect.StructOf. I clarify because I think that the function that does that operation should not be called reflect.MakeInterface. I think that reflect.MakeInterface, possibly better named reflect.InterfaceOf, is a function that creates a new interface type, just as reflect.StructOf creates a new struct type. I don't see any particular harm in being able to do that.

Personally I'm also not too worried about being able to add methods to a type created by reflect. That is nothing like Java's cglib. In Go, the code for the actual methods will be, essentially, a function literal. Adding methods to a type will amount to a different way of calling a function literal. Yes, the stack trace will be complex, but this is still much less powerful and much less complex than cglib.