pbandk icon indicating copy to clipboard operation
pbandk copied to clipboard

Redeclaration error with identically-named messages in different proto packages that share the same java package

Open pddkhoa-fossil opened this issue 3 years ago • 3 comments

I try define few proto files have same name Message with different package name. But when I generate kotlin file, They have same package name(com.example.protobuf) make error Redeclaration: Test

Example:

a.proto:

syntax = "proto3";
option java_package = "com.example.protobuf";
package A;

message Test {
  uint32 percentage = 1;
}

b.proto:

syntax = "proto3";
option java_package = "com.example.protobuf";
package B;

message Test {
  uint32 percentage = 1;
}

protoc config:

protobuf {
    generatedFilesBaseDir = "$projectDir/src"
    protoc {
        artifact = "com.google.protobuf:protoc:$protobufVersion"
    }
    plugins {
        id("protobuftest") {
            artifact = "pro.streem.pbandk:protoc-gen-pbandk-jvm:$pbandkVersion:jvm8@jar"
        }
    }
    generateProtoTasks {
        ofSourceSet("main").forEach { task ->
            task.builtins {
                remove("java")
            }
            task.plugins {
                id("protobuftest") {
                    option("log=debug")
                }
            }
        }
    }
}

I run: ./gradlew :myapp:installDist --stacktrace

pddkhoa-fossil avatar Nov 10 '21 05:11 pddkhoa-fossil

That's expected protobuf behavior since you're defining the same java_package in both files. I think you would get a similar error even with the official protobuf-java library.

Typical protobuf style is to have a 1:1 relationship between the value of package and java_package in every .proto file.

garyp avatar Nov 11 '21 01:11 garyp

@garyp with official protobuf-java library support using the wrapper class as an outer class when I use same java_package in both files. There are generated two files as below:

final class A {
  static final class Test {
      ...
  }
}
final class  B {
  static final class Test {
     ...
  }
}

Do you think we should support this feature in the library?

pddkhoa-fossil avatar Nov 11 '21 02:11 pddkhoa-fossil

Oh interesting. I think I've always used the java_package option together with option java_multiple_files = true, which doesn't nest definitions within an outer class. But I think you're right that the default behavior (with java_multiple_files = false) would nest the definitions inside separate outer classes and avoid the conflict you're seeing in pbandk.

This feels like an edge case that I'm not sure is worth supporting in pbandk. It's definitely not a setup that I would encourage. That said, I also don't like that pbandk generates errors for a .proto file which is handled successfully by protobuf-java since that would make it harder for users with existing protobuf files to easily migrate to pbandk.

I'll leave this issue open and try to think of a way to handle this gracefully in pbandk. I can't promise when we'd have time to implement this, but a PR to handle this situation is welcome.

garyp avatar Nov 11 '21 02:11 garyp