Protobuf: [proto3] Deprecated method `valueOf()` called in generated code

[biran0079]

Sample proto:

option java_multiple_files = true;
option java_package = "io.grpc.examples.helloworld";

enum TestEnum {
  VALUE = 0;
}

message TestMessage {
  TestEnum test_enum = 1;
}

Following code is generated:

  /**
   * <code>optional .TestEnum test_enum = 1;</code>
   */
  public io.grpc.examples.helloworld.TestEnum getTestEnum() {
    io.grpc.examples.helloworld.TestEnum result = io.grpc.examples.helloworld.TestEnum.valueOf(testEnum_);
    return result == null ? io.grpc.examples.helloworld.TestEnum.UNRECOGNIZED : result;
  }

which gives following warnings:

/io/grpc/examples/helloworld/TestMessage.java:87: mandatory_warning: valueOf(int) in io.grpc.examples.helloworld.TestEnum has been deprecated
    io.grpc.examples.helloworld.TestEnum result = io.grpc.examples.helloworld.TestEnum.valueOf(testEnum_);
                                                                                      ^
/io/grpc/examples/helloworld/TestMessage.java:378: mandatory_warning: valueOf(int) in io.grpc.examples.helloworld.TestEnum has been deprecated
      io.grpc.examples.helloworld.TestEnum result = io.grpc.examples.helloworld.TestEnum.valueOf(testEnum_);
                                                                                        ^

[biran0079]

Would appreciate if this issue is fixed soon.
Our application is built with -Werror flag and this is blocking us from upgrading to Grpc 1.0.

[jorgjanke]

+10 - proto2 also generates valueOf() calls - creating warnings - for us also prevent build

[acozzette]

It looks like we need to have the generated code continue to call valueOf(), because we need to remain compatible with older generated code that does not yet have the recommended forNumber() method. So it seems to me like the right solution is to update the compiler to stop putting a deprecation annotation on valueOf. That should happen in these two places:

• https://github.com/google/protobuf/blob/master/src/google/protobuf/compiler/java/java_enum.cc#L186
• https://github.com/google/protobuf/blob/master/src/google/protobuf/compiler/java/java_enum_lite.cc#L150

Would anyone be interested in sending out a pull request to make this change?

[jorgjanke]

... I used c last time a decade ago .. so my temp solution was

sed -i '' 's/@java.lang.Deprecated//g' *.java
sed -i '' 's/ @deprecated/ deprecated/g' *.java

[jorgjanke]

Actually, I think you should not remove the deprecate but change the call to forNumber() as if you update the server library, you should also update the client library. And that is just using the new version in the build - not code changes.

[acozzette]

Unfortunately I don't think it's practical to update all generated code at the same time in many cases. This works well if you control your entire codebase and its dependencies, but it becomes a problem if you're using something like Maven and depending on multiple open source projects that all use protobuf. Projects sometimes publish their protobuf generated code through Maven because that's often easier than having to run protoc during your build. So let's say you depend on three open source projects that all include generated code. If we introduced a breaking change then you would have to simultaneously update all three projects and of course also simultaneously update any generated code of your own. We actually used to require the generated code to match the library version but we're now intentionally supporting older generated code so that we don't run into these problems.

[blendmaster]

If anybody else comes across this using the protobuf-gradle plugin, here's the translation of those sed incantations:

    protobuf {
        ...
        // https://github.com/google/protobuf/issues/2054
        generateProtoTasks {
            all().each { task ->
                task.doLast {
                    ant.replaceregexp(
                        match:'@java.lang.Deprecated|@deprecated',
                        replace:'',
                        flags:'g',
                        byline:true
                    ) {
                        fileset(
                            dir: "${protobuf.generatedFilesBaseDir}/main/java")
                    }
                }
            }
        }
    }

[vgarmash]

@blendmaster Your script would work but the regex too wide. It will match any deprecated notation, not only those on valueOf methods... Can't you make the multi-line regex that would find only @java.lang.Deprecated above valueOf method?

[blendmaster]

looks like if you set byline: false then yeah, you could try to scope this to valueOf methods.

https://ant.apache.org/manual/Tasks/replaceregexp.html

I don't have any other deprecated methods to test against though. If you can figure out the right regex @vgarmash, I'll update my comment.

[mahileeb]

As well as valueOf, can you also fix usages of PARSER instead of parser() as mentioned on #1596

[jgallag88]

Rather than replacing valueOf() with forNumber(), or 'un-deprecating' valueOf(), there might be a third option: add @SuppressWarnings("deprecation") to statements that use valueOf().

I've been doing the this by using a Gradle task along the lines of what @blendmaster has, except with the following for the replaceregexp:

ant.replaceregexp(
    match: '(.*\\.valueOf.*)',
    replace: '    @SuppressWarnings("deprecation")\n\\1',
    flags: 'g',
    byline: true
)

which turns code like

  public io.grpc.examples.helloworld.TestEnum getTestEnum() {
    io.grpc.examples.helloworld.TestEnum result = io.grpc.examples.helloworld.TestEnum.valueOf(testEnum_);
    return result == null ? io.grpc.examples.helloworld.TestEnum.UNRECOGNIZED : result;
  }

into code like

  public io.grpc.examples.helloworld.TestEnum getTestEnum() {
    @SuppressWarnings("deprecation")
    io.grpc.examples.helloworld.TestEnum result = io.grpc.examples.helloworld.TestEnum.valueOf(testEnum_);
    return result == null ? io.grpc.examples.helloworld.TestEnum.UNRECOGNIZED : result;
  }

[vgarmash]

So you prefer to use deprecated code which could be eventually removed from JDK and just hide the warning?

[jorgjanke]

What's the concern moving to forNumber() ??
For outdated JDK support, one could check the JDK version when generating the code and then generate either valueOf() or forNumber().

[BSBandme]

Not sure about this.

[acozzette]

This should be fixed now with pull request #4046.