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

Use checkstyle for javadoc checks #551

Merged
merged 2 commits into from
Mar 14, 2023
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
33 changes: 2 additions & 31 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ plugins {
id "com.diffplug.spotless" version "6.16.0" apply false
id 'jacoco'
id "com.form.diff-coverage" version "0.9.5"
// for javadocs and checks spotless doesn't do
id 'checkstyle'
}


Expand Down Expand Up @@ -71,10 +73,6 @@ repositories {
maven { url "https://d1nvenhzbhpy0q.cloudfront.net/snapshots/lucene/"}
}

configurations {
requireJavadoc
}

dependencies {

def opensearchVersion = "3.0.0-SNAPSHOT"
Expand All @@ -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}")
Expand Down Expand Up @@ -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) {
Expand Down
54 changes: 54 additions & 0 deletions config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
@@ -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>

owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
</module>
</module>
20 changes: 20 additions & 0 deletions config/checkstyle/checkstyle_suppressions.xml
Original file line number Diff line number Diff line change
@@ -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>
7 changes: 0 additions & 7 deletions gradle/formatting.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/opensearch/sdk/ActionExtension.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/opensearch/sdk/EngineExtension.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/opensearch/sdk/SDKClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/opensearch/sdk/SearchExtension.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down