KAFKA-16564: Apply `Xlint` to java code in core module #16965

m1a2st · 2024-08-22T13:19:21Z

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

We should apply the same xlint to java code in core module before we complete the code migration.

Committer Checklist (excluded from commit message)

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

chia7712 · 2024-09-15T19:10:01Z

build.gradle

      options.compilerArgs << "-parameters"
+    } else if (name == "compileJava") {


please change compileJava to compileScala to add the args to "scala compiler"

chia7712 · 2024-10-07T09:47:45Z

@m1a2st could you please rebase code and fix conflicts?

# Conflicts: # core/src/test/java/kafka/admin/ConfigCommandTest.java

m1a2st · 2024-10-07T13:20:03Z

I have been fix it, Thanks for your reminder.

chia7712 · 2024-10-15T06:50:47Z

@m1a2st could you please rebase code and fix conflicts?

# Conflicts: # core/src/main/java/kafka/server/handlers/DescribeTopicPartitionsRequestHandler.java

chia7712 · 2024-10-18T04:05:25Z

build.gradle

+      options.compilerArgs << "-Xlint:-serial"
+      options.compilerArgs << "-Xlint:-try"
+      options.compilerArgs << "-Werror"
+      options.compilerArgs << "-Xlint:-options"


why we need this option?

The -Xlint:-options is "Warns about the issues relating to use of command line options" . doc
I think we don't need this option, I will remove it.

chia7712

@m1a2st thanks for your patch

chia7712 · 2024-10-19T07:56:15Z

build.gradle

      options.compilerArgs << "-parameters"
+    } else if (name == "compileScala") {


Could you please config both scala and java? for example:

if (name == "compileTestJava" || name == "compileTestScala") { options.compilerArgs << "-parameters" } else if (name == "compileJava" || name == "compileScala") { options.compilerArgs << "-Xlint:all" if (!project.path.startsWith(":connect") && !project.path.startsWith(":storage")) options.compilerArgs << "-Xlint:-rawtypes" options.compilerArgs << "-encoding" << "UTF-8" options.compilerArgs << "-Xlint:-rawtypes" options.compilerArgs << "-Xlint:-serial" options.compilerArgs << "-Xlint:-try" options.compilerArgs << "-Werror" options.compilerArgs += ["--release", String.valueOf(minJavaVersion)] }

chia7712 · 2024-10-19T07:56:36Z

build.gradle

@@ -119,8 +119,16 @@ ext {
  addParametersForTests = { name, options ->
    // -parameters generates arguments with parameter names in TestInfo#getDisplayName.
    // ref: https://github.com/junit-team/junit5/blob/4c0dddad1b96d4a20e92a2cd583954643ac56ac0/junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTest.java#L161-L164
-    if (name == "compileTestJava" || name == "compileTestScala")


Additionally, addParametersForTests -> configureJavaCompiler

m1a2st · 2024-10-19T14:24:31Z

...ct/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/SynchronizationTest.java

-            SecurityManager s = System.getSecurityManager();
-            Thread t = new Thread((s != null) ? s.getThreadGroup() :
-                Thread.currentThread().getThreadGroup(), r,
+            Thread t = new Thread(Thread.currentThread().getThreadGroup(), r,


Java 17 is deprecated the SecurityManager, see doc

Could you please use suppression to fix it? we can go back to dig in it later

chia7712 · 2024-10-19T18:01:20Z

@m1a2st please fix build error

chia7712

@m1a2st thanks for this patch

chia7712 · 2024-10-22T18:34:05Z

build.gradle

+      if (!project.path.startsWith(":connect") && !project.path.startsWith(":storage"))
+        options.compilerArgs << "-Xlint:-rawtypes"
+      options.compilerArgs << "-encoding" << "UTF-8"
+      options.compilerArgs << "-Xlint:-rawtypes"


duplicate config?

configureJavaCompiler also use in the tasks.withType(JavaCompile) and subprojects, it is dulpicate in the tasks.withType(JavaCompile)code block so I remove it which in tasks.withType(JavaCompile)

chia7712 · 2024-10-22T18:35:27Z

...va/org/apache/kafka/common/security/ssl/CommonNameLoggingTrustManagerFactoryWrapperTest.java

@@ -122,7 +122,7 @@ void testNeverExpiringX509Certificate() throws Exception {
                    wrappedCert.hasUnsupportedCriticalExtension());
            // We have just generated a valid test certificate, it should still be valid now
            assertEquals(cert.getBasicConstraints(), wrappedCert.getBasicConstraints());
-            assertEquals(cert.getIssuerDN(), wrappedCert.getIssuerDN());


why we need those changes?

it seens this deprecated method won't cause build error, I will revert it

chia7712 · 2024-10-25T10:49:44Z

build.gradle

@@ -771,7 +773,7 @@ subprojects {
    scalaCompileOptions.additionalParameters += ["-release", String.valueOf(minJavaVersion)]
    options.compilerArgs += ["--release", String.valueOf(minJavaVersion)]


Could you please remove this duplicate config?

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

m1a2st force-pushed the gh-KAFKA-16564 branch from e4afe80 to ff7430a Compare August 25, 2024 08:04

m1a2st force-pushed the gh-KAFKA-16564 branch 2 times, most recently from d3d4aa0 to dc7924f Compare September 10, 2024 19:30

m1a2st added 5 commits September 13, 2024 15:55

setting -Xlint in ScalaCompile

d1f9176

wip

eb12f5d

wip

f07ec15

wip

e4e1ddd

wip

d7c6761

m1a2st force-pushed the gh-KAFKA-16564 branch from 91b39ca to d7c6761 Compare September 14, 2024 06:40

wip

edbad9f

chia7712 reviewed Sep 15, 2024

View reviewed changes

m1a2st added 3 commits September 16, 2024 21:34

wip

bd5f616

change to xlint all

1998aee

fix build gradle

0bf9653

m1a2st marked this pull request as ready for review September 17, 2024 05:37

Merge branch 'trunk' into gh-KAFKA-16564

ec8b3bf

# Conflicts: # core/src/test/java/kafka/admin/ConfigCommandTest.java

github-actions bot added core Kafka Broker small Small PRs labels Oct 7, 2024

fix build error

829d996

Merge branch 'trunk' into gh-KAFKA-16564

0be3b2b

# Conflicts: # core/src/main/java/kafka/server/handlers/DescribeTopicPartitionsRequestHandler.java

github-actions bot added the build Gradle build or GitHub Actions label Oct 15, 2024

m1a2st added the ci-approved label Oct 15, 2024

chia7712 reviewed Oct 18, 2024

View reviewed changes

addressed by comments remove xlint options

c515a12

chia7712 mentioned this pull request Oct 19, 2024

MINOR: Set sourceCompatibility/targetCompatibility properties to Scala Compile task #17546

Merged

chia7712 reviewed Oct 19, 2024

View reviewed changes

github-actions bot added the connect label Oct 19, 2024

github-actions bot added the clients label Oct 19, 2024

m1a2st commented Oct 19, 2024

View reviewed changes

update by comments

99dd032

m1a2st force-pushed the gh-KAFKA-16564 branch from 0701e8c to 99dd032 Compare October 20, 2024 00:35

m1a2st added 3 commits October 20, 2024 11:39

update RemoteLogManager

4f80a2e

Merge branch 'trunk' into gh-KAFKA-16564

00fc37e

remove java compile argument

a641b89

chia7712 reviewed Oct 22, 2024

View reviewed changes

m1a2st added 2 commits October 23, 2024 03:03

revert cert deprecated method

7695e2f

addressed comments

0fb1c43

chia7712 approved these changes Oct 25, 2024

View reviewed changes

chia7712 reviewed Oct 25, 2024

View reviewed changes

addressed comment

3385697

chia7712 merged commit 09d76f9 into apache:trunk Oct 25, 2024
6 checks passed

abhishekgiri23 pushed a commit to abhishekgiri23/kafka that referenced this pull request Nov 2, 2024

KAFKA-16564 Apply Xlint to java code in core module (apache#16965)

d659f45

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-16564: Apply `Xlint` to java code in core module #16965

KAFKA-16564: Apply `Xlint` to java code in core module #16965

m1a2st commented Aug 22, 2024

chia7712 Sep 15, 2024

chia7712 commented Oct 7, 2024

m1a2st commented Oct 7, 2024

chia7712 commented Oct 15, 2024

chia7712 Oct 18, 2024

m1a2st Oct 18, 2024

chia7712 left a comment

chia7712 Oct 19, 2024

chia7712 Oct 19, 2024

m1a2st Oct 19, 2024

chia7712 Oct 23, 2024

chia7712 commented Oct 19, 2024

chia7712 left a comment

chia7712 Oct 22, 2024

m1a2st Oct 22, 2024

chia7712 Oct 22, 2024

m1a2st Oct 22, 2024

chia7712 Oct 25, 2024

		options.compilerArgs << "-parameters"
		} else if (name == "compileJava") {

		options.compilerArgs << "-parameters"
		} else if (name == "compileScala") {

		@@ -771,7 +773,7 @@ subprojects {
		scalaCompileOptions.additionalParameters += ["-release", String.valueOf(minJavaVersion)]
		options.compilerArgs += ["--release", String.valueOf(minJavaVersion)]

KAFKA-16564: Apply Xlint to java code in core module #16965

KAFKA-16564: Apply Xlint to java code in core module #16965

Conversation

m1a2st commented Aug 22, 2024

Committer Checklist (excluded from commit message)

Choose a reason for hiding this comment

chia7712 commented Oct 7, 2024

m1a2st commented Oct 7, 2024

chia7712 commented Oct 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 commented Oct 19, 2024

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KAFKA-16564: Apply `Xlint` to java code in core module #16965

KAFKA-16564: Apply `Xlint` to java code in core module #16965