1.x: apply fixes based on PMD suggestions (ReactiveX#4091)

* 1.x: apply fixes based on PMD suggestions

* InternalError(Throwable cause) is not available

* Remove private from inner (static) classes, add final where needed

* Remove by akarnokd, update rules

pmd.xml

@@ -71,7 +71,6 @@
    <rule ref="rulesets/java/basic.xml/DontUseFloatTypeForLoopIndices"/>
    <rule ref="rulesets/java/basic.xml/DoubleCheckedLocking"/>
    <rule ref="rulesets/java/imports.xml/DuplicateImports"/>
-   <rule ref="rulesets/java/empty.xml/EmptyCatchBlock"/>
    <rule ref="rulesets/java/finalizers.xml/EmptyFinalizer"/>
    <rule ref="rulesets/java/empty.xml/EmptyFinallyBlock"/>
    <rule ref="rulesets/java/empty.xml/EmptyInitializer"/>
@@ -134,9 +133,6 @@
    <rule ref="rulesets/java/javabeans.xml/MissingSerialVersionUID"/>
    <rule ref="rulesets/java/design.xml/MissingStaticMethodInNonInstantiatableClass"/>
    <rule ref="rulesets/java/logging-java.xml/MoreThanOneLogger"/>
-   <rule ref="rulesets/java/codesize.xml/NcssConstructorCount"/>
-   <rule ref="rulesets/java/codesize.xml/NcssMethodCount"/>
-   <rule ref="rulesets/java/codesize.xml/NcssTypeCount"/>
    <rule ref="rulesets/java/naming.xml/NoPackage"/>
    <rule ref="rulesets/java/design.xml/NonCaseLabelInSwitchStatement"/>
    <rule ref="rulesets/java/design.xml/NonStaticInitializer"/>
@@ -160,7 +156,6 @@
    <rule ref="rulesets/java/design.xml/ReturnEmptyArrayRatherThanNull"/>
    <rule ref="rulesets/java/basic.xml/ReturnFromFinallyBlock"/>
    <rule ref="rulesets/java/migrating.xml/ShortInstantiation"/>
-   <rule ref="rulesets/java/naming.xml/ShortMethodName"/>
    <rule ref="rulesets/java/strictexception.xml/SignatureDeclareThrowsException"/>
    <rule ref="rulesets/java/design.xml/SimpleDateFormatNeedsLocale"/>
    <rule ref="rulesets/java/junit.xml/SimplifyBooleanAssertion"/>
@@ -183,8 +178,6 @@
    <rule ref="rulesets/java/junit.xml/TestClassWithoutTestCases"/>
    <rule ref="rulesets/java/design.xml/TooFewBranchesForASwitchStatement"/>
    <rule ref="rulesets/java/imports.xml/TooManyStaticImports"/>
-   <rule ref="rulesets/java/design.xml/UncommentedEmptyConstructor"/>
-   <rule ref="rulesets/java/design.xml/UncommentedEmptyMethodBody"/>
    <rule ref="rulesets/java/basic.xml/UnconditionalIfStatement"/>
    <rule ref="rulesets/java/junit.xml/UnnecessaryBooleanAssertion"/>
    <rule ref="rulesets/java/strings.xml/UnnecessaryCaseChange"/>

src/main/java/rx/BackpressureOverflow.java

@@ -24,6 +24,30 @@
 @Experimental
 public final class BackpressureOverflow {
 
+    /**
+     * Signal a MissingBackressureException due to lack of requests.
+     */
+    public static final BackpressureOverflow.Strategy ON_OVERFLOW_ERROR = Error.INSTANCE;
+
+    /**
+     * By default, signal a MissingBackressureException due to lack of requests.
+     */
+    public static final BackpressureOverflow.Strategy ON_OVERFLOW_DEFAULT = ON_OVERFLOW_ERROR;
+
+    /**
+     * Drop the oldest value in the buffer.
+     */
+    public static final BackpressureOverflow.Strategy ON_OVERFLOW_DROP_OLDEST = DropOldest.INSTANCE;
+
+    /**
+     * Drop the latest value.
+     */
+    public static final BackpressureOverflow.Strategy ON_OVERFLOW_DROP_LATEST = DropLatest.INSTANCE;
+
+    /**
+     * Represents a callback called when a value is about to be dropped
+     * due to lack of downstream requests.
+     */
     public interface Strategy {
 
         /**
@@ -36,14 +60,6 @@ public interface Strategy {
         boolean mayAttemptDrop() throws MissingBackpressureException;
     }
 
-    public static final BackpressureOverflow.Strategy ON_OVERFLOW_DEFAULT = Error.INSTANCE;
-
-    public static final BackpressureOverflow.Strategy ON_OVERFLOW_ERROR = Error.INSTANCE;
-
-    public static final BackpressureOverflow.Strategy ON_OVERFLOW_DROP_OLDEST = DropOldest.INSTANCE;
-
-    public static final BackpressureOverflow.Strategy ON_OVERFLOW_DROP_LATEST = DropLatest.INSTANCE;
-
     /**
      * Drop oldest items from the buffer making room for newer ones.
      */

src/main/java/rx/Completable.java

@@ -38,6 +38,8 @@
  */
 @Experimental
 public class Completable {
+    /** The actual subscription action. */
+    private final CompletableOnSubscribe onSubscribe;
     /**
      * Callback used for building deferred computations that takes a CompletableSubscriber.
      */
@@ -192,6 +194,22 @@ public void call(final CompletableSubscriber s) {
                 final CompositeSubscription set = new CompositeSubscription();
                 s.onSubscribe(set);
 
+                Iterator<? extends Completable> it;
+
+                try {
+                    it = sources.iterator();
+                } catch (Throwable e) {
+                    s.onError(e);
+                    return;
+                }
+
+                if (it == null) {
+                    s.onError(new NullPointerException("The iterator returned is null"));
+                    return;
+                }
+
+                boolean empty = true;
+
                 final AtomicBoolean once = new AtomicBoolean();
 
                 CompletableSubscriber inner = new CompletableSubscriber() {
@@ -220,22 +238,6 @@ public void onSubscribe(Subscription d) {
 
                 };
 
-                Iterator<? extends Completable> it;
-
-                try {
-                    it = sources.iterator();
-                } catch (Throwable e) {
-                    s.onError(e);
-                    return;
-                }
-
-                if (it == null) {
-                    s.onError(new NullPointerException("The iterator returned is null"));
-                    return;
-                }
-
-                boolean empty = true;
-
                 for (;;) {
                     if (once.get() || set.isUnsubscribed()) {
                         return;
@@ -377,7 +379,7 @@ public static Completable create(CompletableOnSubscribe onSubscribe) {
         requireNonNull(onSubscribe);
         try {
             return new Completable(onSubscribe);
-        } catch (NullPointerException ex) {
+        } catch (NullPointerException ex) { // NOPMD 
             throw ex;
         } catch (Throwable ex) {
             RxJavaHooks.onError(ex);
@@ -849,7 +851,7 @@ public static <R> Completable using(final Func0<R> resourceFunc0,
         return create(new CompletableOnSubscribe() {
             @Override
             public void call(final CompletableSubscriber s) {
-                final R resource;
+                final R resource; // NOPMD 
 
                 try {
                     resource = resourceFunc0.call();
@@ -965,9 +967,6 @@ public void call() {
         });
     }
 
-    /** The actual subscription action. */
-    private final CompletableOnSubscribe onSubscribe;
-
     /**
      * Constructs a Completable instance with the given onSubscribe callback.
      * @param onSubscribe the callback that will receive CompletableSubscribers when they subscribe,
@@ -1550,7 +1549,7 @@ public void call(CompletableSubscriber s) {
                     CompletableSubscriber sw = onLiftDecorated.call(s);
 
                     unsafeSubscribe(sw);
-                } catch (NullPointerException ex) {
+                } catch (NullPointerException ex) { // NOPMD 
                     throw ex;
                 } catch (Throwable ex) {
                     throw toNpe(ex);
@@ -2008,7 +2007,7 @@ public final void unsafeSubscribe(CompletableSubscriber s) {
             CompletableOnSubscribe onSubscribeDecorated = RxJavaHooks.onCompletableStart(this, this.onSubscribe);
 
             onSubscribeDecorated.call(s);
-        } catch (NullPointerException ex) {
+        } catch (NullPointerException ex) { // NOPMD 
             throw ex;
         } catch (Throwable ex) {
             Exceptions.throwIfFatal(ex);
@@ -2072,7 +2071,7 @@ public void onSubscribe(Subscription d) {
                 }
             });
             RxJavaHooks.onObservableReturn(s);
-        } catch (NullPointerException ex) {
+        } catch (NullPointerException ex) { // NOPMD 
             throw ex;
         } catch (Throwable ex) {
             Exceptions.throwIfFatal(ex);

src/main/java/rx/Notification.java

@@ -182,22 +182,27 @@ public enum Kind {
 
     @Override
     public String toString() {
-        StringBuilder str = new StringBuilder("[").append(super.toString()).append(" ").append(getKind());
-        if (hasValue())
-            str.append(" ").append(getValue());
-        if (hasThrowable())
-            str.append(" ").append(getThrowable().getMessage());
-        str.append("]");
+        StringBuilder str = new StringBuilder(64).append('[').append(super.toString())
+                .append(' ').append(getKind());
+        if (hasValue()) {
+            str.append(' ').append(getValue());
+        }
+        if (hasThrowable()) {
+            str.append(' ').append(getThrowable().getMessage());
+        }
+        str.append(']');
         return str.toString();
     }
 
     @Override
     public int hashCode() {
         int hash = getKind().hashCode();
-        if (hasValue())
+        if (hasValue()) {
             hash = hash * 31 + getValue().hashCode();
-        if (hasThrowable())
+        }
+        if (hasThrowable()) {
             hash = hash * 31 + getThrowable().hashCode();
+        }
         return hash;
     }
 
@@ -224,10 +229,6 @@ public boolean equals(Object obj) {
             return false;
         }
 
-        if (!(throwable == notification.throwable || (throwable != null && throwable.equals(notification.throwable)))) {
-            return false;
-        }
-
-        return true;
+        return (throwable == notification.throwable || (throwable != null && throwable.equals(notification.throwable)));
     }
 }

src/main/java/rx/Observable.java

@@ -9015,7 +9015,7 @@ public final Subscription unsafeSubscribe(Subscriber<? super T> subscriber) {
                 // TODO could the hook be the cause of the error in the on error handling.
                 RxJavaHooks.onObservableError(r);
                 // TODO why aren't we throwing the hook's return value.
-                throw r;
+                throw r; // NOPMD 
             }
             return Subscriptions.unsubscribed();
         }
@@ -9112,7 +9112,7 @@ static <T> Subscription subscribe(Subscriber<? super T> subscriber, Observable<T
                     // TODO could the hook be the cause of the error in the on error handling.
                     RxJavaHooks.onObservableError(r);
                     // TODO why aren't we throwing the hook's return value.
-                    throw r;
+                    throw r; // NOPMD 
                 }
             }
             return Subscriptions.unsubscribed();
@@ -9306,12 +9306,13 @@ public final Observable<T> takeFirst(Func1<? super T, Boolean> predicate) {
      * @see <a href="http://reactivex.io/documentation/operators/takelast.html">ReactiveX operators documentation: TakeLast</a>
      */
     public final Observable<T> takeLast(final int count) {
-        if (count == 0)
+        if (count == 0) {
             return ignoreElements();
-        else if (count == 1 )
+        } else if (count == 1) {
             return lift(OperatorTakeLastOne.<T>instance());
-        else 
+        } else {
             return lift(new OperatorTakeLast<T>(count));
+        }
     }
 
     /**

src/main/java/rx/Single.java

@@ -619,7 +619,7 @@ public static <T> Single<T> fromCallable(final Callable<? extends T> func) {
         return create(new OnSubscribe<T>() {
             @Override
             public void call(SingleSubscriber<? super T> singleSubscriber) {
-                final T value;
+                T value;
 
                 try {
                     value = func.call();
@@ -1719,7 +1719,7 @@ public final Subscription unsafeSubscribe(Subscriber<? super T> subscriber) {
                 // TODO could the hook be the cause of the error in the on error handling.
                 RxJavaHooks.onSingleError(r);
                 // TODO why aren't we throwing the hook's return value.
-                throw r;
+                throw r; // NOPMD 
             }
             return Subscriptions.unsubscribed();
         }
@@ -1829,7 +1829,7 @@ public final Subscription subscribe(Subscriber<? super T> subscriber) {
                 // TODO could the hook be the cause of the error in the on error handling.
                 RxJavaHooks.onSingleError(r);
                 // TODO why aren't we throwing the hook's return value.
-                throw r;
+                throw r; // NOPMD 
             }
             return Subscriptions.empty();
         }
@@ -1875,7 +1875,7 @@ public final Subscription subscribe(final SingleSubscriber<? super T> te) {
 
             @Override
             public void onCompleted() {
-
+                // deliberately ignored
             }
 
             @Override
@@ -2376,6 +2376,7 @@ public final Single<T> doOnError(final Action1<Throwable> onError) {
         Observer<T> observer = new Observer<T>() {
             @Override
             public void onCompleted() {
+                // deliberately ignored
             }
 
             @Override
@@ -2385,6 +2386,7 @@ public void onError(Throwable e) {
 
             @Override
             public void onNext(T t) {
+                // deliberately ignored
             }
         };
 
@@ -2410,10 +2412,12 @@ public final Single<T> doOnSuccess(final Action1<? super T> onSuccess) {
         Observer<T> observer = new Observer<T>() {
             @Override
             public void onCompleted() {
+                // deliberately ignored
             }
 
             @Override
             public void onError(Throwable e) {
+                // deliberately ignored
             }
 
             @Override
@@ -2589,7 +2593,7 @@ public final Single<T> doAfterTerminate(Action0 action) {
      */
     @SuppressWarnings("unchecked")
     static <T> Single<? extends T>[] iterableToArray(final Iterable<? extends Single<? extends T>> singlesIterable) {
-        final Single<? extends T>[] singlesArray;
+        Single<? extends T>[] singlesArray;
         int count;
 
         if (singlesIterable instanceof Collection) {

src/main/java/rx/exceptions/AssemblyStackTraceException.java

@@ -46,7 +46,7 @@ public AssemblyStackTraceException(String message) {
     }
 
     @Override
-    public synchronized Throwable fillInStackTrace() {
+    public synchronized Throwable fillInStackTrace() { // NOPMD 
         return this;
     }
 }