marcoferrer / kroto-plus

gRPC Kotlin Coroutines, Protobuf DSL, Scripting for Protoc
Apache License 2.0
494 stars 28 forks source link

Empty object generation when using maps and multiple files. #51

Closed sauldhernandez closed 5 years ago

sauldhernandez commented 5 years ago

Hi! I started to use this library not too long ago, great work!

So, I ran into a strange duplicate class error when using the protobuf builders generator, and having option java_multiple_files = true, compilation would fail whenever I have a message with a mapped field:

Task :myapp:kaptKotlin FAILED
e: <path to app>/build/generated/source/proto/main/java/com/myapp/Mapped.java:9: error: duplicate class: com.myapp.Mapped
public  final class Mapped extends

I tracked down the error to a generated object Mapped with no elements. Looking into the code I realized that it was generated there, but the filter didn't take into account map definitions (as is in other places in the generator)

I tested it and seems to solve the issue, test pass, but I don't know if there's anything else I should change.

codecov-io commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@f8032c4). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #51   +/-   ##
=========================================
  Coverage          ?   88.12%           
  Complexity        ?       19           
=========================================
  Files             ?       15           
  Lines             ?      278           
  Branches          ?       37           
=========================================
  Hits              ?      245           
  Misses            ?       14           
  Partials          ?       19

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f8032c4...44bee9e. Read the comment docs.

marcoferrer commented 5 years ago

Thanks for the contribution, this was a great catch. Would you mind adding a test sample for the previously failing proto to the test api module? And a very basic unit test in generator tests module. Something as simple as calling the fixed builder. This way the compiler can catch any regressions