Gradle: Kotlin-based DSL is more magic than Kotlin and leads to confusion

[ducrohet]

Quick note: I realize that not all of the following is specific to the new Kotlin-based DSL and may also be present in the Groovy based DSL. I would argue that the dynamic feature of Groovy make this less surprising than it does for Kotlin. (Disclaimer: I’m not really a Groovy user and its dynamic aspect often feel too magic-y for my liking)

There are some aspects of the Kotlin-based DSL that are unexpected to normal Kotlin developers and that make things really confusing, surprising and possibly hard to figure out/debug. Here are 2 examples.

1. Direct access to the root project object from anywhere.

The following piece of DSL works:

android {
  buildTypes {
    debug {
      dependencies {
        implementation ...
      }
    }
  }
}

We see this repeatedly in our users build files, with the expectation that the dependency is specific to the enclosing build type. This is obviously wrong. This works in both Groovy and Kotlin, and I originally thought it was a feature of Groovy but it looks like you managed to do the same in Kotlin!

Here’s another real life example that we saw from a developer that was quite tricky to troubleshoot:

android {
  buildTypes {
    debug {
      extra[“foo”] = “false”
    }
  }
}

This compiles fine but does not add the “foo” extra property on the debug build type. This is because our type (BuildType) does not officially implement ExtensionAware. The instance is created using an ObjectFactory, so the decorated object actually does implement it but not the declared type in the container.

The fact that it works is very counter-intuitive. The same piece of code in a Kotlin file in buildSrc would simply not compile.

To solve this, one would need to use:

android {
  buildTypes {
    debug {
      (this as ExtensionAware).extra[“foo”] = “false”
    }
  }
}

Here’s another random issue found on stackoverflow: https://stackoverflow.com/questions/18534313/gradle-applicationvariants-all-skips-one-variant
Obviously the user was confused and because the code compile it’s hard to understand what’s happening.

Proposed solution: Stop doing this :)
Kotlin has qualified this using labels. It would make the DSL a bit more verbose but oh so much more explicit and obvious. Just need a top level label called “project” and one could do:

android.applicationVariants.all {
  [email protected](...)
}

When the label is not provided then compilation will fail as expected instead of magically doing something that is not what the user wants.
(Bonus point for finding a match in an outer scope and providing hints on where the not found method/property can be found).

Alternative possibility, make all decorated classes have a project property so that you can write:

android.applicationVariants.all {
  project.tasks.register(...)
}

This looks nicer in the DSL but this is still a departure from the normal API.

1. Automatic replacement (or augmentation) of foo(Action<Foo>) with foo(Foo.() -> Unit)

I totally get why this is done. However there is no difference between places where it should be done (to make the DSL nice) and places where it should not be done (regular API)

For instance, .all on DomainObjectCollection gets replaced but this is not the case in Groovy, so this is actually a breaking change

Groovy:

android.applicationVariants.all { variant ->
 println(variant.fullName)
}

KTS:

android.applicationVariants.all {
 println(fullName)
}

So this is both a breakage from the Groovy-based DSL, and again a breakage compared to the same code in a Kotlin file in buildSrc.

I just stumbled upon this code from a dev: https://github.com/ZacSweers/CatchUp/pull/149/files#diff-863e98cb3c8f40e0692c9d27cd349e29R68

As you can see TaskProvider.configure(Action<Task>) is replaced with TaskProvider.configure(Task.() -> Unit). But this is not really a DSL focused API, and I don’t think this should be augmented.

As I mentioned, I understand why it’s done. It’s hard to know which ones to augment and which ones not to. It seems like it would be better to let the API author decide. Obviously this would be a breaking change but I cannot come up with a better idea.

So a proposed solution would be to introduce an annotation that a plugin author would use on a method receiving an Action, to specify that these are DSL methods and that they should be augmented. This would only be used by Java based API as Kotlin ones could just create the right method anyway.
It would obviously take a long time for the new behavior to be enabled by default.

Actually, as I type this, I discover HasImplicitReceiver. Maybe the issue then is that there are APIs in Gradle that are not DSL-focused APIs that use Action instead of using something else. (I don’t think you actually have that something else right now?)

(I’m also not sure why DomainObjectCollection.all does not end up behaving the same in Groovy-based DSL. Maybe it’s because there’s a version with a Closure?)

Anyway, this is clearly complicated to fix properly, but I think the current DSL, in order to stay similar to the Groovy one, ends up being more confusing for developers who like to look at Javadoc rather than DSL docs.
In fact, the DSL reference is often not detailed enough in what the property types are, that one has to go to the Javadoc. At this point you only see the non-augmented class and things get out of sync between the doc and the actual DSL behavior.
There are also cases where the DSL reference just link to the Javadoc. For instance looking for details of ForkOption here: https://docs.gradle.org/current/dsl/org.gradle.api.tasks.compile.CompileOptions.html#org.gradle.api.tasks.compile.CompileOptions:forkOptions gets you to the javadoc.

I think it’s really important that the DSL not be something magic and be something that can easily be used by just looking at the Javadoc/Kdoc of the used plugin. Trying to make it something more just leads to confusion. Maintaining full DSL reference explaining all these complicated, different behavior is not worth the hassle, and just relying on the Java/KDoc as the only source of truth should be the goal. In my opinion of course.

[eskatos]

Thank you for the report @ducrohet!
I agree those things are confusing. Some first thoughts below.

About 1)

As you understood, the main difference with Groovy comes from the fact that Kotlin statically resolves the receiver and BuildType didn't expose to the type system that it is ExtensionAware. Groovy tries each possible receiver at runtime.

https://github.com/gradle/gradle/issues/9255 is related to your first solution suggestion and may be of interest.

There's also the @GradleDsl annotation that could be generalized to be applied to both Kotlin and Groovy scripts for consistency.

About 2)

Yes, Action<T> annotated @HasImplicitReceiver is for DSL use and remove the need for it. in Kotlin scripts.

Closure taking overloads blur the difference because then in Groovy scripts, the closure receiver is the same as it so both syntaxes work.

A good candidate instead of Action<T> for "non-dsl" functions could be Consumer<T> now that we require Java 8. But ambiguous overloads might block the migration path.

For buildSrc, if you apply the kotlin-dsl plugin in buildSrc/build.gradle.kts then you'll get the same behavior in .kt files in buildSrc/src/main/kotlin than in .gradle.kts scripts. This could be the default.

[stale[bot]]

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.

[ZacSweers]

Commenting because this is actually pretty important