-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Concise and statically-typed Property<T>
assignment
#9268
Comments
Property
assignmentProperty<T>
assignment
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. |
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? |
@JLLeitschuh kotlin prefers original members over extensions @jnizet types need to expose |
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 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. |
@mkobit That's a lot of overhead to put on plugin authors for ever field on a task they write. |
To be honest I don't get the current "issue" here. The only thing why something should changed here is that the Groovy DSL and the Kotlin DSL should look similar, right? Just my 2 cents 🙃 |
I'm one of the developers running into this trap when I used the new I don't see a problem here for Plugin authors. When I develop a DSL I use The problem is that DSLs in gradle core uses the Except for The // non-lazy property, can be used with `=` in kotlin DSL
var name: String? = null
// lazy property via closure
fun name(block: () -> String?) |
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 |
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 For that reason and increased readability we suggest the following:
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
|
I'm not sure I agree with removing the difference between 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:
I think I would prefer to have the Groovy syntax sugar print a deprecation warning informing users how to properly use the
What is your feeling on just removing (informing users) the Groovy syntax sugar and keeping everything as-is? |
That's not what's in the proposal. We propose to deprecate
|
As stated, we think that it's important to distinguish Property plain value usage and provider wiring because the latter implies later side effects. |
I think @adammurdoch should weigh in, but I think we don't want e.g., we don't want these kinds of things:
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 Here's a concrete example of what the Provider API looks like with Kotlin: I think it would change to this with
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? |
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.
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. |
On the nullability of
|
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 |
I don't see the In hindsight, maybe 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? |
About nullability, with Java 8 we could look into leveraging type parameter annotation ( Property<String> myNonNullableStringProperty = ...
Property<@Nullable String> myNullableStringProperty = ... |
I don't think this is settled yet. |
Might wait for Kotlin/KEEP#309? |
Overloading the But we are working on the Kotlin compiler plugin, that will support overloading |
Current state of Kotlin assignment:
We found some issues with IntelliJ integrations that have to be addressed in IntelliJ:
We might remove the opt-in flag if all the issues are resolved before 8.1, otherwise, we might remove it in 8.2. |
Status Quo
Extension
Groovy DSL
Kotlin DSL (build script or precompiled script)
Potential solutions
Option 1: Kotlin supports equals
=
operator overloadingImplementation
Kotlin DSL (build script or precompiled script)
Groovy DSL
Unchanged
Pros/Cons
Option 2: Rename
Property.set
/get
tosetValue
/getValue
Just the for the plain value, do not rename
set(Provider<T>)
.Kotlin DSL (build script or precompiled script)
Groovy DSL
Pros/Cons
Alternative
Also rename
set(Provider<T>)
tosetValue(Provider<T>)
.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
Kotlin DSL (build script or precompiled script)
Pros/Cons
The text was updated successfully, but these errors were encountered: