More nullability annotations #5251

mibac138 · 2017-03-31T16:33:37Z

Added nullability annotations to:

Disposables
Observers
and some others

vanniktech

You probably don't want to mess up the imports.

codecov · 2017-03-31T16:48:29Z

Codecov Report

Merging #5251 into 2.x will decrease coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##                2.x    #5251      +/-   ##
============================================
- Coverage     95.97%   95.96%   -0.01%     
- Complexity     5741     5746       +5     
============================================
  Files           628      628              
  Lines         41075    41077       +2     
  Branches       5698     5699       +1     
============================================
- Hits          39420    39418       -2     
- Misses          660      664       +4     
  Partials        995      995

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/io/reactivex/Single.java	`99.32% <ø> (ø)`	`131 <0> (ø)`	⬇️
.../main/java/io/reactivex/exceptions/Exceptions.java	`100% <ø> (ø)`	`6 <0> (ø)`	⬇️
src/main/java/io/reactivex/subjects/Subject.java	`100% <ø> (ø)`	`3 <0> (ø)`	⬇️
...main/java/io/reactivex/subjects/SingleSubject.java	`95.23% <ø> (ø)`	`38 <0> (ø)`	⬇️
...n/java/io/reactivex/parallel/ParallelFlowable.java	`100% <ø> (ø)`	`49 <0> (ø)`	⬇️
...activex/observers/ResourceCompletableObserver.java	`100% <ø> (ø)`	`8 <0> (ø)`	⬇️
...n/java/io/reactivex/observers/DefaultObserver.java	`90% <ø> (ø)`	`4 <0> (ø)`	⬇️
.../io/reactivex/observers/ResourceMaybeObserver.java	`100% <ø> (ø)`	`8 <0> (ø)`	⬇️
...o/reactivex/observers/DisposableMaybeObserver.java	`100% <ø> (ø)`	`7 <0> (ø)`	⬇️
...ain/java/io/reactivex/disposables/Disposables.java	`100% <ø> (ø)`	`8 <0> (ø)`	⬇️
... and 65 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c8b0ef...ec17491. Read the comment docs.

akarnokd

Some of the non-public components don't need annotation.

akarnokd · 2017-03-31T16:52:49Z

src/main/java/io/reactivex/Single.java

-import io.reactivex.internal.operators.flowable.*;
-import io.reactivex.internal.operators.maybe.*;
-import io.reactivex.internal.operators.observable.*;
+import io.reactivex.internal.functions.Functions;


Please don't unroll star imports.

akarnokd · 2017-03-31T16:53:29Z

src/main/java/io/reactivex/disposables/ActionDisposable.java

 import io.reactivex.functions.Action;
 import io.reactivex.internal.util.ExceptionHelper;

 final class ActionDisposable extends ReferenceDisposable<Action> {

    private static final long serialVersionUID = -8219729196779211169L;

-    ActionDisposable(Action value) {
+    ActionDisposable(@NonNull Action value) {


Non-public components don't really need such annotations.

akarnokd · 2017-03-31T16:53:53Z

src/main/java/io/reactivex/disposables/CompositeDisposable.java

-import java.util.*;
-
-import io.reactivex.exceptions.*;
+import io.reactivex.annotations.NonNull;


Please don't unroll star imports.

akarnokd · 2017-03-31T16:54:40Z

src/main/java/io/reactivex/disposables/Disposables.java

 import io.reactivex.functions.Action;
 import io.reactivex.internal.disposables.EmptyDisposable;
-import io.reactivex.internal.functions.*;
+import io.reactivex.internal.functions.Functions;


Please don't unroll star imports.

akarnokd · 2017-03-31T16:55:31Z

src/main/java/io/reactivex/disposables/FutureDisposable.java

@@ -24,7 +26,7 @@

    private final boolean allowInterrupt;

-    FutureDisposable(Future<?> run, boolean allowInterrupt) {
+    FutureDisposable(@Nullable Future<?> run, boolean allowInterrupt) {


Non-public components don't really need this annotation.

akarnokd · 2017-03-31T17:07:19Z

src/main/java/io/reactivex/parallel/ParallelFlowable.java

-import org.reactivestreams.*;
-
-import io.reactivex.*;
+import io.reactivex.Flowable;


Please don't unroll star imports.

akarnokd · 2017-03-31T17:09:07Z

src/main/java/io/reactivex/processors/AsyncProcessor.java

-
-import org.reactivestreams.*;
-
+import io.reactivex.annotations.*;


What IDE are you using that reorders imports?

IntelliJ IDEA

akarnokd · 2017-03-31T17:09:38Z

src/main/java/io/reactivex/processors/AsyncProcessor.java

@@ -36,7 +35,8 @@

    @SuppressWarnings("rawtypes")
    static final AsyncSubscription[] TERMINATED = new AsyncSubscription[0];
-
+
+    @NonNull


Non-public, not necessary.

akarnokd · 2017-03-31T17:10:09Z

src/main/java/io/reactivex/schedulers/Timed.java

 /**
 * Holds onto a value along with time information.
 *
 * @param <T> the value type
 */
 public final class Timed<T> {
+    @Nullable


Not necessary.

akarnokd · 2017-03-31T17:10:52Z

src/main/java/io/reactivex/schedulers/Timed.java

@@ -34,7 +37,7 @@
     * @param unit the time unit, not null
     * @throws NullPointerException if unit is null
     */
-    public Timed(T value, long time, TimeUnit unit) {
+    public Timed(@Nullable T value, long time, @NonNull TimeUnit unit) {


Why is this nullable?

akarnokd

A couple of changes are still needed.

akarnokd · 2017-04-01T17:45:01Z

src/main/java/io/reactivex/exceptions/CompositeException.java

@@ -47,7 +49,7 @@
     *
     * @throws IllegalArgumentException if <code>exceptions</code> is empty.
     */
-    public CompositeException(Throwable... exceptions) {
+    public CompositeException(@Nullable Throwable... exceptions) {


This should be @NonNull.

akarnokd · 2017-04-01T17:45:11Z

src/main/java/io/reactivex/exceptions/CompositeException.java

@@ -59,7 +61,7 @@ public CompositeException(Throwable... exceptions) {
     *
     * @throws IllegalArgumentException if <code>errors</code> is empty.
     */
-    public CompositeException(Iterable<? extends Throwable> errors) {
+    public CompositeException(@Nullable Iterable<? extends Throwable> errors) {


This should be @NonNull.

akarnokd · 2017-04-01T17:46:15Z

src/main/java/io/reactivex/exceptions/Exceptions.java

@@ -32,7 +33,8 @@ private Exceptions() {
     * @return because {@code propagate} itself throws an exception or error, this is a sort of phantom return
     *         value; {@code propagate} does not actually return anything
     */
-    public static RuntimeException propagate(Throwable t) {
+    @NonNull
+    public static RuntimeException propagate(@Nullable Throwable t) {


The argument should be @NonNull.

akarnokd · 2017-04-01T17:46:51Z

src/main/java/io/reactivex/exceptions/Exceptions.java

@@ -61,7 +63,7 @@ public static RuntimeException propagate(Throwable t) {
     *         the {@code Throwable} to test and perhaps throw
     * @see <a href="https://github.com/ReactiveX/RxJava/issues/748#issuecomment-32471495">RxJava: StackOverflowError is swallowed (Issue #748)</a>
     */
-    public static void throwIfFatal(Throwable t) {
+    public static void throwIfFatal(@Nullable Throwable t) {


The argument should be @NonNull.

akarnokd · 2017-04-01T17:48:16Z

src/main/java/io/reactivex/exceptions/OnErrorNotImplementedException.java

@@ -35,7 +35,7 @@
     * @param e
     *          the {@code Throwable} to signal; if null, a NullPointerException is constructed
     */
-    public OnErrorNotImplementedException(String message, Throwable e) {
+    public OnErrorNotImplementedException(String message, @Nullable Throwable e) {


The e argument should be @NonNull.

akarnokd · 2017-04-01T17:51:10Z

src/main/java/io/reactivex/observables/GroupedObservable.java

@@ -31,13 +32,14 @@
 */
 public abstract class GroupedObservable<K, T> extends Observable<T> {

+    @Nullable


No need to annotate this.

akarnokd · 2017-04-01T17:54:51Z

src/main/java/io/reactivex/observers/SerializedObserver.java

@@ -34,7 +35,7 @@
    final boolean delayError;

    static final int QUEUE_LINK_SIZE = 4;
-
+    


Whitespace added?

akarnokd · 2017-04-01T17:56:06Z

src/main/java/io/reactivex/processors/AsyncProcessor.java

@@ -36,7 +35,7 @@

    @SuppressWarnings("rawtypes")
    static final AsyncSubscription[] TERMINATED = new AsyncSubscription[0];
-
+    


Whitespace added?

akarnokd · 2017-04-01T17:56:45Z

src/main/java/io/reactivex/schedulers/Timed.java

@@ -44,6 +45,7 @@ public Timed(T value, long time, TimeUnit unit) {
     * Returns the contained value.
     * @return the contained value
     */
+    @Nullable


This should be @NonNull.

akarnokd · 2017-04-01T17:57:52Z

src/main/java/io/reactivex/subjects/SingleSubject.java

@@ -34,7 +34,7 @@
 */
 @Experimental
 public final class SingleSubject<T> extends Single<T> implements SingleObserver<T> {
-
+    


Whitespace?

PaulWoitaschek · 2017-04-21T10:08:29Z

src/main/java/io/reactivex/exceptions/OnErrorNotImplementedException.java

@@ -35,7 +35,7 @@
     * @param e
     *          the {@code Throwable} to signal; if null, a NullPointerException is constructed
     */
-    public OnErrorNotImplementedException(String message, Throwable e) {
+    public OnErrorNotImplementedException(String message, @NonNull Throwable e) {


Looks like this should be Nullable

PaulWoitaschek · 2017-04-21T10:08:39Z

src/main/java/io/reactivex/exceptions/OnErrorNotImplementedException.java

@@ -47,7 +47,7 @@ public OnErrorNotImplementedException(String message, Throwable e) {
     * @param e
     *          the {@code Throwable} to signal; if null, a NullPointerException is constructed
     */
-    public OnErrorNotImplementedException(Throwable e) {
+    public OnErrorNotImplementedException(@NonNull Throwable e) {


Nullable too.

PaulWoitaschek · 2017-04-21T10:10:33Z

src/main/java/io/reactivex/exceptions/CompositeException.java

@@ -47,7 +49,7 @@
     *
     * @throws IllegalArgumentException if <code>exceptions</code> is empty.
     */
-    public CompositeException(Throwable... exceptions) {
+    public CompositeException(@NonNull Throwable... exceptions) {


Nullable instead? The code handles it explicitly.

More nullability annotations

fdaff72

vanniktech suggested changes Mar 31, 2017

View reviewed changes

Refactored imports

b924be2

akarnokd requested changes Mar 31, 2017

View reviewed changes

akarnokd added 2.x Cleanup labels Apr 1, 2017

mibac138 added 2 commits April 1, 2017 18:51

Changes based on akarnokd's review

b376bfd

A few more annotations

cfee848

akarnokd reviewed Apr 1, 2017

View reviewed changes

Changes based on akarnokd's 2nd review

ec17491

akarnokd approved these changes Apr 1, 2017

View reviewed changes

akarnokd merged commit fa58d36 into ReactiveX:2.x Apr 1, 2017

akarnokd mentioned this pull request Apr 17, 2017

2.0.9 release preparations #5290

Closed

PaulWoitaschek reviewed Apr 21, 2017

View reviewed changes

michaelEllisUy mentioned this pull request Jun 1, 2017

Dependency update: rxjava xmartlabs/bigbang#82

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More nullability annotations #5251

More nullability annotations #5251

mibac138 commented Mar 31, 2017

vanniktech left a comment

codecov bot commented Mar 31, 2017 •

edited

Loading

akarnokd left a comment

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

mibac138 Apr 1, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd Mar 31, 2017

akarnokd left a comment

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

akarnokd Apr 1, 2017

PaulWoitaschek Apr 21, 2017

PaulWoitaschek Apr 21, 2017

PaulWoitaschek Apr 21, 2017


		import org.reactivestreams.*;

		import io.reactivex.annotations.*;

		@@ -34,7 +35,7 @@
		final boolean delayError;

		static final int QUEUE_LINK_SIZE = 4;

		@@ -36,7 +35,7 @@

		@SuppressWarnings("rawtypes")
		static final AsyncSubscription[] TERMINATED = new AsyncSubscription[0];

More nullability annotations #5251

More nullability annotations #5251

Conversation

mibac138 commented Mar 31, 2017

vanniktech left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 31, 2017 • edited Loading

Codecov Report

akarnokd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akarnokd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 31, 2017 •

edited

Loading