Use checkstyle for javadoc checks (opensearch-project#551)

* Use checkstyle for javadoc checks

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Add missing javadoc tags

Signed-off-by: Daniel Widdis <widdis@gmail.com>

---------

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Nurym <nurym0807@gmail.com>

build.gradle

@@ -17,6 +17,8 @@ plugins {
     id "com.diffplug.spotless" version "6.17.0" apply false
     id 'jacoco'
     id "com.form.diff-coverage" version "0.9.5"
+    // for javadocs and checks spotless doesn't do
+    id 'checkstyle'
 }
 
 
@@ -71,10 +73,6 @@ repositories {
     maven { url "https://d1nvenhzbhpy0q.cloudfront.net/snapshots/lucene/"}
 }
 
-configurations {
-  requireJavadoc
-}
-
 dependencies {
 
     def opensearchVersion = "3.0.0-SNAPSHOT"
@@ -87,7 +85,6 @@ dependencies {
     def junit5Version = "5.9.2"
     def junitPlatform = "1.9.2"
     def jaxbVersion = "2.3.1"
-    def requireJavadocVersion = "1.0.6"
 
     implementation("org.opensearch:opensearch:${opensearchVersion}")
     implementation("org.apache.logging.log4j:log4j-api:${log4jVersion}")
@@ -117,33 +114,7 @@ dependencies {
     testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${junit5Version}")
     testImplementation("org.opensearch.test:framework:${opensearchVersion}")
     testRuntimeOnly("org.junit.platform:junit-platform-launcher:${junitPlatform}")
-    requireJavadoc("org.plumelib:require-javadoc:${requireJavadocVersion}")
-}
-
-// this task tests for the presence of javadocs but not the content/tags
-task requireJavadoc(type: JavaExec) {
-  group = 'Documentation'
-  description = 'Ensures that Javadoc documentation exists.'
-  mainClass = "org.plumelib.javadoc.RequireJavadoc"
-  classpath = configurations.requireJavadoc
-  args "src/main/java"
-  // javadocs on private methods optional
-  args "--dont-require-private=true"
-  // javadocs on trivial getters/setters optional
-  args "--dont-require-trivial-properties"
-}
-check.dependsOn requireJavadoc
-
-// this task checks the content/tags of existing javadocs
-task javadocStrict(type: Javadoc) {
-  group = 'Documentation'
-  description = 'Run Javadoc in strict mode: with -Xdoclint:all, on all members.'
-  source = sourceSets.main.allJava
-  classpath = sourceSets.main.runtimeClasspath
-  options.addStringOption('Xdoclint:all', '-quiet')
-  options.memberLevel = JavadocMemberLevel.PRIVATE
 }
-check.dependsOn javadocStrict
 
 // this task runs the helloworld sample extension
 task helloWorld(type: JavaExec) {

config/checkstyle/checkstyle.xml

@@ -0,0 +1,54 @@
+<?xml version="1.0"?>
+<!--
+  ~ SPDX-License-Identifier: Apache-2.0
+
+  ~ The OpenSearch Contributors require contributions made to
+  ~ this file be licensed under the Apache-2.0 license or a
+  ~ compatible open source license.
+  -->
+
+<!DOCTYPE module PUBLIC
+        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+        "https://checkstyle.org/dtds/configuration_1_3.dtd">
+
+<!--
+    Formatting checks are done by spotless. This config is only for checks spotless doesn't do.
+-->
+
+<module name="Checker">
+    <property name="charset" value="UTF-8" />
+
+    <module name="SuppressionFilter">
+        <property name="file" value="${config_loc}/checkstyle_suppressions.xml" />
+    </module>
+
+    <module name="TreeWalker">
+        <!-- Disallows star imports -->
+        <module name="AvoidStarImport" />
+
+        <!--  Requires javadoc on public and protected interfaces, classes, enums -->
+        <module name="MissingJavadocType">
+            <property name="scope" value="protected"/>
+        </module>
+
+        <!-- Requires javadoc on public and protected methods, excluding getters and setters -->
+        <module name="MissingJavadocMethod">
+            <property name="scope" value="protected"/>
+            <property name="allowMissingPropertyJavadoc" value="true"/>
+        </module>
+
+        <!--  Checks that javadocs contain correct param and return -->
+        <module name="JavadocMethod">
+            <property name="id" value="JavadocMethod"/>
+            <property name="accessModifiers" value="public, protected"/>
+        </module>
+
+        <!-- Allows missing returns on extension interface methods -->
+        <module name="JavadocMethod">
+            <property name="id" value="JavadocMethodAllowMissingReturnTag"/>
+            <property name="accessModifiers" value="public, protected"/>
+            <property name="allowMissingReturnTag" value="true"/>
+        </module>
+
+    </module>
+</module>

config/checkstyle/checkstyle_suppressions.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0"?>
+<!--
+  ~ SPDX-License-Identifier: Apache-2.0
+
+  ~ The OpenSearch Contributors require contributions made to
+  ~ this file be licensed under the Apache-2.0 license or a
+  ~ compatible open source license.
+  -->
+
+<!DOCTYPE suppressions PUBLIC
+        "-//Checkstyle//DTD SuppressionFilter Configuration 1.1//EN"
+        "https://checkstyle.org/dtds/suppressions_1_1.dtd">
+
+<suppressions>
+    <!-- No javadoc checks on test classes -->
+    <suppress checks="Javadoc*" files="[\\/]src[\\/]test[\\/].*" />
+    <!-- No return required for Extension interface classes -->
+    <suppress id="JavadocMethod" files="\w+Extension\.java" />
+    <suppress id="JavadocMethodAllowMissingReturnTag" files="^((?!\w+Extension\.java).)*$" />
+</suppressions>

gradle/formatting.gradle

@@ -11,13 +11,6 @@ allprojects {
             trimTrailingWhitespace()
             endWithNewline();
 
-            custom 'Refuse wildcard imports', {
-                // Wildcard imports can't be resolved; fail the build
-                if (it =~ /\s+import .*\*;/) {
-                    throw new AssertionError("Do not use wildcard imports.  'spotlessApply' cannot resolve this issue.")
-                }
-            }
-
             // See DEVELOPER_GUIDE.md for details of when to enable this.
             if (System.getProperty('spotless.paddedcell') != null) {
                 paddedCell()

src/main/java/org/opensearch/sdk/ActionExtension.java

@@ -105,6 +105,8 @@ default Collection<String> getTaskHeaders() {
      *    }
      * }
      * </pre>
+     *
+     * @param threadContext The Thread Context which can be used by the operator
      */
     default UnaryOperator<ExtensionRestHandler> getRestHandlerWrapper(ThreadContext threadContext) {
         return null;

src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java

@@ -98,6 +98,7 @@ protected ExtensionRestResponse unhandledRequest(ExtensionRestRequest request) {
      * Returns a default response when a request handler throws an exception.
      *
      * @param request The request that caused the exception
+     * @param e The exception
      * @return an ExtensionRestResponse identifying the exception
      */
     protected ExtensionRestResponse exceptionalRequest(ExtensionRestRequest request, Exception e) {

src/main/java/org/opensearch/sdk/EngineExtension.java

@@ -31,6 +31,7 @@ public interface EngineExtension {
      * {@link Optional#empty()}. If multiple Extensions return an engine factory for a given index the index will not be created and an
      * {@link IllegalStateException} will be thrown during index creation.
      *
+     * @param indexSettings the index settings to inspect
      * @return an optional engine factory
      */
     default Optional<EngineFactory> getEngineFactory(IndexSettings indexSettings) {
@@ -43,6 +44,9 @@ default Optional<EngineFactory> getEngineFactory(IndexSettings indexSettings) {
      * to determine if a custom {@link CodecServiceFactory} should be provided for the given index. A Extension that is not overriding
      * the {@link CodecServiceFactory} through the Extension can ignore this method and the default Codec specified in the
      * {@link IndexSettings} will be used.
+     *
+     * @param indexSettings the index settings to inspect
+     * @return an optional engine factory
      */
     default Optional<CodecServiceFactory> getCustomCodecServiceFactory(IndexSettings indexSettings) {
         return Optional.empty();

src/main/java/org/opensearch/sdk/SDKClient.java

@@ -294,13 +294,17 @@ public SDKRestClient(SDKClient sdkClient, RestHighLevelClient restHighLevelClien
 
         /**
          * The admin client that can be used to perform administrative operations.
+         *
+         * @return An instance of this client. Method provided for backwards compatibility.
          */
         public SDKRestClient admin() {
             return this;
         }
 
         /**
          * A client allowing to perform actions/operations against the cluster.
+         *
+         * @return An instance of a cluster admin client.
          */
         public SDKClusterAdminClient cluster() {
             return new SDKClusterAdminClient(restHighLevelClient.cluster());
@@ -325,6 +329,8 @@ public final <Request extends ActionRequest, Response extends ActionResponse> vo
 
         /**
          * A client allowing to perform actions/operations against the indices.
+         *
+         * @return An instance of an indices client.
          */
         public SDKIndicesClient indices() {
             return new SDKIndicesClient(restHighLevelClient.indices());

src/main/java/org/opensearch/sdk/SearchExtension.java

@@ -93,6 +93,8 @@ default List<SearchExtensionSpec<MovAvgModel, MovAvgModel.AbstractModelParser>>
 
     /**
      * The new {@link FetchSubPhase}s defined by this Extension.
+     *
+     * @param context The context for which to fetch the subphases.
      */
     default List<FetchSubPhase> getFetchSubPhases(FetchPhaseConstructionContext context) {
         return emptyList();