From c1a26c3fabaed3ba90d27d135c2f16f99926258c Mon Sep 17 00:00:00 2001 From: akarnokd Date: Wed, 22 Jan 2020 12:18:10 +0100 Subject: [PATCH 1/2] 3.x: Verify the use of base interfaces in operator inputs & lambdas --- .../io/reactivex/rxjava3/core/Flowable.java | 4 +- .../validators/OperatorsUseInterfaces.java | 189 ++++++++++++++++++ 2 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 src/test/java/io/reactivex/rxjava3/validators/OperatorsUseInterfaces.java diff --git a/src/main/java/io/reactivex/rxjava3/core/Flowable.java b/src/main/java/io/reactivex/rxjava3/core/Flowable.java index 038c3f680b..b85b04fd5e 100644 --- a/src/main/java/io/reactivex/rxjava3/core/Flowable.java +++ b/src/main/java/io/reactivex/rxjava3/core/Flowable.java @@ -6790,7 +6790,7 @@ public final Flowable> buffer(long timespan, @NonNull TimeUnit unit, @No @SchedulerSupport(SchedulerSupport.NONE) @NonNull public final Flowable> buffer( - @NonNull Flowable openingIndicator, + @NonNull Publisher openingIndicator, @NonNull Function> closingIndicator) { return buffer(openingIndicator, closingIndicator, ArrayListSupplier.asSupplier()); } @@ -6831,7 +6831,7 @@ public final Flowable> buffer( @SchedulerSupport(SchedulerSupport.NONE) @NonNull public final > Flowable buffer( - @NonNull Flowable openingIndicator, + @NonNull Publisher openingIndicator, @NonNull Function> closingIndicator, @NonNull Supplier bufferSupplier) { Objects.requireNonNull(openingIndicator, "openingIndicator is null"); diff --git a/src/test/java/io/reactivex/rxjava3/validators/OperatorsUseInterfaces.java b/src/test/java/io/reactivex/rxjava3/validators/OperatorsUseInterfaces.java new file mode 100644 index 0000000000..e521c15b6e --- /dev/null +++ b/src/test/java/io/reactivex/rxjava3/validators/OperatorsUseInterfaces.java @@ -0,0 +1,189 @@ +/** + * 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.rxjava3.validators; + +import static org.junit.Assert.*; + +import java.lang.reflect.*; +import java.util.*; +import java.util.Observable; +import java.util.concurrent.Callable; + +import org.junit.Test; +import org.reactivestreams.Publisher; + +import io.reactivex.rxjava3.core.*; +import io.reactivex.rxjava3.functions.*; +import io.reactivex.rxjava3.parallel.ParallelFlowable; + +/** + * Verify that an operator method uses base interfaces as its direct input or + * has lambdas returning base interfaces. + */ +public class OperatorsUseInterfaces { + + @Test + public void checkFlowable() { + checkClass(Flowable.class); + } + + @Test + public void checkObservable() { + checkClass(Observable.class); + } + + @Test + public void checkMaybe() { + checkClass(Maybe.class); + } + + @Test + public void checkSingle() { + checkClass(Single.class); + } + + @Test + public void checkCompletable() { + checkClass(Completable.class); + } + + @Test + public void checkParallelFlowable() { + checkClass(ParallelFlowable.class); + } + + void checkClass(Class clazz) { + StringBuilder error = new StringBuilder(); + int errors = 0; + + for (Method method : clazz.getMethods()) { + if (method.getDeclaringClass() == clazz) { + int pidx = 1; + for (Parameter param : method.getParameters()) { + Class type = param.getType(); + if (type.isArray()) { + type = type.getComponentType(); + } + if (CLASSES.contains(type)) { + errors++; + error.append("Non-interface input parameter #") + .append(pidx) + .append(": ") + .append(type) + .append("\r\n") + .append(" ") + .append(method) + .append("\r\n") + ; + } + if (CAN_RETURN.contains(type)) { + Type gtype = method.getGenericParameterTypes()[pidx - 1]; + if (gtype instanceof GenericArrayType) { + gtype = ((GenericArrayType)gtype).getGenericComponentType(); + } + ParameterizedType ptype = (ParameterizedType)gtype; + for (;;) { + Type[] parameterArgTypes = ptype.getActualTypeArguments(); + Type argType = parameterArgTypes[parameterArgTypes.length - 1]; + if (argType instanceof GenericArrayType) { + argType = ((GenericArrayType)argType).getGenericComponentType(); + } + if (argType instanceof ParameterizedType) { + ParameterizedType lastArg = (ParameterizedType)argType; + + if (CLASSES.contains(lastArg.getRawType())) { + errors++; + error.append("Non-interface lambda return #") + .append(pidx) + .append(": ") + .append(type) + .append("\r\n") + .append(" ") + .append(method) + .append("\r\n") + ; + } + + if (CAN_RETURN.contains(lastArg.getRawType())) { + ptype = lastArg; + continue; + } + } + break; + } + } + pidx++; + } + } + } + + if (errors != 0) { + error.insert(0, "Found " + errors + " issues\r\n"); + fail(error.toString()); + } + } + + public void method1(Flowable f) { + // self-test + } + + public void method2(Callable> c) { + // self-test + } + + public void method3(Supplier>> c) { + // self-test + } + + public void method4(Flowable[] array) { + // self-test + } + + public void method5(Callable[]> c) { + // self-test + } + + public void method6(Callable[]>> c) { + // self-test + } + + @Test + public void checkSelf() { + try { + checkClass(OperatorsUseInterfaces.class); + throw new RuntimeException("Should have failed"); + } catch (AssertionError expected) { + assertTrue(expected.toString(), expected.toString().contains("method1")); + assertTrue(expected.toString(), expected.toString().contains("method2")); + assertTrue(expected.toString(), expected.toString().contains("method3")); + assertTrue(expected.toString(), expected.toString().contains("method4")); + assertTrue(expected.toString(), expected.toString().contains("method5")); + assertTrue(expected.toString(), expected.toString().contains("method6")); + } + } + + static final Set> CLASSES = new HashSet<>(Arrays.asList( + Flowable.class, Observable.class, + Maybe.class, Single.class, + Completable.class + )); + + static final Set> CAN_RETURN = new HashSet<>(Arrays.asList( + Callable.class, Supplier.class, + Function.class, BiFunction.class, Function3.class, Function4.class, + Function5.class, Function6.class, Function7.class, Function8.class, + Function9.class, + Publisher.class, ObservableSource.class, MaybeSource.class, SingleSource.class + )); +} From 31cd8b8fbf56502938d900a2d0096abefa1a7140 Mon Sep 17 00:00:00 2001 From: akarnokd Date: Wed, 22 Jan 2020 12:53:09 +0100 Subject: [PATCH 2/2] Add @NonNull annotations too. --- src/main/java/io/reactivex/rxjava3/core/Flowable.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/reactivex/rxjava3/core/Flowable.java b/src/main/java/io/reactivex/rxjava3/core/Flowable.java index b85b04fd5e..5f5aff68c6 100644 --- a/src/main/java/io/reactivex/rxjava3/core/Flowable.java +++ b/src/main/java/io/reactivex/rxjava3/core/Flowable.java @@ -6790,7 +6790,7 @@ public final Flowable> buffer(long timespan, @NonNull TimeUnit unit, @No @SchedulerSupport(SchedulerSupport.NONE) @NonNull public final Flowable> buffer( - @NonNull Publisher openingIndicator, + @NonNull Publisher<@NonNull ? extends TOpening> openingIndicator, @NonNull Function> closingIndicator) { return buffer(openingIndicator, closingIndicator, ArrayListSupplier.asSupplier()); } @@ -6831,7 +6831,7 @@ public final Flowable> buffer( @SchedulerSupport(SchedulerSupport.NONE) @NonNull public final > Flowable buffer( - @NonNull Publisher openingIndicator, + @NonNull Publisher<@NonNull ? extends TOpening> openingIndicator, @NonNull Function> closingIndicator, @NonNull Supplier bufferSupplier) { Objects.requireNonNull(openingIndicator, "openingIndicator is null");