From cc1122836f7ed9d7b236c80fa1f4c438ec0b91db Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 13 Mar 2023 17:59:36 -0700 Subject: [PATCH] Use checkstyle for javadoc checks (#551) * Use checkstyle for javadoc checks Signed-off-by: Daniel Widdis * Add missing javadoc tags Signed-off-by: Daniel Widdis --------- Signed-off-by: Daniel Widdis Signed-off-by: Kuanysh --- build.gradle | 33 +----------- config/checkstyle/checkstyle.xml | 54 +++++++++++++++++++ config/checkstyle/checkstyle_suppressions.xml | 20 +++++++ gradle/formatting.gradle | 7 --- .../org/opensearch/sdk/ActionExtension.java | 2 + .../sdk/BaseExtensionRestHandler.java | 1 + .../org/opensearch/sdk/EngineExtension.java | 4 ++ .../java/org/opensearch/sdk/SDKClient.java | 6 +++ .../org/opensearch/sdk/SearchExtension.java | 2 + 9 files changed, 91 insertions(+), 38 deletions(-) create mode 100644 config/checkstyle/checkstyle.xml create mode 100644 config/checkstyle/checkstyle_suppressions.xml diff --git a/build.gradle b/build.gradle index 0b846229..8206539c 100644 --- a/build.gradle +++ b/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) { diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml new file mode 100644 index 00000000..2869b661 --- /dev/null +++ b/config/checkstyle/checkstyle.xml @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/config/checkstyle/checkstyle_suppressions.xml b/config/checkstyle/checkstyle_suppressions.xml new file mode 100644 index 00000000..e5d2e2b7 --- /dev/null +++ b/config/checkstyle/checkstyle_suppressions.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + diff --git a/gradle/formatting.gradle b/gradle/formatting.gradle index ee675364..3e8120bb 100644 --- a/gradle/formatting.gradle +++ b/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() diff --git a/src/main/java/org/opensearch/sdk/ActionExtension.java b/src/main/java/org/opensearch/sdk/ActionExtension.java index fc0bdbff..8cd2cb47 100644 --- a/src/main/java/org/opensearch/sdk/ActionExtension.java +++ b/src/main/java/org/opensearch/sdk/ActionExtension.java @@ -105,6 +105,8 @@ default Collection getTaskHeaders() { * } * } * + * + * @param threadContext The Thread Context which can be used by the operator */ default UnaryOperator getRestHandlerWrapper(ThreadContext threadContext) { return null; diff --git a/src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java index 82aa27b6..edb5f8d6 100644 --- a/src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java +++ b/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) { diff --git a/src/main/java/org/opensearch/sdk/EngineExtension.java b/src/main/java/org/opensearch/sdk/EngineExtension.java index c44834ee..95a47f0c 100644 --- a/src/main/java/org/opensearch/sdk/EngineExtension.java +++ b/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 getEngineFactory(IndexSettings indexSettings) { @@ -43,6 +44,9 @@ default Optional 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 getCustomCodecServiceFactory(IndexSettings indexSettings) { return Optional.empty(); diff --git a/src/main/java/org/opensearch/sdk/SDKClient.java b/src/main/java/org/opensearch/sdk/SDKClient.java index d8ecc6db..1117b935 100644 --- a/src/main/java/org/opensearch/sdk/SDKClient.java +++ b/src/main/java/org/opensearch/sdk/SDKClient.java @@ -294,6 +294,8 @@ 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; @@ -301,6 +303,8 @@ public SDKRestClient admin() { /** * 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 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()); diff --git a/src/main/java/org/opensearch/sdk/SearchExtension.java b/src/main/java/org/opensearch/sdk/SearchExtension.java index 16b73c1a..e5387338 100644 --- a/src/main/java/org/opensearch/sdk/SearchExtension.java +++ b/src/main/java/org/opensearch/sdk/SearchExtension.java @@ -93,6 +93,8 @@ default List> /** * The new {@link FetchSubPhase}s defined by this Extension. + * + * @param context The context for which to fetch the subphases. */ default List getFetchSubPhases(FetchPhaseConstructionContext context) { return emptyList();