Skip to content

Generated Java code causes "redundant cast to Builder" warnings #5139

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

Closed
slovdahl opened this issue Sep 12, 2018 · 20 comments · Fixed by #5247
Closed

Generated Java code causes "redundant cast to Builder" warnings #5139

slovdahl opened this issue Sep 12, 2018 · 20 comments · Fixed by #5247
Assignees

Comments

@slovdahl
Copy link
Contributor

slovdahl commented Sep 12, 2018

What version of protobuf and what language are you using?
Version: v3.6.1
Gradle plugin version: v0.8.6
Language: Java

What operating system (Linux, Windows, ...) and version?
Ubuntu 16.04

What runtime / compiler are you using (e.g., python version or gcc version)

$ java -version
openjdk version "1.8.0_181"
OpenJDK Runtime Environment (build 1.8.0_181-8u181-b13-0ubuntu0.16.04.1-b13)
OpenJDK 64-Bit Server VM (build 25.181-b13, mixed mode)

What did you do?

I don't know if this is a regression or always has been like this, this is the first time I'm using the protobuf java stuff.

I prepared a project demonstrating the issue https://github.com/slovdahl/protobuf-java-warnings, but in short:

  1. Created a gradle project:
buildscript {
    repositories {
        mavenCentral()
    }
    dependencies {
        classpath 'com.google.protobuf:protobuf-gradle-plugin:0.8.6'
    }
}

repositories {
    mavenCentral()
}

apply plugin: 'java'
apply plugin: 'com.google.protobuf'

protobuf {
    protoc {
        artifact = 'com.google.protobuf:protoc:3.6.1'
    }
}

dependencies {
    compile 'com.google.protobuf:protobuf-java:3.6.1'
}

compileJava {
    options.compilerArgs += ['-Xlint']
}
  1. Created a proto definition:
syntax = "proto3";

package com.foo.bar.proto;

option java_outer_classname = "Event";

message Foo {
    int32 id = 1;
}
  1. Executed ./gradlew compileJava, which generates the java code and compiles the generated code.

What did you expect to see
No java compiler warnings.

What did you see instead?

/home/dev/protobuf-test/build/generated/source/proto/main/java/com/foo/bar/proto/Event.java:341: warning: [cast] redundant cast to Builder
        return (Builder) super.clone();
               ^
/home/dev/protobuf-test/build/generated/source/proto/main/java/com/foo/bar/proto/Event.java:347: warning: [cast] redundant cast to Builder
        return (Builder) super.setField(field, value);
               ^
/home/dev/protobuf-test/build/generated/source/proto/main/java/com/foo/bar/proto/Event.java:352: warning: [cast] redundant cast to Builder
        return (Builder) super.clearField(field);
               ^
/home/dev/protobuf-test/build/generated/source/proto/main/java/com/foo/bar/proto/Event.java:357: warning: [cast] redundant cast to Builder
        return (Builder) super.clearOneof(oneof);
               ^
/home/dev/protobuf-test/build/generated/source/proto/main/java/com/foo/bar/proto/Event.java:363: warning: [cast] redundant cast to Builder
        return (Builder) super.setRepeatedField(field, index, value);
               ^
/home/dev/protobuf-test/build/generated/source/proto/main/java/com/foo/bar/proto/Event.java:369: warning: [cast] redundant cast to Builder
        return (Builder) super.addRepeatedField(field, value);
               ^
6 warnings
@slovdahl
Copy link
Contributor Author

Would you be willing to accept a pull request that fixes this?

@slovdahl
Copy link
Contributor Author

Thanks for accepting the fix!

@BSBandme What's your policy regarding new releases, how often are releases made? Not asking that you make a release immediately, just want to know how to set my expectations 🙂

@terje2001
Copy link

Thanks for fixing this!

@BSBandme : I second @slovdahl request, the javac output is disruptive and noisy in our process. If a new release is not imminent, would it be possible to backport to the 3.6.x branch and get a release of that?

@perlun
Copy link
Contributor

perlun commented Nov 28, 2018

@acozzette You seem to have tagged a 3.6.1.1 release 7 days ago, but this version is not yet available in Maven Central from what I can tell: https://search.maven.org/search?q=g:com.google.protobuf

image

Also, this commit (dc81f79) doesn't seem to be present on the 3.6.x branch for now: v3.6.1...3.6.x, so I'm pretty certain it isn't included in the version that got tagged as 3.6.1.1.

If you could cherry-pick this change (or, make someone else do it) and put out a 3.6.1.2 or 3.6.2 version, me (and others) would be incredibly thankful. Thanks in advance! 👍

@acozzette
Copy link
Member

@perlun That 3.6.1.1 release was just meant to include an important fix that was blocking the Bazel 0.20 release, so I tagged a commit without doing a full release and publishing artifacts and everything. We are going to do a real 3.7 release soon, so that will include all the changes that are currently on master.

@perlun
Copy link
Contributor

perlun commented Dec 4, 2018

@acozzette Wonderful, thanks for the update. 👍

@luster
Copy link

luster commented Jan 4, 2019

any update on the timeline for new release?

@perlun
Copy link
Contributor

perlun commented Jan 8, 2019

Ping @acozzette or others - any updates? (I was reminded about this now since it floods our build output)

@klaraward
Copy link

Another ping, any ETA for the next release?

@acozzette
Copy link
Member

We are working on it now. I think it's probably ready to go but we're going to do another rc release just to be safe before we release 3.7.0.

@voidzcy
Copy link

voidzcy commented Mar 16, 2020

Seems this issue is still not fixed in javalite generated code. I saw those warnings when using protobuf-javalite:3.11.0.

@bmarwell
Copy link

bmarwell commented Feb 2, 2021

please reopen as requested in #8208 by @perezd in #8208 (comment).

@perezd
Copy link
Contributor

perezd commented Feb 2, 2021

Oops, didn't realize it was closed.

@perezd perezd reopened this Feb 2, 2021
@bmarwell
Copy link

Hi, any progress on this?

@perezd
Copy link
Contributor

perezd commented Aug 10, 2021

Has this been reproduced w/ our latest java release? (3.17)

@bmarwell
Copy link

Huh, maybe you could add a test. PR #5247 sadly did not add any.

@danadam
Copy link

danadam commented Feb 11, 2022

The script in attachment, with protoc, protobuf-java-3.19.4.jar and protobuf-javalite-3.19.4.jar in current directory, gives output:

javac 11.0.13
libprotoc 3.19.4
test proto2 full
test proto3 full
test proto2 lite
java-test/TestProto2.java:147: warning: [cast] redundant cast to Builder
      return (Builder) DEFAULT_INSTANCE.createBuilder(prototype);
             ^
1 warning
test proto3 lite
java-test/TestProto3.java:133: warning: [cast] redundant cast to Builder
      return (Builder) DEFAULT_INSTANCE.createBuilder(prototype);
             ^
1 warning

redundant.cast.sh.txt

@perezd
Copy link
Contributor

perezd commented Feb 11, 2022

We'll accept PRs to resolve this similar to the others if you'd like.

@bmarwell
Copy link

We'll accept PRs to resolve this similar to the others if you'd like.

You already did this for non-lite versions:
dc81f79

@perezd
Copy link
Contributor

perezd commented Feb 14, 2022

Great! That'll make it easier for someone to send us a PR, appreciate it!

@googleberg googleberg assigned googleberg and unassigned BSBandme Sep 1, 2022
copybara-service bot pushed a commit that referenced this issue May 17, 2023
Fixes #5139

Previously the fix #5247 has removed unnecessary cast to Builder in generated Java code, and the current PR is doing a similar one for Java lite code.

After this PR, both Java and Java lite code won't have an unnecessary cast to Builder, thus #5139 will be fixed.

This change simply removed those casts to Builder for 2 methods, and the reason it will work is:

DEFAULT_INSTANCE.createBuilder method is defined in the superclass, and the return value is a generic value which is exactly the Builder class passed in.

Closes #10781

COPYBARA_INTEGRATE_REVIEW=#10781 from 1e0ng:main e8f9c98
FUTURE_COPYBARA_INTEGRATE_REVIEW=#10781 from 1e0ng:main e8f9c98
PiperOrigin-RevId: 532797271
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment