gradle / gradle

Adaptable, fast automation for all
https://gradle.org
Apache License 2.0
17k stars 4.78k forks source link

ObjectFactory should be able to instantiate types with a no-arg ctor and no @Inject annotation #7645

Open eskatos opened 6 years ago

eskatos commented 6 years ago

Expected behavior

Given a Kotlin class with properties with default values declared in its constructor and jvm overloads providing a no-arg constructor:


open class Some @JvmOverloads constructor(
    var foo: String = "bar",
)

When using the ObjectFactory to create an instance of that class:

objects.newInstance(Some::class)

It succeeds.

Note that this would also apply to a simple Java class without any declared constructor:

public class Some {}

Current behavior

The above fails with:

java.lang.IllegalArgumentException: Class Some has no constructor that is annotated with @Inject.

Workaround

Explicitly declare a no-arg constructor annotated with @Inject:

open class Some(
    var foo: String = "bar",
) {
    @Inject // I shouldn't need to specify an additional constructor like this
    constructor(): this("bar")
}

Proposed solution

Gradle should instantiate types without the @Inject annotation using the no-arg constructor:

Context

From gradle/kotlin-dsl#1028:

If I want to use the ObjectFactory to create an instance of a kotlin object, I need a single noarg constructor annotated with @Inject.

For classes where I want properties to be in the constructor of the object this requires you to jump though extra hoops like the following:

open class CustomPasswordCredentials
constructor(
    @get:Internal
    private var user: String? = null,
    @get:Internal
    var pass: String? = null
) : PasswordCredentials {

    @Inject // I shouldn't need to specify an additional constructor like this
    constructor(): this(null, null)

    override fun setUsername(userName: String?) {
        user = userName
    }

    @Input
    override fun getUsername(): String? {
        return user
    }

    @Input
    override fun getPassword(): String? {
        return pass
    }

    override fun setPassword(password: String?) {
        pass = password
    }
}

Kotlin 1.0.6 introduced the kotlin-noarg plugin that allows you have entities annotated with some annotations to automatically have the noarg constructor get generated allowing them to be used by a DI framework like the one Gradle utilizes.

adammurdoch commented 6 years ago

The current behaviour is intentional and follows the JSR-330 spec. We'd be moving away from JSR-330 semantics if we choose the no-args constructor when there are multiple constructors and none of them are annotated (this is the only real difference between current and proposed behaviour). Which might be ok.

JLLeitschuh commented 6 years ago

From the JSR-303 spec.

@Inject is optional for public, no-argument constructors when no other constructors are present. This enables injectors to invoke default constructors.

I was going to try to make an argument that the user hasn't technically declared multiple constructors, but even I can't make that argument work in my head.

I was trying to come up with an argument that, from a conceptual perspective, the kotlin code only has one declared constructor. This one constructor can accept zero or more arguments, and it's just the compiler that has generated multiple constructors for them. With this argument, the best solution is to pick the single no-arg constructor that the user declared.

However, I'm guessing that all major DI frameworks would fail to inject a class that takes a vararg as it's single constructor.

class Example {
    Example(String ...something) {
    }
}

I agree with @adammurdoch on this one.

However, I think the error neeeds to be more informative for users. The error should include a propposal for how to fix the issue.

adammurdoch commented 6 years ago

I wasn't necessarily arguing against changing the behaviour, only pointing out why the behaviour is as it is.

Increasingly, we're expecting that model objects have a certain structure that is specific to Gradle and there's no intention that these types will work anywhere else. This means that enforcing the JSR rules becomes less important and we can tailor the structure to whatever works best for writing plugins in Java, Groovy and Kotlin.

Whatever structure we choose to support, we can certainly improve the error messages to help guide people towards the that structure.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.