Skip to content
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

feat: add kotlin compiler args to gradle plugin #3156

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

taefi
Copy link
Contributor

@taefi taefi commented Jan 14, 2025

Description

Vaadin-Hilla apps can be written in Kotlin with either Gradle or Maven. This PR is only addressing the Gradle side (since it is
the default in SpringBoot projects downloaded from spring initializr. The same can be done for the Maven but in a different PR, since the approach is totally different.

The approach of this PR was to use reflection to dynamically add the compiler arges to the compileKotlin task, so that users don't have to do it manually in their projects. The reflection approach has the following benefits:

  • It only needs the kotlin-reflect dependency to be available in the user projects, and since that dependency is needed in almost all the Kotlin SpringBoot projects because of the limitations of Kotlin closed classes e.g. in spring-data-jpa, no extra dependency is imposed to user projects.

This could've been done in another way, without reflection, as follows:
HillaPlugin.kt

project.plugins.withId("org.jetbrains.kotlin.jvm") {
      project.tasks.withType(KotlinCompile::class.java) { kotlinCompile ->
          // Configure Kotlin compiler options
          kotlinCompile.compilerOptions { kotlin ->
              kotlin.freeCompilerArgs.addAll(
                  listOf(
                      "-Xjsr305=strict",
                      "-Xemit-jvm-type-annotations"
                  )
             )
        }
    }
}

But, this needs KotlinCompile class to be available in the buildScript classpath (the classpath for the plugins and the processing of the build file, not the user application code), user projects need to add a buildScript block in either their build.gradle or settings.gradle to add the following dependency for our plugin:

settings.gradle

buildscript {
    repositories {
        mavenCentral()
    }
    dependencies {
        classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.25")
    }
}

This approach has a benefit that is more robust when upgrading gradle version, and possible errors in case of deprecation/removal of the used API would be easily noticed, while the reflection approach would fail at runtime (which can be covered by tests instead).

Part of #2343

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.60%. Comparing base (efc7da0) to head (6f0dc1c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3156   +/-   ##
=======================================
  Coverage   92.60%   92.60%           
=======================================
  Files          85       85           
  Lines        3164     3164           
  Branches      775      775           
=======================================
  Hits         2930     2930           
  Misses        183      183           
  Partials       51       51           
Flag Coverage Δ
unittests 92.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@cromoteca cromoteca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a comment, but I'm requesting changes to make it visible.

Comment on lines 63 to 64
val compilerOptions = task.javaClass.getMethod("getCompilerOptions").invoke(task)
val freeCompilerArgs = compilerOptions.javaClass.getMethod("getFreeCompilerArgs")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if one of those invokes can return null

Copy link
Contributor Author

@taefi taefi Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, when the Kotlin jvm plugin is applied, the compileKotlin task and its options cannot be null. But, because of executing methods dynamically using reflection, added null check guards for cases such as API removal in future, or other possible edge cases we are not aware of atm.

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

Successfully merging this pull request may close these issues.

2 participants