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

"error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code" flagged by error prone #483

Closed
davido opened this issue Apr 30, 2020 · 6 comments

Comments

@davido
Copy link
Contributor

davido commented Apr 30, 2020

I bumped error prone to 2.3.4 in Bazel, and conducted custom java tools release to catch more warnings in Gerrit Code Review project.

With bumped error prone version I see this breakage in Gerrit build, in rules_closure:

  $ bazel build --java_toolchain=@remote_java_tools_linux//:toolchain --host_java_toolchain=@remote_java_tools_linux//:toolchain :release
  [...]
INFO: Analyzed target //:release (129 packages loaded, 787 targets configured).
INFO: Found 1 target...
ERROR: /home/davido/.cache/bazel/_bazel_davido/5c01f4f713b675540b8b424c5c647f63/external/io_bazel_rules_closure/java/io/bazel/rules/closure/BUILD:40:13: Building external/io_bazel_rules_closure/java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/external/io_bazel_rules_closure/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:8: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
 final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
       ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)
Target //:release failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 44.028s, Critical Path: 26.56s
INFO: 69 processes: 4 remote cache hit, 47 linux-sandbox, 2 local, 16 worker.
FAILED: Build did NOT complete successfully

Workaround for now would be to disable that check or downgrade the severity to warning.

@davido
Copy link
Contributor Author

davido commented May 7, 2020

Reproducer.

Apply this diff:

diff --git a/WORKSPACE b/WORKSPACE
index c6cad80..f1c1f87 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -7,6 +7,23 @@ load("@io_bazel_rules_closure//closure:repositories.bzl", "rules_closure_depende
 rules_closure_dependencies()
 rules_closure_toolchains()
 
+http_archive(
+    name = "remote_java_tools_linux",
+    sha256 = "74d30ccf161c58bb69db9b2171c954a0563b2d1ff6f5831debbe71ced105c388",
+    urls = [
+        "https://github.com/davido/java_tools/releases/download/javac11-v11.0/java_tools_javac11_linux-v11.0.zip",
+    ],
+)
+
+http_archive(
+    name = "remote_java_tools_darwin",
+    sha256 = "252520f0cd5dd7e9b18062dc731f8ae248993650f12a9b613fcd9ebda591d242",
+    urls = [
+        "https://github.com/davido/java_tools/releases/download/javac11-v11.0/java_tools_javac11_darwin-v11.0.zip",
+ ],
+)
+
+
 http_archive(
     name = "bazel_skylib",
     sha256 = "bbccf674aa441c266df9894182d80de104cabd19be98be002f6d478aaa31574d",

And run:

  $ bazel build --java_toolchain=@remote_java_tools_linux//:toolchain --host_java_toolchain=@remote_java_tools_linux//:toolchain //...
INFO: Analyzed 1357 targets (136 packages loaded, 1603 targets configured).
INFO: Found 1357 targets...
ERROR: /home/davido/projects/davido_rules_clsoure/java/io/bazel/rules/closure/BUILD:40:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:8: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
 final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
       ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)
INFO: Elapsed time: 71.663s, Critical Path: 4.61s
INFO: 11 processes: 7 linux-sandbox, 4 worker.
FAILED: Build did NOT complete successfully

@gkdn
Copy link
Collaborator

gkdn commented May 7, 2020

Went over this. I don't see rules_closure doing anything special around auto value.

Looking at error-prone: the class is suppose to be skipped by javax.annotation.Generated, javax.annotation.processing.Generated annotation. And AutoValue would add that annotation if it exists.

So things you can try:

  1. Check if AutoValue is adding Generated. If it s adding that, you should seek help on error prone side.
  2. If it is not adding it: try upgrading AutoValue just in case to if there was any improvements around that and also check if your custom toolchain has the annotation.

But anyway I don't think there is much to do on the rules_closure if upgrading AutoValue doesn't help.

@davido
Copy link
Contributor Author

davido commented May 7, 2020

@gkdn Thanks for looking into it.

I think the issue is similar to rules_kotlin, see my PR here: [1], and the issue here: [2]. auto-value cannot add @generated annotation, because it is not on the classpath: javax.annotation dependency is missing. And javax.annotation.processing.Generated cannot be used, because source language level is Java 8 (that's the default in Bazel).

If you check the generated files in rules_closure, there is no annotation emitted by auto-value, only this comment:

vim bazel-out/host/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webfile.java

package io.bazel.rules.closure.webfiles;

import io.bazel.rules.closure.Webpath;
import java.nio.file.Path;

// Generated by com.google.auto.value.processor.AutoValueProcessor
 final class AutoValue_Webfile extends Webfile {

We wouldn't care much that the @generated annotation is missing on generated files, but the new Error Prone version 2.3.4, that I'm trying to upgrade in Bazel (in this PR: [3]) is flagging the code as invalid.

I will upload PR for review in a moment.

[1] bazelbuild/rules_kotlin#320
[2] bazelbuild/rules_kotlin#321
[3] bazelbuild/bazel#11271

davido added a commit to davido/rules_closure that referenced this issue May 7, 2020
davido added a commit to davido/rules_closure that referenced this issue May 7, 2020
@davido
Copy link
Contributor Author

davido commented May 7, 2020

With the referenced PR, the @generated annotation is now there and this makes new version of Error Prone totally happy, no failure any more:

vim bazel-out/host/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webfile.java

package io.bazel.rules.closure.webfiles;

import io.bazel.rules.closure.Webpath;
import java.nio.file.Path;
import javax.annotation.Generated;

@Generated("com.google.auto.value.processor.AutoValueProcessor")
 final class AutoValue_Webfile extends Webfile {

  private final Webpath webpath;
  private final Path zip;
  private final String label;
  private final BuildInfo.WebfileInfo info;

davido added a commit to davido/rules_closure that referenced this issue May 8, 2020
davido added a commit to davido/rules_closure that referenced this issue May 8, 2020
davido added a commit to davido/rules_closure that referenced this issue May 8, 2020
@davido
Copy link
Contributor Author

davido commented May 17, 2020

The reason why @generated annotation is missing is the same as pointed out by @cushon in google/error-prone#1348 (comment).

rules_closure is building also for host configuration. So the situation before applying the PR linked to this issue is already broken, and Error Prone upgrade only detects and reports it:

  $ java -fullversion
openjdk full version "1.8.0_242-b08"

  $ bazel build //...
  $ cd bazel-out

  $ find . -name "Auto*"
  ./k8-fastbuild/bin/javatests/io/bazel/rules/closure/worker/testing/_javac/testing/libtesting_sourcegenfiles/io/bazel/rules/closure/worker/testing/AutoValue_ProgramResult.java
./k8-fastbuild/bin/javatests/io/bazel/rules/closure/worker/testing/_javac/testing/libtesting_classes/io/bazel/rules/closure/worker/testing/AutoValue_ProgramResult.class
./k8-fastbuild/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java
./k8-fastbuild/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_classes/io/bazel/rules/closure/AutoValue_Tarjan_Result.class
./k8-fastbuild/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_classes/io/bazel/rules/closure/webfiles/AutoValue_Webset.class
./k8-fastbuild/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_classes/io/bazel/rules/closure/webfiles/AutoValue_Webfile.class
./k8-fastbuild/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webfile.java
./k8-fastbuild/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webset.java
./k8-fastbuild/bin/java/io/bazel/rules/closure/worker/_javac/worker/libworker_classes/io/bazel/rules/closure/worker/AutoValue_InputCache_Key.class
./k8-fastbuild/bin/java/io/bazel/rules/closure/worker/_javac/worker/libworker_sourcegenfiles/io/bazel/rules/closure/worker/AutoValue_InputCache_Key.java
./host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java
./host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_classes/io/bazel/rules/closure/AutoValue_Tarjan_Result.class
./host/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webfile.java
./host/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webset.java
./host/bin/java/io/bazel/rules/closure/worker/_javac/worker/libworker_classes/io/bazel/rules/closure/worker/AutoValue_InputCache_Key.class
./host/bin/java/io/bazel/rules/closure/worker/_javac/worker/libworker_sourcegenfiles/io/bazel/rules/closure/worker/AutoValue_InputCache_Key.java

Now, as expected, in ./k8-fastbuild/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue Webfile.java, the annotation is there:

package io.bazel.rules.closure.webfiles;

import io.bazel.rules.closure.Webpath;
import java.nio.file.Path;
import javax.annotation.Generated;

@Generated("com.google.auto.value.processor.AutoValueProcessor")
final class AutoValue_Webfile extends Webfile {
[...]

But in: ./host/bin/java/io/bazel/rules/closure/webfiles/_javac/webfiles/libwebfiles_sourcegenfiles/io/bazel/rules/closure/webfiles/AutoValue_Webfile.java, it is missing:

package io.bazel.rules.closure.webfiles;

import io.bazel.rules.closure.Webpath;
import java.nio.file.Path;

// Generated by com.google.auto.value.processor.AutoValueProcessor
final class AutoValue_Webfile extends Webfile {
[...]

The missing annotation generation is using -source 8 -target 8 with a JDK 11 -bootclasspath (because it's targeting the remote JDK 11). So AutoValue processor is trying to use javax.annotation.Generated based on the source level, but it isn't available because the JDK 11 bootclasspath only contains javax.annotation.processing.Generated.

davido added a commit to davido/bazel that referenced this issue May 19, 2020
Fixes bazelbuild#11446.

Java tools are decoupled from Bazel distribution and downloaded from
java_tools repository.

If some of the tools is updated, the archive name is changed in Bazel
and new Bazel release is conducted. If, however, new tool has some
breaking changes, then the upgrade needs to be rolled back.

To make user experience with upgrading tools less disruptive but still
not unnecessary procrastinate releases of important Java tools like
javac and Error Prone --incompatible_use_java_tools_beta_release is
added and CI-job should be set up to check all downstream projects
against upcoming java_tools release.

Test Plan:

1. Upgrade Error Prone version to 2.3.4 Bazel that is known to have
breaking changes and will break all Bazel users who rely on cross
compilation use case. There are ongoing efforts to demote the offensive
EP checks to warning severity instead of error, though: [1].

2. Bump java_tools in beta_release archive to new java_tools
   distribution that includes Error Prone 2.3.4

3. Build rules_closure with --incompatible_use_java_tools_beta_release
   would fail with the known issue, because of unsatisfied dependency
   on javax.annotation:

ERROR: /home/davido/projects/rules_closoure/java/io/bazel/rules/closure/BUILD:41:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:6: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
      ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)

See also upstream issue with in depth explanation what is going on
there: [2]

4. Build rules_closure without the incompatible option: all is fine.
   That would mean, that all broken downstream projects would have
   enough time to adapt their build tool chains for upgraded
   dependencies in java_tools distribution.

[1] google/error-prone#1619
[2] bazelbuild/rules_closure#483

Change-Id: I352297a8d0d86cecf30821c132c7871383e49b50
davido added a commit to davido/bazel that referenced this issue May 21, 2020
Fixes bazelbuild#11446.

Java tools are decoupled from Bazel distribution and downloaded from
java_tools repository.

If some of the tools is updated, the archive name is changed in Bazel
and new Bazel release is conducted. If, however, new tool has some
breaking changes, then the upgrade needs to be rolled back.

To make user experience with upgrading tools less disruptive but still
not unnecessary procrastinate releases of important Java tools like
javac and Error Prone --incompatible_use_java_tools_beta_release is
added and CI-job should be set up to check all downstream projects
against upcoming java_tools release.

Test Plan:

1. Upgrade Error Prone version to 2.3.4 Bazel that is known to have
breaking changes and will break all Bazel users who rely on cross
compilation use case. There are ongoing efforts to demote the offensive
EP checks to warning severity instead of error, though: [1].

2. Bump java_tools in beta_release archive to new java_tools
   distribution that includes Error Prone 2.3.4

3. Build rules_closure with --incompatible_use_java_tools_beta_release
   would fail with the known issue, because of unsatisfied dependency
   on javax.annotation:

ERROR: /home/davido/projects/rules_closoure/java/io/bazel/rules/closure/BUILD:41:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:6: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
      ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)

See also upstream issue with in depth explanation what is going on
there: [2]

4. Build rules_closure without the incompatible option: all is fine.
   That would mean, that all broken downstream projects would have
   enough time to adapt their build tool chains for upgraded
   dependencies in java_tools distribution.

[1] google/error-prone#1619
[2] bazelbuild/rules_closure#483

Change-Id: I352297a8d0d86cecf30821c132c7871383e49b50
davido added a commit to davido/bazel that referenced this issue May 21, 2020
Fixes bazelbuild#11446.

Java tools are decoupled from Bazel distribution and downloaded from
java_tools repository.

If some of the tools is updated, the archive name is changed in Bazel
and new Bazel release is conducted. If, however, new tool has some
breaking changes, then the upgrade needs to be rolled back.

To make user experience with upgrading tools less disruptive but still
not unnecessary procrastinate releases of important Java tools like
javac and Error Prone --incompatible_use_java_tools_beta_release is
added and CI-job should be set up to check all downstream projects
against upcoming java_tools release.

Test Plan:

1. Upgrade Error Prone version to 2.3.4 Bazel that is known to have
breaking changes and will break all Bazel users who rely on cross
compilation use case. There are ongoing efforts to demote the offensive
EP checks to warning severity instead of error, though: [1].

2. Bump java_tools in beta_release archive to new java_tools
   distribution that includes Error Prone 2.3.4

3. Build rules_closure with --incompatible_use_java_tools_beta_release
   would fail with the known issue, because of unsatisfied dependency
   on javax.annotation:

ERROR: /home/davido/projects/rules_closoure/java/io/bazel/rules/closure/BUILD:41:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:6: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
      ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)

See also upstream issue with in depth explanation what is going on
there: [2]

4. Build rules_closure without the incompatible option: all is fine.
   That would mean, that all broken downstream projects would have
   enough time to adapt their build tool chains for upgraded
   dependencies in java_tools distribution.

[1] google/error-prone#1619
[2] bazelbuild/rules_closure#483

Change-Id: I352297a8d0d86cecf30821c132c7871383e49b50
@davido
Copy link
Contributor Author

davido commented Jul 2, 2020

As of: [1] Gerrit removed rules_closure dependency.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/272773

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 a pull request may close this issue.

2 participants