Concise and statically-typed Property<T> assignment

[eskatos]

Status Quo

Extension

public interface Extension {
    Property<String> getDescription();
}
public interface OtherExtension {
    Property<String> getDescription();
}

Groovy DSL

extension {
    description = "My $project.description"
    description = otherExtension.description
    description = provider {
        // compute stuff lazily
    }
}

Kotlin DSL (build script or precompiled script)

extension {
    description.set("My $project.description")
    description.set(otherExtension.description)
    description.set(provider {
        // compute stuff lazily
    })
}

Potential solutions

Option 1: Kotlin supports equals = operator overloading

Implementation

operator fun <T> Property<T>.EQUALS(value: T): Unit = set(value)
operator fun <T> Property<T>.EQUALS(provider: Provider<T>): Unit = set(provider)

Kotlin DSL (build script or precompiled script)

extension {
    description = "My $project.description"
    description = otherExtension.description
    description = provider {
        // compute stuff lazily
    }
}

Groovy DSL
Unchanged

Pros/Cons

• (+) Simple, just add two kotlin extensions
• (+) Groovy and Kotlin DSL look the same
• (-) Kotlin support not likely to happen

Option 2: Rename Property.set/get to setValue/getValue

Just the for the plain value, do not rename set(Provider<T>).

Kotlin DSL (build script or precompiled script)

extension {
    description.set("My $project.description") // DEPRECATED
    description.value = "My $project.description"
    description.set(otherExtension.description)
    description.set(provider {
        // compute stuff lazily
    })
}

Groovy DSL

extension {
    description = "My $project.description" // DEPRECATED
    description.value = "My $project.description"
    description = otherExtension.description
}

Pros/Cons

• (+) Simple, just add two methods, deprecate two old ones
• (+) Explicit difference between using the plain value or connecting to other properties
• (-/+) Will not look the same in Groovy and Kotlin DSL unless we remove sugar from Groovy DSL
• (-) Looks nice for eager assignment only

Alternative

Also rename set(Provider<T>) to setValue(Provider<T>).

extension {
    description.set("My $project.description") // DEPRECATED
    description.value = "My $project.description"
    description.setValue(otherExtension.description)
    description.setValue(provider {
        // compute stuff lazily
    })
}

This makes it worse at first until Kotlin supports overloaded property setters gradle/kotlin-dsl-samples#380

BUT it becomes semantically wrong, value = provider doesn't make much sense.

Option 3: Kotlin invoke operator overloading

Implementation

operator fun <T> Property<T>.invoke(value: T?): Unit = set(value)
operator fun <T> Property<T>.invoke(provider: Provider<T>): Unit = set(provider)

Kotlin DSL (build script or precompiled script)

extension {
    description("foo")
    description(otherExtension.description)
    description(provider {
        // compute stuff lazily
    })
}

Pros/Cons

• (+) Simple, just add two kotlin extensions
• (-) Still not assignments

[JLLeitschuh]

Option 3 might also collide with plugin author's extensions where they've defined

fun description(a: String) {
    // ...
}

This might be an non-issue though.

[jnizet]

And simply not exposing properties or type Property directly, but exposing plain old String/whatever properties delegating to private Property fields isn't an option, even for incubating classes?

[eskatos]

@JLLeitschuh kotlin prefers original members over extensions

@jnizet types need to expose Property<T> JavaBean properties otherwise it means giving up on lazy configuration

[mkobit]

I wonder if setter overloading (KT-4075) or setter-only properties (KT-6519) would allow something like this to work? Seems like there might be a few missing Kotlin feautre requests (like how do you have setters for a val?), but maybe something that looks like this?

open class MyTask : DefaultTask() {
  val myProp: Property<String> = project.objects.property()
  // ignore that this is val for sake of example
    set(value: String) {
      myProp.set(value)
    }
    set(value: Provider<out String) {
      myProp.set(value)
    }
}
task<MyTask>("myTask") {
  myProp = "value"
  myProp = provider { "value" } 
}

Maybe accessor generation could generate these setters?

Just brainstorming if there are other options.

[JLLeitschuh]

@mkobit That's a lot of overhead to put on plugin authors for ever field on a task they write.

[StefMa]

To be honest I don't get the current "issue" here.
I mean - when a Kotlin developer take a look to the current javadoc they see that they have to call set and don't "assign".

The only thing why something should changed here is that the Groovy DSL and the Kotlin DSL should look similar, right?
But none of your options can solve that.
From my point view nothing should be changed here. Kotlin is not Groovy (for good reasons). So the API is slightly different. No need to adjust Gradle/Kotlin here to be similar to Groovy.

Just my 2 cents 🙃

[passsy]

I'm one of the developers running into this trap when I used the new MavenPom DSL using Property<T>. I first implemented it in Groovy and then tried to use it in Kotlin in a different project and failed hard. It took me hours to find the .set(T). Using it is a dissatisfying experience.

I don't see a problem here for Plugin authors. When I develop a DSL I use var name: String? = null and it can be used the same way from kotlin and groovy.

The problem is that DSLs in gradle core uses the Property API which makes DSL inconsistent. Sometimes = can be used (i.e. in android DSL), sometimes .set(T) is required.

Except for Option 1: Kotlin supports equals = operator overloading which would restore a consistent DSL experience Option 2 and Option 3 also break consistency.

The Property<T> API was introduced because properties can now be lazily evaluated. That's a cool feature but often not required. I'm really unhappy that this changes how DSLs are used. As a plugin author, I'd much rather provide an alternative API for lazy properties.

// non-lazy property, can be used with `=` in kotlin DSL 
var name: String? = null

// lazy property via closure
fun name(block: () -> String?)

[JLLeitschuh]

The Property API was introduced because properties can now be lazily evaluated. That's a cool feature but often not required.

Actually, it is required to fix a lot of issues with task evaluation ordering that has plagued Gradle for a long time. The goal is to provide a mechanism where plugin authors do not have to utilize the Project::afterEvaluate method.

[eskatos]

The position of the @gradle/kotlin-dsl team is to advocate for Option 2 with minor changes.

We think that it's important to distinguish Property<T> plain value usage and provider wiring because the latter implies later side effects.

For that reason and increased readability we suggest the following:

• Introduce a value: T? property i.e. @Nullable T getValue() / void setValue(@Nullable T)
• Deprecate @Nullable T getOrNull() in favor of @Nullable T getValue()
• Deprecate void set(@Nullable T) in favor of void setValue(@Nullable T)
• Introduce from(Provider<T>) for wiring
• Deprecate void set(Provider<T>) in favor of void from(Provider<T>)

Kotlin DSL

extension {
    description.value = "My $project.description"
    description.from(otherExtension.description)
    description.from(provider {
        // compute stuff lazily
    })
}

Groovy DSL

extension {
    description.value = "My $project.description"
    description.from otherExtension.description
    description.from provider {
        // compute stuff lazily
    }

    // Additional existing Groovy DSL sugar
    description = "My $project.description"
    description = otherExtension.description
}

For trying it out

Put the following extensions in scope:

var <T> Property<T>.value: T?
    get() = getOrNull()
    set(value) = set(value)

fun <T> Property<T>.from(provider: Provider<T>) =
    set(provider)

Pros/Cons

• (+) Simple, just add a few methods, deprecate old ones (doesn't require any compiler plugins, bytecode munging etc..)
• (+) Explicit difference between using the plain value or connecting to other properties
• (-/+) Will not look the same in Groovy and Kotlin DSL unless we remove sugar from Groovy DSL
• (-) Deprecations might affect the perception of the Gradle API, changing too fast

[lacasseio]

I'm not sure I agree with removing the difference between getOrNull() and get() as it adds some intention behind the querying of the property/provider.

Could option 4 be a viable solution as well. I understand it doesn't feel property-ish in Java and Kotlin. I fear that option 2 will create 3 ways to understand the Provider API:

extension {
    // Valid in Groovy
    description = "some-value"

    // Valid in Groovy and Kotlin
    description.value = "some-value"

    // Valid in Java, Groovy and Kotlin
    description.setValue("some-value")
}

I think I would prefer to have the Groovy syntax sugar print a deprecation warning informing users how to properly use the Property object and pointing them to use set until Gradle 6.0 or further (if we don't convert the entire code base to use provider API by then). This way, we would have a single way of setting values for Property in all language:

extension {
    // Valid in Java, Groovy, and Kotlin
    description.set("some-value")
    //  Note: in Java is would be getDescription().set(...)

    // Normal groovy syntax sugar
    description.set "some-value"
}

What is your feeling on just removing (informing users) the Groovy syntax sugar and keeping everything as-is?

[bamboo]

I'm not sure I agree with removing the difference between getOrNull() and get()

That's not what's in the proposal. We propose to deprecate getOrNull() in favor of getValue() with the same signature and semantics.

get() remains unaffected by this proposal.

[bamboo]

What is your feeling on just removing (informing users) the Groovy syntax sugar and keeping everything as-is?

As stated, we think that it's important to distinguish Property plain value usage and provider wiring because the latter implies later side effects.

[big-guy]

I think @adammurdoch should weigh in, but I think we don't want T getValue() and void setValue(T) to be the easy option. We want the pattern for assigning a property to be consistent, that doesn't mean the same method names, but it means that if you use =, you should always use =. I think Adam just recently added a value(T) to Property, so he might have some direction in mind already.

e.g., we don't want these kinds of things:

val myName = objects.property<String>()
val helloTo = objects.property<String>()
...
myName.value = "Me"
helloTo.value = myName.value // not wired together
...
myName.value = "Me"
helloTo.from(myName) // inconsistent

I think we also want to handle some other conditions that aren't just "set this value or provider". e.g., default values, which might make a separate getValue/setValue method a bit strange.

Here's a concrete example of what the Provider API looks like with Kotlin:
https://raw.githubusercontent.com/gradle/kotlin-dsl/master/samples/provider-properties/build.gradle.kts

I think it would change to this with getValue/setValue:

apply<GreetingPlugin>()

fun buildFile(path: String) = layout.buildDirectory.file(path)

configure<GreetingPluginExtension> {

    message.value = "Hi from Gradle"

    outputFiles.from(
        buildFile("a.txt"),
        buildFile("b.txt"))
}

open class GreetingPlugin : Plugin<Project> {

    override fun apply(project: Project): Unit = project.run {

        // Add the 'greeting' extension object
        val greeting = extensions.create(
            "greeting",
            GreetingPluginExtension::class,
            project)

        // Add a task that uses the configuration
        tasks {
            register("hello", Greeting::class) {
                group = "Greeting"
                message.from(greeting.message)
                outputFiles.setFrom(greeting.outputFiles)
            }
        }
    }
}

open class GreetingPluginExtension(project: Project) {

    val message = project.objects.property<String>()

    val outputFiles: ConfigurableFileCollection = project.files()
}

open class Greeting : DefaultTask() {

    @get:Input
    val message = project.objects.property<String>()

    @get:OutputFiles
    val outputFiles: ConfigurableFileCollection = project.files()

    @TaskAction
    fun printMessage() {
        val message = message.value
        val outputFiles = outputFiles.files
        logger.info("Writing message '$message' to files $outputFiles")
        outputFiles.forEach { it.writeText(message) }
    }
}

I don't know if the distinction between "this is a simple value" and "this is a provider" is something that's useful from a user's POV. What side effects do you have in mind?

[bamboo]

What side effects do you have in mind?

Anything that can happen between the wiring of the property and its consumption that can affect its value.

For a simple value assignment there are none because value semantics. For wiring the value you "set" is no value at all but a promise of a future value.

Again, I think there's value in making operations that have different semantics, value assignment versus property wiring, for instance, look different.

it means that if you use =, you should always use =.

As an aside, I understand this might be standard Groovy practice but changing the meaning of the assignment operator and even giving it a different connotation depending on the type of the assigned value seems confusing.

[JLLeitschuh]

On the nullability of value

RE: @bamboo's sample:

var <T> Property<T>.value: T?
    get() = getOrNull()
    set(value) = set(value)

I think for this to be truly representative, the "try it out" needs to be:

val <T> Provider<T>.value: T?
    get() = getOrNull()

var <T> Property<T>.value: T?
    get() = getOrNull()
    set(value) = set(value)

I'm not a huge fan of having value be null by default. As a plugin author writing a task, there are certain properties that I know should never be null; for example, if the task provides a default value.

It's unfortunate that the Provider interfaces are all implemented in Java and the T type on the generic can't capture the "nullness" of the possible return values.

There might be some advantage to converting the Provider and associated interfaces to Kotlin. I believe it could be done without adding any dependencies on the Kotlin standard library for Groovy DSL users.

On the method rename topic

I agree with @big-guy that this would be nice to avoid as it could cause undue confusion.

helloTo.value = myName.value // not wired together

I've already had to help the detekt team fix these problems in their plugin. This sort of confusion has arisen with other developers I've helped write plugins.

detekt/detekt#1181

Many plugin developers new to the Provider API assume that they can just do this:

apply<GreetingPlugin>()

fun buildFile(path: String) = layout.buildDirectory.file(path)

configure<GreetingPluginExtension> {

    message= "Hi from Gradle"

    format = GreetingFormat.XML

    outputFiles.from(
        buildFile("a.txt"),
        buildFile("b.txt"))
}

enum class GreetingFormat {
    XML, CSV;
}

open class GreetingPlugin : Plugin<Project> {

    override fun apply(project: Project): Unit = project.run {

        // Add the 'greeting' extension object
        val greeting = extensions.create(
            "greeting",
            GreetingPluginExtension::class,
            project)

        // Add a task that uses the configuration
        tasks {
            register("hello", Greeting::class) {
                group = "Greeting"
                message.set(greeting.message)
                format.set(greeting.format)
                outputFiles.setFrom(greeting.outputFiles)
            }
        }
    }
}

open class GreetingPluginExtension(project: Project) {

    var message: String?

    var format: GreetingFormat = GreetingFormat.CSV

    val outputFiles: ConfigurableFileCollection = project.files()
}

open class Greeting : DefaultTask() {

    @get:Input
    val message = project.objects.property<String>()

    @get:Input
    val format = project.objects.property<GreetingFormat>()

    @get:OutputFiles
    val outputFiles: ConfigurableFileCollection = project.files()

    @TaskAction
    fun printMessage() {
        val message = message.value
        val outputFiles = outputFiles.files
        // TODO: Do something with the format
        logger.info("Writing message '$message' to files $outputFiles")
        outputFiles.forEach { it.writeText(message) }
    }
}

The terrible thing about this is that this completely works in tests and in isolated sample projects.

I've had to carefully explain to plugin developers that this logic completely stops working if anyone has any logic at all that forces the configuration of their task before the configure block runs.
The best way I've come up with to demonstrate this is to ask devs to add this to their test scripts:

tasks.all { }

configure<GreetingPluginExtension> {
    // ...

I'm concerned that by adding, void setValue(@Nullable T) this confusion is just going to get worse for new developers writing plugins.

However, the downside of trying to prevent plugin authors from shooting themselves in the foot is that the API for plugin consumers isn't as friendly.

One way this problem could be mitigated is the Gradle test runner could, by default, eagerly configure all tasks by default with an opt-out option. However, this won't help projects that don't write unit tests and just test using sample projects via included builds.

[adammurdoch]

I strongly prefer that whatever convenience that we add to query the value or to set the value should be non-nullable. I think we want explicit ways for the code that queries the value and the code that sets the value to express whether they expect a null value, and we want the most convenient and obvious syntax to match what the author of the code most likely expects wrt nullability.

I also prefer that the most convenient and obvious syntax does not encourage people to unpack the value of an upstream provider, as this is the worst of all possible outcomes when this happens. Then all we've done is add a bunch of complexity with none of the performance, comprehension, traceability, validation, correctness, decoupling, etc benefits.

I'd be ok with something like option 2, if we also provide some mechanism to prevent the early unpacking that it encourages. This wouldn't necessarily need to be a syntax mechanism. For example, we might add a bunch of validation at various points.

Regardless of whatever we come up with, we should take the opportunity to get rid of the differences between the Java, Groovy and Kotlin usage where it makes sense.

To clarify something wrt side effects, there's no guarantee that there won't be side effects when a value is provided using set() (or whatever might replace it). For example, GString instances are converted to String, Iterable instances are converted to an immutable collection of the relevant type. We'll add further normalisations in the future, and other ways to mutate and otherwise mess with the provided value to produce the final value. So I'm not sure that side effects are really that useful distinction to use in this particular API, because the categories are "may have side effects" and "slightly less likely to have side effects" not "may have side effects" and "may not have side effects".

[big-guy]

I don't see the .set() as being especially cumbersome given the different alternatives. The Groovy DSL is the only odd one out (since it can use =), but plugins written in Groovy, Java or Kotlin all have to use .set().

In hindsight, maybe from would have been a better name than set. Since you'd have things like extension.description.from("my description") and mytask.description.from(extension.description). At least to me, from feels more like get the value from here vs set which sounds like it might go get the value right now. This is similar to FileCollection too.

Is there some other reason that we would want a different alternative than just the look and feel of the pattern? e.g., is there a problem with the current methods that allow you to do something unsafe or dubious in Kotlin?

[eskatos]

About nullability, with Java 8 we could look into leveraging type parameter annotation (target = ElementType.TYPE_PARAMETER) so that nullability can be declared at property declaration site, in Kotlin or in Java. Here in java:

Property<String> myNonNullableStringProperty = ...

Property<@Nullable String> myNullableStringProperty = ...

[bamboo]

What we would like to do is to make = work with properties of type Property and friends.

I don't think this is settled yet.

[Goooler]

Might wait for Kotlin/KEEP#309?

[asodja]

Overloading the = operator as described in the KEEP probably won't be a language feature, at least in a short term.

But we are working on the Kotlin compiler plugin, that will support overloading = for Kotlin Gradle DSL. It was not possible to write a plugin till recently since it required some changes in the Kotlin compiler. So if everything goes according to plan we will ship the experimental opt-in assignment for Property<T> in some 8.x version.

[asodja]

Current state of Kotlin assignment:

• Kotlin plugin integration was done in Kotlin DSL assignment plugin integration #22240 and is merged to master
• = for Property type for Kotlin can be enabled with systemProp.org.gradle.unsafe.kotlin.assignment=true. To make it work with IntelliJ this property has to be in the project root gradle.properties file
• Dogfood PR for gradle/gradle is in Dogfood Kotlin DSL assignment in build kt and kts files #23547

We found some issues with IntelliJ integrations that have to be addressed in IntelliJ:

1. Cannot use "Go to declaration" for properties on Java types when = is used (for Kotlin types it works ok), e.g.:

2. IntelliJ incorrectly shows compile error for properties on extensions when using <extension>.<property> = <value> notation, e.g.:

3. Imports are not recognized in build logic .kt files

We might remove the opt-in flag if all the issues are resolved before 8.1, otherwise, we might remove it in 8.2.