Added Javadoc checks to Checkstyle. Fix violating Javadoc. (#7210)

benjamintboyle · web-flow · commit 5c0ca0359262 · 2021-03-12T08:41:35.000+01:00
Add JavadocMethod and MissingJavadocMethod checks to Checkstyle.
Checks ensure there are no missing Javadoc comments for public methods,
 and that each of the Javadoc comments describe all method parameters.

Add suppressions.xml Checkstyle configuration file to exclude Jmh, Test,
and Internal from MissingJavadocMethod check.

Move Checkstyle configuration files and License headers into config directory.

Fix Javadoc comments that violate the added checks.
diff --git a/build.gradle b/build.gradle
@@ -129,7 +129,7 @@ jar {
 }
 
 license {
-    header rootProject.file("HEADER")
+    header rootProject.file("config/license/HEADER")
     ext.year = Calendar.getInstance().get(Calendar.YEAR)
     skipExistingHeaders true
     ignoreFailures true
@@ -234,10 +234,11 @@ jacocoTestReport.dependsOn GCandMem
 build.dependsOn jacocoTestReport
 
 checkstyle {
-    configFile = file("checkstyle.xml")
+    configFile = rootProject.file("config/checkstyle/checkstyle.xml")
     toolVersion = checkstyleVersion
     configProperties = [
-            "checkstyle.header.file": file("HEADER_JAVA")
+            "checkstyle.suppressions.file": rootProject.file("config/checkstyle/suppressions.xml"),
+            "checkstyle.header.file": rootProject.file("config/license/HEADER_JAVA")
     ]
 }
 
diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml
@@ -4,6 +4,9 @@
         "https://checkstyle.org/dtds/configuration_1_3.dtd">
 
 <module name="Checker">
+    <module name="SuppressionFilter">
+        <property name="file" value="${checkstyle.suppressions.file}"/>
+    </module>
 
     <!-- Headers -->
     <module name="Header">
@@ -12,6 +15,9 @@
     </module>
 
     <module name="TreeWalker">
+        <module name="JavadocMethod"/>
+        <module name="MissingJavadocMethod"/>
+
         <module name="RegexpSinglelineJava">
             <property name="severity" value="warning"/>
             <property name="format" value="^(?!\s+\* $).*?\s+$"/>
diff --git a/config/checkstyle/suppressions.xml b/config/checkstyle/suppressions.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE suppressions PUBLIC
+    "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
+    "https://checkstyle.org/dtds/suppressions_1_2.dtd">
+
+<suppressions>
+
+  <suppress checks="MissingJavadocMethod" files="[\\/]main[\\/]java[\\/]io[\\/]reactivex[\\/]rxjava3[\\/]internal[\\/]"/>
+  <suppress checks="MissingJavadocMethod" files="[\\/]jmh[\\/]java[\\/]io[\\/]reactivex[\\/]rxjava3[\\/]"/>
+  <suppress checks="MissingJavadocMethod" files="[\\/]test[\\/]java[\\/]io[\\/]reactivex[\\/]rxjava3[\\/]"/>
+
+</suppressions>
diff --git a/config/license/HEADER b/config/license/HEADER
diff --git a/config/license/HEADER_JAVA b/config/license/HEADER_JAVA
diff --git a/src/main/java/io/reactivex/rxjava3/core/Completable.java b/src/main/java/io/reactivex/rxjava3/core/Completable.java
@@ -1908,11 +1908,12 @@ public final Completable doOnLifecycle(@NonNull Consumer<? super Disposable> onS
      *  <dt><b>Scheduler:</b></dt>
      *  <dd>{@code doOnLifecycle} does not operate by default on a particular {@link Scheduler}.</dd>
      * </dl>
-     * @param onSubscribe the consumer called when a {@link CompletableObserver} subscribes.
-     * @param onError the consumer called when this emits an {@code onError} event
-     * @param onComplete the runnable called just before when the current {@code Completable} completes normally
-     * @param onAfterTerminate the runnable called after this {@code Completable} completes normally
-     * @param onDispose the {@link Runnable} called when the downstream disposes the subscription
+     * @param onSubscribe the {@link Consumer} called when a {@link CompletableObserver} subscribes.
+     * @param onError the {@code Consumer} called when this emits an {@code onError} event
+     * @param onComplete the {@link Action} called just before when the current {@code Completable} completes normally
+     * @param onTerminate the {@code Action} called just before this {@code Completable} terminates
+     * @param onAfterTerminate the {@code Action} called after this {@code Completable} completes normally
+     * @param onDispose the {@code Action} called when the downstream disposes the subscription
      * @return the new {@code Completable} instance
      * @throws NullPointerException if {@code onSubscribe}, {@code onError}, {@code onComplete}
      * {@code onTerminate}, {@code onAfterTerminate} or {@code onDispose} is {@code null}
diff --git a/src/main/java/io/reactivex/rxjava3/core/Flowable.java b/src/main/java/io/reactivex/rxjava3/core/Flowable.java
@@ -9723,6 +9723,10 @@ public final Flowable<T> doOnComplete(@NonNull Action onComplete) {
      *  <dd>{@code doOnEach} does not operate by default on a particular {@link Scheduler}.</dd>
      * </dl>
      *
+     * @param onNext the {@link Consumer} to invoke when the current {@code Flowable} calls {@code onNext}
+     * @param onError the {@code Consumer} to invoke when the current {@code Flowable} calls {@code onError}
+     * @param onComplete the {@link Action} to invoke when the current {@code Flowable} calls {@code onComplete}
+     * @param onAfterTerminate the {@code Action} to invoke when the current {@code Flowable} calls {@code onAfterTerminate}
      * @return the new {@code Flowable} instance
      * @throws NullPointerException if {@code onNext}, {@code onError}, {@code onComplete} or {@code onAfterTerminate} is {@code null}
      * @see <a href="http://reactivex.io/documentation/operators/do.html">ReactiveX operators documentation: Do</a>
diff --git a/src/main/java/io/reactivex/rxjava3/core/Notification.java b/src/main/java/io/reactivex/rxjava3/core/Notification.java
@@ -26,7 +26,9 @@ public final class Notification<T> {
 
     final Object value;
 
-    /** Not meant to be implemented externally. */
+    /** Not meant to be implemented externally.
+     * @param value the value to carry around in the notification, not {@code null}
+     */
     private Notification(@Nullable Object value) {
         this.value = value;
     }
diff --git a/src/main/java/io/reactivex/rxjava3/core/Observable.java b/src/main/java/io/reactivex/rxjava3/core/Observable.java
@@ -8608,6 +8608,10 @@ public final Observable<T> doOnComplete(@NonNull Action onComplete) {
      *  <dd>{@code doOnEach} does not operate by default on a particular {@link Scheduler}.</dd>
      * </dl>
      *
+     * @param onNext the {@link Consumer} to invoke when the current {@code Observable} calls {@code onNext}
+     * @param onError the {@code Consumer} to invoke when the current {@code Observable} calls {@code onError}
+     * @param onComplete the {@link Action} to invoke when the current {@code Observable} calls {@code onComplete}
+     * @param onAfterTerminate the {@code Action} to invoke when the current {@code Observable} calls {@code onAfterTerminate}
      * @return the new {@code Observable} instance
      * @throws NullPointerException if {@code onNext}, {@code onError}, {@code onComplete} or {@code onAfterTerminate} is {@code null}
      * @see <a href="http://reactivex.io/documentation/operators/do.html">ReactiveX operators documentation: Do</a>
diff --git a/src/main/java/io/reactivex/rxjava3/exceptions/CompositeException.java b/src/main/java/io/reactivex/rxjava3/exceptions/CompositeException.java
@@ -229,7 +229,9 @@ private void appendStackTrace(StringBuilder b, Throwable ex, String prefix) {
     }
 
     abstract static class PrintStreamOrWriter {
-        /** Prints the specified string as a line on this StreamOrWriter. */
+        /** Prints the specified string as a line on this StreamOrWriter.
+         * @param o string to print
+         */
         abstract void println(Object o);
     }
 
diff --git a/src/main/java/io/reactivex/rxjava3/internal/operators/flowable/FlowableReplay.java b/src/main/java/io/reactivex/rxjava3/internal/operators/flowable/FlowableReplay.java
@@ -118,10 +118,11 @@ public static <T> ConnectableFlowable<T> create(Flowable<T> source,
     }
 
     /**
-     * Creates a OperatorReplay instance to replay values of the given source observable.
-     * @param source the source observable
-     * @param bufferFactory the factory to instantiate the appropriate buffer when the observable becomes active
-     * @return the connectable observable
+     * Creates a OperatorReplay instance to replay values of the given source {@code Flowable}.
+     * @param <T> the value type
+     * @param source the source {@code Flowable} to use
+     * @param bufferFactory the factory to instantiate the appropriate buffer when the {@code Flowable} becomes active
+     * @return the {@code ConnectableFlowable} instance
      */
     static <T> ConnectableFlowable<T> create(Flowable<T> source,
             final Supplier<? extends ReplayBuffer<T>> bufferFactory) {
@@ -544,6 +545,7 @@ public void dispose() {
         }
         /**
          * Convenience method to auto-cast the index object.
+         * @param <U> type to cast index object
          * @return the current index object
          */
         @SuppressWarnings("unchecked")
diff --git a/src/main/java/io/reactivex/rxjava3/internal/operators/mixed/ScalarXMapZHelper.java b/src/main/java/io/reactivex/rxjava3/internal/operators/mixed/ScalarXMapZHelper.java
@@ -78,6 +78,7 @@ static <T> boolean tryAsCompletable(Object source,
      * Try subscribing to a {@link MaybeSource} mapped from
      * a scalar source (which implements {@link Supplier}).
      * @param <T> the upstream value type
+     * @param <R> the downstream value type
      * @param source the source reactive type ({@code Flowable} or {@code Observable})
      *               possibly implementing {@link Supplier}.
      * @param mapper the function that turns the scalar upstream value into a
@@ -117,6 +118,7 @@ static <T, R> boolean tryAsMaybe(Object source,
      * Try subscribing to a {@link SingleSource} mapped from
      * a scalar source (which implements {@link Supplier}).
      * @param <T> the upstream value type
+     * @param <R> the downstream value type
      * @param source the source reactive type ({@code Flowable} or {@code Observable})
      *               possibly implementing {@link Supplier}.
      * @param mapper the function that turns the scalar upstream value into a
diff --git a/src/main/java/io/reactivex/rxjava3/internal/operators/observable/ObservableReplay.java b/src/main/java/io/reactivex/rxjava3/internal/operators/observable/ObservableReplay.java
@@ -122,6 +122,7 @@ public static <T> ConnectableObservable<T> create(ObservableSource<T> source,
 
     /**
      * Creates a OperatorReplay instance to replay values of the given source observable.
+     * @param <T> the value type
      * @param source the source observable
      * @param bufferFactory the factory to instantiate the appropriate buffer when the observable becomes active
      * @return the connectable observable
@@ -453,6 +454,7 @@ public void dispose() {
         }
         /**
          * Convenience method to auto-cast the index object.
+         * @param <U> type index to be casted to
          * @return the index Object or null
          */
         @SuppressWarnings("unchecked")
diff --git a/src/main/java/io/reactivex/rxjava3/internal/util/QueueDrainHelper.java b/src/main/java/io/reactivex/rxjava3/internal/util/QueueDrainHelper.java
@@ -291,6 +291,7 @@ static boolean isCancelled(BooleanSupplier cancelled) {
     /**
      * Drains the queue based on the outstanding requests in post-completed mode (only!).
      *
+     * @param <T> the value type
      * @param n the current request amount
      * @param actual the target Subscriber to send events to
      * @param queue the queue to drain if in the post-complete state
diff --git a/src/main/java/io/reactivex/rxjava3/observers/BaseTestConsumer.java b/src/main/java/io/reactivex/rxjava3/observers/BaseTestConsumer.java
@@ -54,6 +54,9 @@ public abstract class BaseTestConsumer<T, U extends BaseTestConsumer<T, U>> {
      */
     protected boolean timeout;
 
+    /**
+     * Constructs a {@code BaseTestConsumer} with {@code CountDownLatch} set to 1.
+     */
     public BaseTestConsumer() {
         this.values = new VolatileSizeArrayList<>();
         this.errors = new VolatileSizeArrayList<>();
diff --git a/src/test/java/io/reactivex/rxjava3/exceptions/CompositeExceptionTest.java b/src/test/java/io/reactivex/rxjava3/exceptions/CompositeExceptionTest.java
@@ -148,7 +148,7 @@ public void compositeExceptionFromTwoDuplicateComposites() {
         cex.getCause().printStackTrace();
     }
 
-    /**
+    /*
      * This hijacks the Throwable.printStackTrace() output and puts it in a string, where we can look for
      * "CIRCULAR REFERENCE" (a String added by Throwable.printEnclosedStackTrace)
      */
diff --git a/src/test/java/io/reactivex/rxjava3/schedulers/SchedulerTestHelper.java b/src/test/java/io/reactivex/rxjava3/schedulers/SchedulerTestHelper.java
@@ -28,6 +28,8 @@ private SchedulerTestHelper() {
     /**
      * Verifies that the given Scheduler does not deliver handled errors to its executing Thread's
      * {@link java.lang.Thread.UncaughtExceptionHandler}.
+     *
+     * @param scheduler {@link Scheduler} to verify.
      */
     static void handledErrorIsNotDeliveredToThreadHandler(Scheduler scheduler) throws InterruptedException {
         Thread.UncaughtExceptionHandler originalHandler = Thread.getDefaultUncaughtExceptionHandler();

Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ jar {`
`129`	`129`	`}`
`130`	`130`
`131`	`131`	`license {`
`132`		`- header rootProject.file("HEADER")`
	`132`	`+ header rootProject.file("config/license/HEADER")`
`133`	`133`	`ext.year = Calendar.getInstance().get(Calendar.YEAR)`
`134`	`134`	`skipExistingHeaders true`
`135`	`135`	`ignoreFailures true`
`@@ -234,10 +234,11 @@ jacocoTestReport.dependsOn GCandMem`
`234`	`234`	`build.dependsOn jacocoTestReport`
`235`	`235`
`236`	`236`	`checkstyle {`
`237`		`- configFile = file("checkstyle.xml")`
	`237`	`+ configFile = rootProject.file("config/checkstyle/checkstyle.xml")`
`238`	`238`	`toolVersion = checkstyleVersion`
`239`	`239`	`configProperties = [`
`240`		`- "checkstyle.header.file": file("HEADER_JAVA")`
	`240`	`+ "checkstyle.suppressions.file": rootProject.file("config/checkstyle/suppressions.xml"),`
	`241`	`+ "checkstyle.header.file": rootProject.file("config/license/HEADER_JAVA")`
`241`	`242`	`]`
`242`	`243`	`}`
`243`	`244`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,9 @@ public final class Notification<T> {`
`26`	`26`
`27`	`27`	`final Object value;`
`28`	`28`
`29`		`- /** Not meant to be implemented externally. */`
	`29`	`+ /** Not meant to be implemented externally.`
	`30`	`+ * @param value the value to carry around in the notification, not {@code null}`
	`31`	`+ */`
`30`	`32`	`private Notification(@Nullable Object value) {`
`31`	`33`	`this.value = value;`
`32`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -229,7 +229,9 @@ private void appendStackTrace(StringBuilder b, Throwable ex, String prefix) {`
`229`	`229`	`}`
`230`	`230`
`231`	`231`	`abstract static class PrintStreamOrWriter {`
`232`		`- /** Prints the specified string as a line on this StreamOrWriter. */`
	`232`	`+ /** Prints the specified string as a line on this StreamOrWriter.`
	`233`	`+ * @param o string to print`
	`234`	`+ */`
`233`	`235`	`abstract void println(Object o);`
`234`	`236`	`}`
`235`	`237`