Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.x: Fix method argument naming across types #6853

Merged
merged 2 commits into from
Jan 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 76 additions & 76 deletions src/main/java/io/reactivex/rxjava3/core/Completable.java

Large diffs are not rendered by default.

148 changes: 75 additions & 73 deletions src/main/java/io/reactivex/rxjava3/core/Flowable.java

Large diffs are not rendered by default.

111 changes: 56 additions & 55 deletions src/main/java/io/reactivex/rxjava3/core/Maybe.java

Large diffs are not rendered by default.

269 changes: 136 additions & 133 deletions src/main/java/io/reactivex/rxjava3/core/Observable.java

Large diffs are not rendered by default.

154 changes: 78 additions & 76 deletions src/main/java/io/reactivex/rxjava3/core/Single.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@
import io.reactivex.rxjava3.schedulers.TestScheduler;

public class BlockingFlowableMostRecentTest extends RxJavaTest {
@Test
public void mostRecentNull() {
assertNull(Flowable.<Void>never().blockingMostRecent(null).iterator().next());
}

@Test
public void mostRecent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@
import io.reactivex.rxjava3.subjects.*;

public class BlockingObservableMostRecentTest extends RxJavaTest {
@Test
public void mostRecentNull() {
assertNull(Observable.<Void>never().blockingMostRecent(null).iterator().next());
}

static <T> Iterable<T> mostRecent(Observable<T> source, T initialValue) {
return source.blockingMostRecent(initialValue);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/**
* 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.internal.util;

import java.lang.reflect.*;
import java.util.*;

import org.reactivestreams.*;

import com.google.common.base.Strings;

import io.reactivex.rxjava3.core.*;
import io.reactivex.rxjava3.core.Observer;
import io.reactivex.rxjava3.core.Observable;

/**
* Compare method argument naming across base classes.
* This is not a full test because some naming mismatch is legitimate, such as singular in Maybe/Single and
* plural in Flowable/Observable
*/
public final class OperatorArgumentNaming {

private OperatorArgumentNaming() {
throw new IllegalStateException("No instances!");
}

/** Classes to compare with each other. */
static final Class<?>[] CLASSES = { Flowable.class, Observable.class, Maybe.class, Single.class, Completable.class };

/** Types that refer to a reactive type and is generally matching the parent class; for comparison, these have to be unified. */
static final Set<Class<?>> BASE_TYPE_SET = new HashSet<>(Arrays.asList(
Flowable.class, Publisher.class, Subscriber.class, FlowableSubscriber.class,
Observable.class, ObservableSource.class, Observer.class,
Maybe.class, MaybeSource.class, MaybeObserver.class,
Single.class, SingleSource.class, SingleObserver.class,
Completable.class, CompletableSource.class, CompletableObserver.class
));

public static void main(String[] args) {
// className -> methodName -> overloads -> arguments
Map<String, Map<String, List<List<ArgumentNameAndType>>>> map = new HashMap<>();

for (Class<?> clazz : CLASSES) {
Map<String, List<List<ArgumentNameAndType>>> classMethods = map.computeIfAbsent(clazz.getSimpleName(), v -> new HashMap<>());
for (Method method : clazz.getDeclaredMethods()) {
if (method.getDeclaringClass() == clazz && method.getParameterCount() != 0) {
List<List<ArgumentNameAndType>> overloads = classMethods.computeIfAbsent(method.getName(), v -> new ArrayList<>());

List<ArgumentNameAndType> overload = new ArrayList<>();
overloads.add(overload);

for (Parameter param : method.getParameters()) {
String typeName;
Class<?> type = param.getType();
if (type.isArray()) {
Class<?> componentType = type.getComponentType();
if (BASE_TYPE_SET.contains(componentType)) {
typeName = "BaseType";
} else {
typeName = type.getComponentType().getSimpleName() + "[]";
}
} else
if (BASE_TYPE_SET.contains(type)) {
typeName = "BaseType";
} else {
typeName = type.getSimpleName();
}
String name = param.getName();
if (name.equals("bufferSize") || name.equals("prefetch") || name.equals("capacityHint")) {
name = "bufferSize|prefetch|capacityHint";
}
if (name.equals("subscriber") || name.equals("observer")) {
name = "subscriber|observer";
}
if (name.contains("onNext")) {
name = name.replace("onNext", "onNext|onSuccess");
} else
if (name.contains("onSuccess")) {
name = name.replace("onSuccess", "onNext|onSuccess");
}
overload.add(new ArgumentNameAndType(typeName, name));
}
}
}
}

int counter = 0;

for (int i = 0; i < CLASSES.length - 1; i++) {
String firstName = CLASSES[i].getSimpleName();
Map<String, List<List<ArgumentNameAndType>>> firstClassMethods = map.get(firstName);
for (int j = i + 1; j < CLASSES.length; j++) {
String secondName = CLASSES[j].getSimpleName();
Map<String, List<List<ArgumentNameAndType>>> secondClassMethods = map.get(secondName);

for (Map.Entry<String, List<List<ArgumentNameAndType>>> methodOverloadsFirst : firstClassMethods.entrySet()) {

List<List<ArgumentNameAndType>> methodOverloadsSecond = secondClassMethods.get(methodOverloadsFirst.getKey());

if (methodOverloadsSecond != null) {
for (List<ArgumentNameAndType> overloadFirst : methodOverloadsFirst.getValue()) {
for (List<ArgumentNameAndType> overloadSecond : methodOverloadsSecond) {
if (overloadFirst.size() == overloadSecond.size()) {
// Argument types match?
boolean match = true;
for (int k = 0; k < overloadFirst.size(); k++) {
if (!overloadFirst.get(k).type.equals(overloadSecond.get(k).type)) {
match = false;
break;
}
}
// Argument names match?
if (match) {
for (int k = 0; k < overloadFirst.size(); k++) {
if (!overloadFirst.get(k).name.equals(overloadSecond.get(k).name)) {
System.out.print("Argument naming mismatch #");
System.out.println(++counter);

System.out.print(" ");
System.out.print(Strings.padEnd(firstName, Math.max(firstName.length(), secondName.length()) + 1, ' '));
System.out.print(methodOverloadsFirst.getKey());
System.out.print(" ");
System.out.println(overloadFirst);

System.out.print(" ");
System.out.print(Strings.padEnd(secondName, Math.max(firstName.length(), secondName.length()) + 1, ' '));
System.out.print(methodOverloadsFirst.getKey());
System.out.print(" ");
System.out.println(overloadSecond);
System.out.println();
break;
}
}
}
}
}
}
}
}
}
}
}

static final class ArgumentNameAndType {
final String type;
final String name;

ArgumentNameAndType(String type, String name) {
this.type = type;
this.name = name;
}

@Override
public String toString() {
return type + " " + name;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,6 @@ public void checkParallelFlowable() {
addOverride(new ParamOverride(Flowable.class, 0, ParamMode.ANY, "delay", Long.TYPE, TimeUnit.class, Scheduler.class));
addOverride(new ParamOverride(Flowable.class, 0, ParamMode.ANY, "delay", Long.TYPE, TimeUnit.class, Scheduler.class, Boolean.TYPE));

// null default is allowed
addOverride(new ParamOverride(Flowable.class, 0, ParamMode.ANY, "blockingMostRecent", Object.class));

// negative time is considered as zero time
addOverride(new ParamOverride(Flowable.class, 0, ParamMode.ANY, "delaySubscription", Long.TYPE, TimeUnit.class));
addOverride(new ParamOverride(Flowable.class, 0, ParamMode.ANY, "delaySubscription", Long.TYPE, TimeUnit.class, Scheduler.class));
Expand Down Expand Up @@ -390,9 +387,6 @@ public void checkParallelFlowable() {
addOverride(new ParamOverride(Observable.class, 0, ParamMode.ANY, "delay", Long.TYPE, TimeUnit.class, Scheduler.class));
addOverride(new ParamOverride(Observable.class, 0, ParamMode.ANY, "delay", Long.TYPE, TimeUnit.class, Scheduler.class, Boolean.TYPE));

// null default is allowed
addOverride(new ParamOverride(Observable.class, 0, ParamMode.ANY, "blockingMostRecent", Object.class));

// negative time is considered as zero time
addOverride(new ParamOverride(Observable.class, 0, ParamMode.ANY, "delaySubscription", Long.TYPE, TimeUnit.class));
addOverride(new ParamOverride(Observable.class, 0, ParamMode.ANY, "delaySubscription", Long.TYPE, TimeUnit.class, Scheduler.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ static void processFile(Class<?> clazz) throws Exception {

int quote = line.indexOf('"', comma);

String message = line.substring(quote + 1, quote + 2 + paramName.length());
String message = line.substring(quote + 1, Math.min(line.length(), quote + 2 + paramName.length()));

if (line.contains("\"A Disposable")) {
continue;
Expand Down