From 4cf775df859ab7a8d10f60d755df243e2cf6703d Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 19 Nov 2018 18:28:06 -0800 Subject: [PATCH 1/3] Add OptionalCheckReturnValue --- .../annotations/OptionalCheckReturnValue.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 src/main/java/io/reactivex/annotations/OptionalCheckReturnValue.java diff --git a/src/main/java/io/reactivex/annotations/OptionalCheckReturnValue.java b/src/main/java/io/reactivex/annotations/OptionalCheckReturnValue.java new file mode 100644 index 0000000000..69c3647eb2 --- /dev/null +++ b/src/main/java/io/reactivex/annotations/OptionalCheckReturnValue.java @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2016-present, RxJava Contributors. + * + * Licensed 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 io.reactivex.annotations; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks methods whose return values should be optionally checked by separate tooling. + *

History: 2.2.4 - experimental + * @since 2.2.4 + */ +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Target(ElementType.METHOD) +public @interface OptionalCheckReturnValue { + +} From 64fcd67442e3395f09c1aedfa6afb89da6da37b9 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 19 Nov 2018 18:28:22 -0800 Subject: [PATCH 2/3] Use OptionalCheckReturnValue where appropriate --- src/main/java/io/reactivex/Completable.java | 4 ++-- src/main/java/io/reactivex/Flowable.java | 8 ++++---- src/main/java/io/reactivex/Maybe.java | 6 +++--- src/main/java/io/reactivex/Observable.java | 8 ++++---- src/main/java/io/reactivex/Single.java | 6 +++--- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/main/java/io/reactivex/Completable.java b/src/main/java/io/reactivex/Completable.java index 948d8aecde..9771f6fbfd 100644 --- a/src/main/java/io/reactivex/Completable.java +++ b/src/main/java/io/reactivex/Completable.java @@ -2318,7 +2318,7 @@ public final E subscribeWith(E observer) { * @return the Disposable that can be used for disposing the subscription asynchronously * @throws NullPointerException if either callback is null */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(final Action onComplete, final Consumer onError) { ObjectHelper.requireNonNull(onError, "onError is null"); @@ -2345,7 +2345,7 @@ public final Disposable subscribe(final Action onComplete, final ConsumerReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @BackpressureSupport(BackpressureKind.UNBOUNDED_IN) @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext) { @@ -14486,7 +14486,7 @@ public final Disposable subscribe(Consumer onNext) { * if {@code onNext} is null, or * if {@code onError} is null */ - @CheckReturnValue + @OptionalCheckReturnValue @BackpressureSupport(BackpressureKind.UNBOUNDED_IN) @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext, Consumer onError) { @@ -14520,7 +14520,7 @@ public final Disposable subscribe(Consumer onNext, ConsumerReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @BackpressureSupport(BackpressureKind.UNBOUNDED_IN) @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext, Consumer onError, @@ -14558,7 +14558,7 @@ public final Disposable subscribe(Consumer onNext, ConsumerReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @BackpressureSupport(BackpressureKind.SPECIAL) @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext, Consumer onError, diff --git a/src/main/java/io/reactivex/Maybe.java b/src/main/java/io/reactivex/Maybe.java index b1d34260f1..a02af760d9 100644 --- a/src/main/java/io/reactivex/Maybe.java +++ b/src/main/java/io/reactivex/Maybe.java @@ -4095,7 +4095,7 @@ public final Disposable subscribe() { * if {@code onSuccess} is null * @see ReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onSuccess) { return subscribe(onSuccess, Functions.ON_ERROR_MISSING, Functions.EMPTY_ACTION); @@ -4121,7 +4121,7 @@ public final Disposable subscribe(Consumer onSuccess) { * if {@code onSuccess} is null, or * if {@code onError} is null */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onSuccess, Consumer onError) { return subscribe(onSuccess, onError, Functions.EMPTY_ACTION); @@ -4151,7 +4151,7 @@ public final Disposable subscribe(Consumer onSuccess, ConsumerReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onSuccess, Consumer onError, Action onComplete) { diff --git a/src/main/java/io/reactivex/Observable.java b/src/main/java/io/reactivex/Observable.java index ef177b5c6a..8ed9912694 100644 --- a/src/main/java/io/reactivex/Observable.java +++ b/src/main/java/io/reactivex/Observable.java @@ -12079,7 +12079,7 @@ public final Disposable subscribe() { * if {@code onNext} is null * @see ReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext) { return subscribe(onNext, Functions.ON_ERROR_MISSING, Functions.EMPTY_ACTION, Functions.emptyConsumer()); @@ -12105,7 +12105,7 @@ public final Disposable subscribe(Consumer onNext) { * if {@code onNext} is null, or * if {@code onError} is null */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext, Consumer onError) { return subscribe(onNext, onError, Functions.EMPTY_ACTION, Functions.emptyConsumer()); @@ -12135,7 +12135,7 @@ public final Disposable subscribe(Consumer onNext, ConsumerReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext, Consumer onError, Action onComplete) { @@ -12169,7 +12169,7 @@ public final Disposable subscribe(Consumer onNext, ConsumerReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onNext, Consumer onError, Action onComplete, Consumer onSubscribe) { diff --git a/src/main/java/io/reactivex/Single.java b/src/main/java/io/reactivex/Single.java index 168c9c216a..aad0ac405c 100644 --- a/src/main/java/io/reactivex/Single.java +++ b/src/main/java/io/reactivex/Single.java @@ -3418,7 +3418,7 @@ public final Disposable subscribe() { * @throws NullPointerException * if {@code onCallback} is null */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(final BiConsumer onCallback) { ObjectHelper.requireNonNull(onCallback, "onCallback is null"); @@ -3446,7 +3446,7 @@ public final Disposable subscribe(final BiConsumer * if {@code onSuccess} is null * @see ReactiveX operators documentation: Subscribe */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(Consumer onSuccess) { return subscribe(onSuccess, Functions.ON_ERROR_MISSING); @@ -3471,7 +3471,7 @@ public final Disposable subscribe(Consumer onSuccess) { * if {@code onSuccess} is null, or * if {@code onError} is null */ - @CheckReturnValue + @OptionalCheckReturnValue @SchedulerSupport(SchedulerSupport.NONE) public final Disposable subscribe(final Consumer onSuccess, final Consumer onError) { ObjectHelper.requireNonNull(onSuccess, "onSuccess is null"); From ced572de13c7a8d0678d0983279699aeecd8160e Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 19 Nov 2018 22:36:31 -0800 Subject: [PATCH 3/3] Rework checkCheckReturnValueSupport test for optional check --- .../reactivex/validators/BaseTypeAnnotations.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/test/java/io/reactivex/validators/BaseTypeAnnotations.java b/src/test/java/io/reactivex/validators/BaseTypeAnnotations.java index 5445115b44..9b21c9a383 100644 --- a/src/test/java/io/reactivex/validators/BaseTypeAnnotations.java +++ b/src/test/java/io/reactivex/validators/BaseTypeAnnotations.java @@ -42,12 +42,18 @@ static void checkCheckReturnValueSupport(Class clazz) { continue; } if (m.getDeclaringClass() == clazz) { - boolean isSubscribeMethod = "subscribe".equals(m.getName()) && m.getParameterTypes().length == 0; + boolean isSubscribeMethod = "subscribe".equals(m.getName()); + boolean isNoArgSubscribeMethod = isSubscribeMethod && m.getParameterTypes().length == 0; boolean isAnnotationPresent = m.isAnnotationPresent(CheckReturnValue.class); + boolean isVoid = m.getReturnType().equals(Void.TYPE); if (isSubscribeMethod) { - if (isAnnotationPresent) { - b.append("subscribe() method has @CheckReturnValue: ").append(m).append("\r\n"); + if (isNoArgSubscribeMethod) { + if (isAnnotationPresent) { + b.append("subscribe() method has @CheckReturnValue: ").append(m).append("\r\n"); + } + } else if (!isVoid && !m.isAnnotationPresent(OptionalCheckReturnValue.class)) { + b.append("subscribe() method missing @OptionalCheckReturnValue: ").append(m).append("\r\n"); } continue; } @@ -57,7 +63,7 @@ static void checkCheckReturnValueSupport(Class clazz) { continue; } - if (m.getReturnType().equals(Void.TYPE)) { + if (isVoid) { if (isAnnotationPresent) { b.append("Void method has @CheckReturnValue: ").append(m).append("\r\n"); }