Skip to content
This repository has been archived by the owner on Aug 19, 2020. It is now read-only.

Provider API recommendations and differences from Groovy DSL #597

Closed
big-guy opened this issue Nov 15, 2017 · 10 comments
Closed

Provider API recommendations and differences from Groovy DSL #597

big-guy opened this issue Nov 15, 2017 · 10 comments

Comments

@big-guy
Copy link
Member

big-guy commented Nov 15, 2017

This was a discussion that came up WRT the Provider API documentation we're adding in 4.4: gradle/gradle#3478

We're recommending that plugin authors expose a single getter for their Property (or Provider) properties.

From the Groovy DSL, we generate methods to allow people to use = in build scripts with Propertys:

class MyTask extends DefaultTask {
    @Input Property<String> someProperty = ...
}

task myTask(type: MyTask) {
   someProperty = "foo"
}

Under the covers, this is calling someProperty.setFromAnyValue(...), which eventually calls Property.set(T) or Property.set(Provider).

I know this isn't quite possible from the Kotlin DSL. @eskatos pointed me to the PropertyStateExtensions and sample https://github.com/gradle/kotlin-dsl/blob/master/samples/provider-properties/build.gradle.kts

This is pretty close, but it has some differences/problems:

  • Non-Kotlin plugins can't really expose a Kotlin property, so how people use a Property in the Kotlin DSL will have to be different
  • Kotlin plugin authors need to write some boilerplate and come up with clever names (message vs messageProperty)
  • Non-Kotlin plugins configuring Kotlin plugins don't have access to the Property.
  • Groovy DSL don't have access to the Property (so they can't use the assignment syntax above)
  • @Optional doesn't play nice with the PropertyStateExtensions.

I don't think the idiomatic way for writing a Gradle plugin in Kotlin needs to be exactly the same as any other language, but we should consider the impact to the Groovy DSL, other plugins (not written in Kotlin) and the general confusion users may have if Propertys are handled differently depending on which language the plugin was written in.

@mkobit
Copy link
Contributor

mkobit commented Nov 15, 2017

Is this something overloaded setters might help solve? It is some more boilerplate for authors but everything is clearly specified through types with no reliance on coffee generation magic.
#380 has some context around that.

The kotlin DSL might then be able to generate those setters if they aren't present, but I'm not sure how that works with type erasure and how it works in Groovy land either.

@eskatos eskatos added this to the 1.0.0 milestone Jan 17, 2018
big-guy added a commit that referenced this issue Feb 23, 2018
…n now

This takes the approach to make Kotlin look more like the explicitness
of Java until we can do something fancier (like setter overloads).

- We want to expose a single getter that returns the Provider/Property
- The GreetingExtension and Greeting task had unnecessarily mutable variables
- Handwritten separate setters/getters for a Property isn't what we want people
  to do.  And I think we don't want authors to expose special methods for
  setting Provider or raw values.

Fixes #597
big-guy added a commit that referenced this issue Feb 23, 2018
…n now

This takes the approach to make Kotlin look more like the explicitness
of Java until we can do something fancier (like setter overloads).

- We want to expose a single getter that returns the Provider/Property
- The GreetingExtension and Greeting task had unnecessarily mutable variables
- Handwritten separate setters/getters for a Property isn't what we want people
  to do.  And I think we don't want authors to expose special methods for
  setting Provider or raw values.

Fixes #597
@JLLeitschuh
Copy link
Contributor

Still not a fan of how the API's will be different for plugins written in Java/Groovy vs Kotlin. I think it will just add additional confusion for new users, but I don't have any better solutions.

@big-guy
Copy link
Member Author

big-guy commented May 1, 2018

I think my original post made it sound like things were more different than they are the same. Everything is the same between Java, Groovy and Kotlin, except for one area.

The pattern for writing plugins is the same for all languages (expose a single getter for the Property). The API for a task written in Java/Groovy/Kotlin is the same and the way you interact with it from your own plugin code is the same.

For Java and Kotlin, you have to use property.set(...) to set values on a Property.
For a plugin written in Groovy (e.g., in buildSrc), you have to use the .set() method just the same. But for the Groovy DSL, we can provide generated and overloaded setters in the DSL.

We could remove the generated setters from the Groovy DSL to make things look the same as Kotlin DSL. That would make everything the same.

@JLLeitschuh
Copy link
Contributor

Are these Property types supposed to be used only on tasks or on both tasks and extensions?

@big-guy
Copy link
Member Author

big-guy commented May 1, 2018

@JLLeitschuh
Copy link
Contributor

JLLeitschuh commented May 1, 2018

Almost all plugins I've ever worked with uses the "Assignment" method for configuring extensions and Tasks.

Eg:

kotlin {
    // Enable coroutines supports for Kotlin.
    experimental.coroutines = Coroutines.ENABLE
}
task<Wrapper>("wrapper") {
    gradleVersion = "4.7"
    distributionType = Wrapper.DistributionType.ALL
}

Exposing an API that doesn't use these sort of assignment setters seems completely unintuitive, especially since users have been trained by all of the various plugins and Gradle's own API that those are what will exist.

Also, is there a clear migration path for plugin developers who already have these setters and want to migrate to the Property API?

It really seems like whether or not the plugin developer is using the Property API is leaking to the user when the user really shouldn't care.

As a plugin developer, I'd rather go for consistency with the existing paradigm for my users.

@eskatos
Copy link
Member

eskatos commented May 2, 2018

We should be able to support simple assignments with overloaded setters once the Kotlin language supports them, see #380 and KT-4075.

@bamboo
Copy link
Member

bamboo commented May 3, 2018

@eskatos But adding a setter would deviate from the suggested pattern, wouldn't it?

The pattern for writing plugins is the same for all languages (expose a single getter for the Property).

Or do you mean something different?

@eskatos
Copy link
Member

eskatos commented May 4, 2018

Right, what I had in mind is the .value property on Provider/Property types instead of .get()/.set(T) functions:

interface Provider<T> {
    T getValue();
}
interface Property<T> extends Provider<T> {
    void setValue(T value);
    void setValue(Provider<? extends T> provider);
}

which would allow to write:

val some = someTask.stringProp.value
// and:
someTask.stringProp.value = "some"
// and with overloaded setters support in Kotlin:
someTask.stringProp.value = provider { "some" }

Then, for a property that allows being set with different types:

interface SomeProperty extends Property<String> {
    void setValue(Object anything);
}

and an implementation doing the conversion from Object to String,
and overloaded setters support in Kotlin,
that would allow to write:

someTask.someProp.value = whatever

Its not as nice as a direct assignment on the Property itself but it may be a viable option.
WDYT?


Considering simple assignment on the Property itself, we would have to generate the overloaded setters for all plugin types as Kotlin extensions at runtime. It sounds both tricky and would rely on the Kotlin setters overload resolution to take extensions into account. Nothing impossible I'd say, but it would have other implications like performance or, as Mike said above, difficulties around nested generics and type erasure.

@JLLeitschuh
Copy link
Contributor

@eskatos You're getting the closest I've seen to what we'd probably want to users to see.
It's still looks/feels a little bit weird. That being said it does provide the ability to assign a value or a provider which is quite nice.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants