You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/21 00:10:24 UTC
[GitHub] [beam] kennknowles opened a new pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
kennknowles opened a new pull request #12323:
URL: https://github.com/apache/beam/pull/12323
This PR includes independent commits that:
- excludes generated `AutoValue` classes
- port the codebase to checker annotations so that types can be annotated properly and move annotations adjacent to the types that they are annotating (before Java 8 this was not possible but now it results in a warning if we do not do it)
- annotate overrides of `equals` to note that the argument is nullable
I did not try to fix nullability errors in `equals` methods because the PR is big enough as it is, and stands alone.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles merged pull request #12323: [BEAM-10540] Migrate to checker annotations and properly annotate Object#equals overrides
Posted by GitBox <gi...@apache.org>.
kennknowles merged pull request #12323:
URL: https://github.com/apache/beam/pull/12323
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-662244372
run java postcommit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-662029763
Automated mass refactor did not catch use of nullable for arrays, which is somewhat wacky looking. `@Nullable Foo []` is an array of nullable `Foo` while `Foo @Nullable []` is an array of `Foo` where the whole array can be null.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-661483476
I should add that this PR is a noop for executable code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-662113732
And it seems IntelliJ's "optimize imports" got a bit too aggressive.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-662116597
Oh wow. It seems that switching to proper Nullable type annotations on the types has re-enabled spotbugs catching null issues. We don't actually want that, for modules that have gotten really bad in the meantime...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12323:
URL: https://github.com/apache/beam/pull/12323#discussion_r457763050
##########
File path: learning/katas/java/Common Transforms/Aggregation/Count/src/org/apache/beam/learning/katas/commontransforms/aggregation/count/Task.java
##########
@@ -16,7 +16,7 @@
* limitations under the License.
*/
-package org.apache.beam.learning.katas.commontransforms.aggregation.count;
+package org.apache.beam.learning.katas.commontransforms.aggregation.count; import org.checkerframework.checker.nullness.qual.Nullable;
Review comment:
`learning/` is not built by Beam module plugins, so they were not applied.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12323:
URL: https://github.com/apache/beam/pull/12323#discussion_r457761014
##########
File path: learning/katas/java/Common Transforms/Aggregation/Count/src/org/apache/beam/learning/katas/commontransforms/aggregation/count/Task.java
##########
@@ -16,7 +16,7 @@
* limitations under the License.
*/
-package org.apache.beam.learning.katas.commontransforms.aggregation.count;
+package org.apache.beam.learning.katas.commontransforms.aggregation.count; import org.checkerframework.checker.nullness.qual.Nullable;
Review comment:
This line is caused by my choice of the easiest search/replace solution. I expected spotless to fix it. It did not. I will file a bug against spotless. I will then fix it by adding a newline and re-running spotless.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-661472526
@robinyqiu just let me know if these reviews bore you :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-662073751
Also uncovered a limitation and what could be called a bug that the limitation was masking
- https://issues.apache.org/jira/browse/BEAM-10547 schema inference doesn't notice annotations on return types
- https://issues.apache.org/jira/browse/BEAM-10548 schema inference only works for `javax.annotation.Nullable` when it is actually one of the more limited annotations available
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12323: [BEAM-10540] Note that argument to Object#equals is always nullable; port all instances to checker annotations
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12323:
URL: https://github.com/apache/beam/pull/12323#issuecomment-662237399
run java postcommit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org