Inconsistent Java deprecation warnings

[fzakaria]

Description of the bug:

This is a bug report for something I brought up on slack.

I have provided a minimal reproduction code: https://github.com/fzakaria/bazel-xlint-repro

We have a library that contains a @Deprecation annotation.

@Deprecated
public class DeprecatedLib {

    public void oldMethod() {
        System.out.println("This is an old, deprecated method.");
    }

    public static String oldUtility(String input) {
        return "Deprecated: " + input;
    }
}

We have turned on -Xlint:all and -Werror on our Java toolchain.

When we write a test that uses the deprecated class, the Java compiler complains.

import org.junit.Test;

public class DeprecatedLibTest {

    @Test
    public void testDoSomething() {
        DeprecatedLib lib = new DeprecatedLib();
        lib.oldMethod(); // This will show a warning
    }
}

> bazel build //:DeprecatedLibTest --action_env=1=3
INFO: Invocation ID: a25ed412-763d-4c72-96be-b986a465f139
INFO: Analyzed target //:DeprecatedLibTest (0 packages loaded, 0 targets configured).
ERROR: /Users/fzakaria/code/playground/bazel/xlint-repro/BUILD.bazel:18:10: Building DeprecatedLibTest.jar (1 source file) failed: (Exit 1): java failed: error executing Javac command (from target //:DeprecatedLibTest) external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
DeprecatedLibTest.java:7: error: [deprecation] DeprecatedLib in unnamed package has been deprecated
        DeprecatedLib lib = new DeprecatedLib();
        ^
DeprecatedLibTest.java:7: error: [deprecation] DeprecatedLib in unnamed package has been deprecated
        DeprecatedLib lib = new DeprecatedLib();
                                ^

However if we add some changes to the class, such as a static import, the warning seems to no longer be present.

import org.junit.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

/**
 * By *fail* i mean this target builds correctly even though it uses a deprecated library.
 * 
 * You can see the alternative class in DeprecatedLibTest.java which fails to build
 * although it uses the same code with the sole exception of not using jupiter static assertions.
 */
public class DeprecatedLibFailTest {

    @Test
    public void testDoSomething() {
        DeprecatedLib lib = new DeprecatedLib();
        lib.oldMethod(); // This will show a warning
    }
}

> bazel build //:DeprecatedLibFailTest --action_env=1=5
INFO: Invocation ID: 64af9b38-0189-4906-9d04-0bd3a1b4420b
WARNING: Build option --action_env has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
INFO: Analyzed target //:DeprecatedLibFailTest (56 packages loaded, 1616 targets configured).
INFO: Found 1 target...
Target //:DeprecatedLibFailTest up-to-date:
  bazel-bin/DeprecatedLibFailTest
  bazel-bin/DeprecatedLibFailTest.jar
INFO: Elapsed time: 0.860s, Critical Path: 0.58s
INFO: 7 processes: 2 internal, 3 darwin-sandbox, 2 worker.
INFO: Build completed successfully, 7 total action

I am very stumped as to why the import is causing the deprecation notice to be triggered or not.

Which category does this issue belong to?

Java Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

To see the "failure mode" (it passing)

> bazel build //:DeprecatedLibFailTest

To see the "correct mode" (it failing)

> bazel build //:DeprecatedLibTest

Which operating system are you running Bazel on?

MacOS

What is the output of bazel info release?

release 7.6.0

[fzakaria]

Other warnings are exhibiting the same problem.
We have also the raw-types warning that seems to now pop up on what should be considered erroneous changes.

[fmeum]

@cushon

[fzakaria]

I found the issue!

Looks like the use of the Java compiler is not correctly working with -Werror and when there are more warnings present.

You can see this in the reproduction I have.

I run the javac command locally:

> bazel-xlint-repro/external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/javac -source 21 -target 21 DeprecatedLibFailTest.java -cp bazel-out/darwin_arm64-fastbuild/bin/external/junit_jupiter/jar/_ijar/jar/junit_jupiter/jar/downloaded-ijar.jar:bazel-out/darwin_arm64-fastbuild/bin/libDeprecatedLib-hjar.jar:bazel-out/darwin_arm64-fastbuild/bin/external/bazel_tools/tools/jdk/_ijar/TestRunner/rules_java~~toolchains~remote_java_tools/java_tools/Runner_deploy-ijar.jar -Xlint:all -Werror

warning: unknown enum constant API$Status.STABLE
warning: unknown enum constant API$Status.STABLE
warning: unknown enum constant API$Status.STABLE
...
.....
bazel-out/darwin_arm64-fastbuild/bin/external/junit_jupiter/jar/_ijar/jar/junit_jupiter/jar/downloaded-ijar.jar(/org/junit/jupiter/api/Assertions.class): warning: Cannot find annotation method 'status()' in type 'API'
error: warnings found and -Werror specified
Note: DeprecatedLibFailTest.java uses or overrides a deprecated API.
1 error
100 warnings
only showing the first 100 warnings, of 378 total; use -Xmaxwarns if you would like to see more
> echo $?
1

So although only 100 warnings are shown, the exit_code is still 1

In the demo source repository, I augment maxwarns to -1 (don't limit)

diff --git a/BUILD.bazel b/BUILD.bazel
index b98116b..59301e2 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -69,6 +69,7 @@ java_package_configuration(
 java_package_configuration(
     name = "xlint",
     javacopts = [
+        "-Xmaxwarns -1",
         "-Xlint:all",
         "-Xlint:deprecation",
         "-Xlint:-classfile",

bazel build correctly fails now.

> bazel build -s //:DeprecatedLibFailTest
warning: unknown enum constant API$Status.STABLE
DeprecatedLibFailTest.java:15: error: [deprecation] DeprecatedLib in unnamed package has been deprecated
        DeprecatedLib lib = new DeprecatedLib();
        ^
DeprecatedLibFailTest.java:15: error: [deprecation] DeprecatedLib in unnamed package has been deprecated
        DeprecatedLib lib = new DeprecatedLib();
                                ^
error: warnings found and -Werror specified
Target //:DeprecatedLibFailTest failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.230s, Critical Path: 0.08s
INFO: 2 processes: 2 internal.
ERROR: Build did NOT complete successfully

If you set maxwarns to anything lower than the limit, the error is suprressed

diff --git a/BUILD.bazel b/BUILD.bazel
index b98116b..339fb92 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -69,6 +69,7 @@ java_package_configuration(
 java_package_configuration(
     name = "xlint",
     javacopts = [
+        "-Xmaxwarns 1",
         "-Xlint:all",
         "-Xlint:deprecation",
         "-Xlint:-classfile",

> bazel build -s //:DeprecatedLibFailTest
INFO: Found 1 target...
Target //:DeprecatedLibFailTest up-to-date:
  bazel-bin/DeprecatedLibFailTest
  bazel-bin/DeprecatedLibFailTest.jar
INFO: Elapsed time: 0.612s, Critical Path: 0.33s
INFO: 4 processes: 1 internal, 1 darwin-sandbox, 2 worker.
INFO: Build completed successfully, 4 total actions

[cushon]

Thanks for the report.

use -Xmaxwarns if you would like to see more

That gets logged here, I think maybe because they're logged using printLines instead of as regular diagnostics they're getting filtered out by JavaBuilder:

https://github.com/openjdk/jdk/blob/bf8a881f02da14a3d47b30eaf414dea76a1c231f/src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java#L993-L994

JavaBuilder also has custom logic around -Werror to support something like JDK-8350044 that may be interfering here.

I also wonder if we should also raise -Xmaxwarns to a higher value, at Google we have it set to 500, I think it's usually not that helpful to have lower values.

[fzakaria]

@cushon thank you for the links.
I'll take a read shortly. I'm just out for lunch.

My 2c:
I think anything aside from -1 or catching all lint warnings will lead to problems.

This has been propping up for our engineers as failures in the build in other parts of the code base unrelated to the files they are changing.

The disconnected nature of the changes seems to cause some erosion of trust of Bazel.