Skip to content

KAFKA-18186: set options.release to make Intellij configure suitable language level automatically#18104

Merged
chia7712 merged 7 commits intoapache:trunkfrom
m1a2st:KAFKA-18186
Dec 11, 2024
Merged

KAFKA-18186: set options.release to make Intellij configure suitable language level automatically#18104
chia7712 merged 7 commits intoapache:trunkfrom
m1a2st:KAFKA-18186

Conversation

@m1a2st
Copy link
Copy Markdown
Collaborator

@m1a2st m1a2st commented Dec 9, 2024

Jira: https://issues.apache.org/jira/browse/KAFKA-18186

To enhance the developer experience, when the user runs ./gradlew idea, IntelliJ IDEA should automatically configure the Java settings.

test in my local
CleanShot 2024-12-10 at 00 28 04@2x

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added build Gradle build or GitHub Actions small Small PRs labels Dec 9, 2024
Comment thread build.gradle Outdated
@@ -125,6 +125,7 @@ ext {

options.compilerArgs << "-encoding" << "UTF-8"
options.compilerArgs += ["--release", String.valueOf(releaseVersion)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please try to use options.compilerArgs += ["--release", String.valueOf(releaseVersion)] instead of options.compilerArgs += ["--release", String.valueOf(releaseVersion)]. The former can set the sourceCompatibility automatically (see https://github.com/gradle/gradle/blob/784ffbe7b04e3dd31f4e05e12906efc7a410757f/platforms/jvm/plugins-java-base/src/main/java/org/gradle/api/plugins/JavaBasePlugin.java#L331)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: options.release = releaseVersion -> we should use this instead

@m1a2st
Copy link
Copy Markdown
Collaborator Author

m1a2st commented Dec 10, 2024

I test in my local machine
CleanShot 2024-12-10 at 15 06 57@2x

@Yunyung
Copy link
Copy Markdown
Collaborator

Yunyung commented Dec 10, 2024

It works on my local "IntelliJ IDEA 2024.3 (Community Edition)".

@chia7712
Copy link
Copy Markdown
Member

@agavra @ableegoldman Could you please take a look, as you provided feedback on #17522? With this patch, IntelliJ IDEA can automatically recognize the language level for each module.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 10, 2024

This doesn't seem to work for me. I see "Language Level: 21" for all my modules while using this branch. None of the screenshots above seem to show it working either.

image

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 10, 2024

Adding sourceCompatibility doesn't change things either for me. I also tried deleting compiler.xml (where these settings are seemingly set) and invalidating the IntelliJ cache.

Version details:
IntelliJ IDEA 2024.3.1 (Community Edition)
Runtime version: 21.0.5+8-b631.28 aarch64 (JCEF 122.1.9)
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 10, 2024

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 10, 2024

As far as I can tell, the language level ends up the same as the Java version Gradle is configured to run with. Once I changed the Java version to 17 in the screenshot below, the module language level switched to 17 (after re-running the Gradle import).

image

@chia7712
Copy link
Copy Markdown
Member

@ijuma Thank you for your report. Could you please try using JDK 17 features in the clients module, as here? I encountered the same issue as you and will investigate it later. However, the main improvement is that IntelliJ IDEA can recognize the supported features for different modules, even though the project structure doesn't seem to work correctly

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 10, 2024

Good point @chia7712 - it seems like the internal model for IntelliJ is actually correct even though the settings screen isn't. One other inconsistency I noticed is that IntelliJ seems to only have a single Java standard library association, so whichever SDK is picked at the project level will be the one one used when someone follows a class/method definition (even if the actual version used by some modules may be older).

So, to summarize:

  1. This seems good enough for now, both IntelliJ editor tooltips and compiler output seem to be correct.
  2. We should probably state in the README that configuring the IntelliJ SDK to Java 17 is likely to be the best compromise given that most modules have Java 17 as the minimum version (this matters for code navigation). In fact, it makes sense to cover the expected IntelliJ experience in the README too as it's a bit unintuitive.
  3. We should probably file an issue with IntelliJ to address the settings inconsistency.

2 and 3 are unrelated to this PR, but I think good to do if people have cycles.

@m1a2st
Copy link
Copy Markdown
Collaborator Author

m1a2st commented Dec 11, 2024

Thanks for @ijuma suggestion

We should probably state in the README that configuring the IntelliJ SDK to Java 17 is likely to be the best compromise given that most modules have Java 17 as the minimum version (this matters for code navigation). In fact, it makes sense to cover the expected IntelliJ experience in the README too as it's a bit unintuitive.

I file a Jira to trace it KAFKA-18203

We should probably file an issue with IntelliJ to address the settings inconsistency.

I will also file an issue to intellij community

@ableegoldman
Copy link
Copy Markdown
Member

I'm trying to test this out by setting the language level for the streams module under 'Project Structure', but:

  1. setting this to 11 does not seem to have any effect (eg I can still use Optional#isPresent in the IDE)
  2. refreshing gradle wipes out the language level override and sets it back to "Project Default"

Am I doing something wrong?

@m1a2st
Copy link
Copy Markdown
Collaborator Author

m1a2st commented Dec 11, 2024

Hello @ableegoldman,

We should not set java version in the Project Structure > Project Settings > Modules, when we import the new kafka project, and refresh the gradle build, You can test below in different modules
CleanShot 2024-12-11 at 14 37 32@2x

  1. setting this to 11 does not seem to have any effect (eg I can still use Optional#isPresent in the IDE)
  2. refreshing gradle wipes out the language level override and sets it back to "Project Default"

In Project Structure > Project Settings > Modules it also is project default, but I file a issue to intellij community to trace it.
IDEA-364556

@chia7712
Copy link
Copy Markdown
Member

setting this to 11 does not seem to have any effect (eg I can still use Optional#isPresent in the IDE)

Apologies, but Optional#isPresent was introduced in JDK 8 (Java 8 Documentation), so it should be available in all modules for now.

@ableegoldman
Copy link
Copy Markdown
Member

@chia7712 My bad 😅 Why was I thinking Optional#isPresent was newer than that? Anyways thanks for the sanity check

@m1a2st Tested with a better API and can now verify that this is working as intended. Thanks for the fix!

image

@chia7712 chia7712 changed the title KAFKA-18186: add sourceCompatibility back to build.gradle to allow idea to configure suitable language level automatically KAFKA-18186: set options.release to make Intellij configure suitable language level automatically Dec 11, 2024
@chia7712 chia7712 merged commit 010b9ff into apache:trunk Dec 11, 2024
@chia7712
Copy link
Copy Markdown
Member

This change breaks the "version" used by joint compilation, resulting in the Java code in the core module being compiled into incorrect byte-code. I have opened KAFKA-18242 to address this issue. As a simple solution, we need to add options.compilerArgs += ["--release", String.valueOf(releaseVersion)] back to the Scala compiler due to Gradle issue #13762.

peterxcli pushed a commit to peterxcli/kafka that referenced this pull request Dec 18, 2024
… language level automatically (apache#18104)

Reviewers: "A. Sophie Blee-Goldman" <ableegoldman@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>
tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
… language level automatically (apache#18104)

Reviewers: "A. Sophie Blee-Goldman" <ableegoldman@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Gradle build or GitHub Actions ci-approved small Small PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants