[ 
https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Manu Sridharan updated THRIFT-4530:
-----------------------------------
    Description: 
I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway 
([https://github.com/uber/NullAway)] and we'd like to better support users who 
are using Thrift.  The change would involve changing the Java code generator to 
include {{@Nullable}} annotations on every field, method return value, and 
method parameter in the public API of generated code that may be null.  With 
these annotations, NullAway users will get warnings when their client code is 
missing appropriate null checks.  Also, IDEs like IntelliJ will give better 
warnings about missing null checks.  As part of this change, I would also add 
support to NullAway for understanding {{isSetX()}} methods to avoid excessive 
false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!

  was:
I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway 
([https://github.com/uber/NullAway)] and we'd like to better support users who 
are using Thrift.  The change would involve changing the Java code generator to 
include {{@Nullable}} annotations on every field, method return value, and 
method parameter in the public API that may be null.  With these annotations, 
NullAway users will get warnings when their client code is missing appropriate 
null checks.  Also, IDEs like IntelliJ will give better warnings about missing 
null checks.  As part of this change, I would also add support to NullAway for 
understanding {{isSetX()}} methods to avoid excessive false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!


> proposal: add nullability annotations to generated Java code
> ------------------------------------------------------------
>
>                 Key: THRIFT-4530
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4530
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Java - Compiler
>            Reporter: Manu Sridharan
>            Priority: Major
>
> I'd like to propose (optionally) including {{@Nullable}} annotations in 
> Thrift-generated Java code.  I'm the main author of NullAway 
> ([https://github.com/uber/NullAway)] and we'd like to better support users 
> who are using Thrift.  The change would involve changing the Java code 
> generator to include {{@Nullable}} annotations on every field, method return 
> value, and method parameter in the public API of generated code that may be 
> null.  With these annotations, NullAway users will get warnings when their 
> client code is missing appropriate null checks.  Also, IDEs like IntelliJ 
> will give better warnings about missing null checks.  As part of this change, 
> I would also add support to NullAway for understanding {{isSetX()}} methods 
> to avoid excessive false positives.
> Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
> minimize third-party dependencies, but we could simply include a new Thrift 
> {{@Nullable}} annotation, and it will work fine with NullAway and most other 
> tools.
> I have a WIP patch to generate these annotations, but I wanted to get 
> feedback from the maintainers before opening a PR.  We could of course make 
> the annotation generation optional and default it to being off, if desired.  
> Anyway, thoughts / feedback welcome.  Thanks!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to