Infinite loop in createInstance() caused by Copy Constructors

[desilvai]

Create instance (as invoked by the any() matcher) loops infinitely if any of the parameters of the "easiest constructor" are the same type as the class we are trying to construct (as one might expect from a copy constructor). This can be triggered by creating a primary constructor with more than one non-array parameters and a copy constructor (containing only a single parameter). Create instance will treat the copy constructor as the easiest one and try to use it. However, this will cause create instance to be called again on the same class with the same result. This would also apply to any constructor that takes in an object of its same type (not just copy constructors). Let's call these self-referential constructors.

The simplest fix (included here) is to remove all self-referential constructors from the set of easiest constructors. I believe this is sound because there must be at least one non-self-referential constructor in order to get the initial object to copy and create instance still works when all constructors are private (implying there must be at least one constructor it can call).

One additional note from looking through this code. The easiest constructor is always determined by the number of parameters plus some additional filters. However, if all parameters have a default value, wouldn't that be the easiest constructor to use? This isn't addressed in this code. The goals of this may be better served if you could sort by the number of non-optional parameters. It is easy enough to do, but I wanted to make sure I understood the objective of the method before making the change.

[desilvai]

Technically, you can specify a class with only a copy constructor, but I can't find any way for this to be used. Here's the example:

class WithOnlyACopyConstructor constructor(val other: WithOnlyACopyConstructor)

Because it is not open, no one can subclass it. Since there is no other constructor (private or otherwise), there is no way to create an object that can be used by this constructor. Effectively, this class cannot be used. Unfortunately, the compiler doesn't flag this as an error. If someone tries to mock this, they will get a NoSuchElementException because easiestConstructor() in createInstance() returns an empty list.

Similarly, if we have only a constructor containing an optional, self-referential argument, the argument would have to either be nullable or the class would have to be open. If the class is open, then the changed lines will not be executed since it is mockable. If the argument was not nullable and closed, we, again, cannot create an object that can serve as the default value (since it can't be constructed). If it is nullable and closed, the type of the default value is not the same as the type of the class so the provided code will work.

[nhaarman]

Thanks!
I don't think we really need to care about the WithOnlyACopyConstructor case since it cannot be used in production code either. If someone finds a way to do this anyway, they might be able to workaround this by using an instance creator.

The easiestConstructor indeed assumes that a constructor with less parameters is 'easier' than one with more. So the objective of the method is indeed to do the minimum amount of work necessary to get the job done.

You're right that optional parameters can be skipped. I'll implement that as well, thanks!

[nhaarman]

I made a small documentation change and merged the PR manually in 7b6c5b7.