protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Annotate nullability in generated code

Open NN--- opened this issue 6 years ago • 11 comments

If it's about generated code change, what programming language? C#, Java

Describe the problem you are trying to solve. Generated code could be annotated with NotNull , CanBeNull attributes which improves developing experience and save from null value exceptions.

Describe the solution you'd like Generated code with annotations. They can be either added as external library or just included in the source See: https://blog.jetbrains.com/dotnet/2015/08/12/how-to-use-jetbrains-annotations-to-improve-resharper-inspections/ https://www.jetbrains.com/help/idea/annotating-source-code.html

Describe alternatives you've considered Alternative is to parse generated code and add annotations by myself. I don't really like this solution.

Additional context Add any other context or screenshots about the feature request here.

NN--- avatar Jan 02 '19 06:01 NN---

For java, internally in google protobuf isn't very easy to add new dependency like this (in google we can add guava to mark Nullable annotation if new dependency is allowed). So this improvement may not be seen in the near future.

BSBandme avatar Jan 03 '19 18:01 BSBandme

I also agree that this would be useful. We are adding external annotations using annotations.xml at the moment but it feels like quite a kludge.

Supporting either com.google.code.findbugs:jsr305 (i.e. javax.annotations) or Guava annotations would be useful. Perhaps it could be an optional flag? I.e. you could run the protobuf compiler in "traditional" mode, imposing no extra dependency on the generated code whatsoever. This could be the perfect balance between "be conservative with adding new dependencies" and "add functionality generally useful for people".

If we'd go this route, I would even say that being able to configure what non-null annotation class to use would be perfect and a really great approach. (We are using javax.annotation.Nonnull ourselves, i.e. JSR305, which plays well with both IntellIJ IDEA and Spotbugs.)

(Note: I have no idea how hard this would be to implement in the Protobuf compiler.)

perlun avatar Mar 29 '19 15:03 perlun

I was going to post one issue about the same until I found this one. It would be super useful annotating arguments of setters with @NotNull, It throws NPE when null is passed anyway and make working with Protobuf in Kotlin way easier, since Kotlin offers a pretty good null safety when using pure Kotlin or just the Java standard library (and if you're not programming in a defensive way while using the generated code that could be a potential bug since Kotlin does not warn you about the exception).

kolmant avatar May 31 '21 19:05 kolmant

@cpovirk re. nullness annotations

kluever avatar Jun 08 '21 17:06 kluever

protoc generated java code annotates non-null APIs with @com.google.protobuf.Internal.ProtoNonnullApi

googleberg avatar Sep 01 '22 18:09 googleberg

Only inside Google, though :)

cpovirk avatar Sep 01 '22 21:09 cpovirk

My mistake. I thought these annotations were also in OSS. We should use the standard annotations rather than our custom ones.

googleberg avatar Oct 13 '22 16:10 googleberg

@googleberg While you're at it, it would be great to get this looked into at some point: https://github.com/protocolbuffers/protobuf/issues/4351#issuecomment-1117284074. It's an incredibly annoying detail of the generated (Java) code where certain methods can in fact return null... :grimacing:

perlun avatar Oct 14 '22 09:10 perlun

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar May 05 '24 10:05 github-actions[bot]

Still very valid issue.

slovdahl avatar May 06 '24 05:05 slovdahl

With edition 2023 and explicit presence, can be (e.g.) C# generator tweaked to use nullable types as a sugar for Has and Clear methods?

git-torrent avatar May 14 '24 15:05 git-torrent