thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Manu Sridharan (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (THRIFT-4530) proposal: add nullability annotations to generated Java code
Date Fri, 23 Mar 2018 18:26:00 GMT

     [ 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 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 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 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)

Mime
View raw message