[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16418942#comment-16418942 ] ASF GitHub Bot commented on THRIFT-4530: Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1522 > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16418924#comment-16418924 ] ASF GitHub Bot commented on THRIFT-4530: Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1522 That is an intermittent test failure that I thought I resolved. :) it is C++ only > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16418176#comment-16418176 ] ASF GitHub Bot commented on THRIFT-4530: Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1522 I don't think it's relevant either, seems like a flaky test. I think we are good to merge. @jeking3 any idea on the failing appveyor test, have you seen it before? I'm inclined to ignore it unless you say otherwise. > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16418132#comment-16418132 ] ASF GitHub Bot commented on THRIFT-4530: Github user msridhar commented on the issue: https://github.com/apache/thrift/pull/1522 Hrm, now I see a concurrency test failure on appveyor. Doesn't seem related to my change, though I'm not 100% sure. > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16417654#comment-16417654 ] ASF GitHub Bot commented on THRIFT-4530: Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1522 No need, I'll squash once Travis is all green. > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16417645#comment-16417645 ] ASF GitHub Bot commented on THRIFT-4530: Github user msridhar commented on the issue: https://github.com/apache/thrift/pull/1522 I just pushed up another commit with the renaming. I assume you can do a squash and merge, but if you'd like me to squash just let me know. > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16417221#comment-16417221 ] ASF GitHub Bot commented on THRIFT-4530: Github user jeking3 commented on a diff in the pull request: https://github.com/apache/thrift/pull/1522#discussion_r177720513 --- Diff: lib/java/src/org/apache/thrift/annotations/Nullable.java --- @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.thrift.annotations; --- End diff -- Any reason why java language uses "annotation" but you selected "annotations" for thrift? Seems like we should be using "annotation". > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16416594#comment-16416594 ] ASF GitHub Bot commented on THRIFT-4530: Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1522 Yes there is an issue that appears in builds where lisp cannot find a file it just supposedly wrote. Lisp is new to the project, this is an ongoing thing. > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16416581#comment-16416581 ] ASF GitHub Bot commented on THRIFT-4530: Github user msridhar commented on the issue: https://github.com/apache/thrift/pull/1522 Looks like the Travis failure is on Common Lisp codegen (https://api.travis-ci.org/v3/job/359136811/log.txt), which has nothing to do with this PR > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16416443#comment-16416443 ] ASF GitHub Bot commented on THRIFT-4530: Github user msridhar commented on the issue: https://github.com/apache/thrift/pull/1522 Note that while there are no new tests, the code under `tutorial/java/gen-java` includes the annotations and compiles without issue. Since this change doesn't affect runtime behavior of the generated code I think this might be sufficient. > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16416437#comment-16416437 ] ASF GitHub Bot commented on THRIFT-4530: GitHub user msridhar opened a pull request: https://github.com/apache/thrift/pull/1522 THRIFT-4530: add @Nullable annotations to generated Java code Use our own `org.apache.thrift.annotations.Nullable` annotation to avoid introducing a third-party dependency You can merge this pull request into a Git repository by running: $ git pull https://github.com/msridhar/thrift THRIFT-4530 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1522.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1522 commit 6bc560a1a55a1fca720b4fe7057f25c06fc358d1 Author: Manu SridharanDate: 2018-03-20T21:45:22Z THRIFT-4530: add @Nullable annotations to generated Java code Use our own `org.apache.thrift.annotations.Nullable` type to avoid introducing a third-party dependency > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16413981#comment-16413981 ] James E. King, III commented on THRIFT-4530: If nullable annotations have no other side-effects why don't we just add the annotation and then at such time someone finds a need to disable it they can add the code necessary to do no? > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16412550#comment-16412550 ] Can Celasun commented on THRIFT-4530: - I think it's better if they are on by default, but if anyone has objections I'm happy with off as well. cc [~jking3] > 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16412392#comment-16412392 ] Manu Sridharan commented on THRIFT-4530: Great! I will definitely post the PR on GitHub. Do we want {{@Nullable}} annotations off by default, with a compiler flag to enable them? Or should they just be on by default? In any case, hope to have a PR up soon. 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)
[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code
[ https://issues.apache.org/jira/browse/THRIFT-4530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16412173#comment-16412173 ] Can Celasun commented on THRIFT-4530: - I think it's a good idea. Regarding which annotation to use, I think we should definitely include a default {{@Nullable}} implementation, but it might also be worth adding a compiler flag to use a different one. For the patch, it's better if you send it as a PR on Github instead of posting it here, as a PR will trigger a Travis build. > 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)