Annotate closure args in Project with @DelegatesTo.

[rhencke]

Makes code completion in IDE less bad. (also fixed a tiny typo)

Any of the checked boxes below indicate that I took action:

• Reviewed the Contribution Guidelines.
• Signed the Gradle CLA.
• Ensured that basic checks pass: ./gradlew quickCheck

I believe this is a very trivial change. (No API changes or functionality changes. Just a better IDE experience). But, please let me know if more is needed.

[eriwen]

@melix Could I request a very quick review from you here (given that you wrote @DelegatesTo)? This seems like such an obviously good thing that I'm sure we have good reasons for not using @DelegatesTo here, I just don't know what they are.

[melix]

Sorry I forgot to answer this PR. I think it is valuable to add @DelegatesTo as well @ClosureParams. The only reason the codebase is not annotated with this is that the annotations didn't exist when the code was written. It provides valuable information, not only for IDE completion, but also for static compilation of plugins. But is also needs to be reviewed carefully.

[melix]

It's a very nice improvement but it needs further refinements:

• the strategy (optional parameter of @DelegatesTo) needs to be set to Closure.DELEGATE_FIRST
• it would be nice to have tests highlighting the fact that static compilation works with this (but not required)

[rhencke]

@melix Thank you for your review!

I will work on addressing your requests and get you an updated version of this pull request.

[rhencke]

@melix It turns out many delegates are actually OWNER_ONLY. However, the owner is set to org.gradle.internal.metaobject.ConfigureDelegate in these cases, and its implementation of GroovyObjectSupport does appears to attempt property access/method invocation in the order of delegate first, and then owner second, which matches the spirit of DELEGATE_FIRST.

For example (pardon my bad Groovy):

HashMap<Integer,String> rs = [:]
rs[Closure.DELEGATE_FIRST] = "DELEGATE_FIRST"
rs[Closure.DELEGATE_ONLY] = "DELEGATE_ONLY"
rs[Closure.OWNER_FIRST] = "OWNER_FIRST"
rs[Closure.OWNER_ONLY] = "OWNER_ONLY"
rs[Closure.TO_SELF] = "TO_SELF"

task('q') {
    println "task():"
    println "strategy is         ${rs[getResolveStrategy()]}"
    println "delegate.getClass() ${delegate.getClass()}"
    println "it.getClass()       ${it.getClass()}"
    println "this.getClass()     ${this.getClass()}"
    println "owner.getClass()    ${owner.getClass()}"
    println "(but owner.class is ${owner.class}...?)"
}

yields:

task():
strategy is         OWNER_ONLY
delegate.getClass() class org.gradle.api.DefaultTask_Decorated
it.getClass()       class org.gradle.api.DefaultTask_Decorated
this.getClass()     class build_3919psp5ai3nt8u1758a2qpse
owner.getClass()    class org.gradle.internal.metaobject.ConfigureDelegate
(but owner.class is class org.gradle.api.DefaultTask_Decorated...?)

I'm assuming, for cases like this, it's closer in spirit to mark this as @DelegatesTo(value = Task.class, strategy = DELEGATE_FIRST) than it would be to mark it @DelegatesTo(value = ConfigureDelegate.class, strategy = OWNER_ONLY), for the sake of IDE completion. But, I don't know how this information gets used during static compilation, and this was quirky enough I just wanted to give you a heads up in case this wasn't the right way to proceed.

[melix]

Yes the delegation system of Gradle is a bit complex. Basically for methods that take an Action<T>, we automatically generate a Closure version where the strategy is set to DELEGATE_FIRST. I think it's fine to use DELEGATE_FIRST here. In practice it should be the case.

[rhencke]

I couldn't figure out a way to validate @DelegatesTo with this approach, unfortunately, nor how to do any kind of meaningful check for static compilation failures. But, you can experiment with modifying the @DelegatesTo and @ClosureParams in Project, and watch the effects on this test.

[bmuschko]

@melix Any thoughts on this TODO?

[melix]

Yes, we need to call methods from the delegate directly in the closure (without it).

[rhencke]

I couldn't figure out a way to use @ClosureParams and directly specify the type in a strongly-typed way, as done with @DelegatesTo. Since it seems to be a common pattern in Gradle to have the closure's argument be the same as the method's return type, I created this. For cases this did not fit, I used SimpleType. If there's a cleaner way to do this or you'd prefer a different approach just let me know.

[rhencke]

The delegate for this seems to be the build script class itself, which didn't end up being all that useful to specify here, and was also marked internal. So, I left off the @DelegatesTo. Same with afterEvaluate and container.

[rhencke]

@melix This should be ready for review but if you need any other changes please just let me know.

[bmuschko]

@melix Would you mind finishing off this PR?

[bmuschko]

@melix Any chance this can still make it into 4.0-RC1?

[melix]

@bmuschko Finally took time to finish the review. There are some pitfalls, which I discovered with the statically compiled Gradle build script experiment months ago, but the changes LGTM.

[bmuschko]

This PR has been merged manually by @melix. Closing the PR. @melix Please don't forget to add the contributor to the release notes.