Skip to content

Commit

Permalink
Fix checkstyle (opensearch-project#3283)
Browse files Browse the repository at this point in the history
Fix checkstyle. Now it works as expected.

The problem is that chectyle works only with one config file. It does
not support multiple files except per-defined files like
`suppressions.xml`.
So the previous config effectively replaced `sun_checks.xml` and
validated only `System.out.println` lines in `tools`.
Since `println_checks.xml` replaced the main file we did not notice that
new version of checktyle removed `PrintlnModule` from the code base.

Changes:
- All code related to checking `tools` folder was moved in the main file
- Renamed the `sun_...xml` file to the `checktyle.xml` which is default
settings for checkstyle so we can track changes in it
- Set the latest version for `checktyle` so it can validate new JDK
features as well
 - Fixed problematic files which checkstyle highlighted

`System.out.println` I tested manually all I can say it works :-)

- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

Signed-off-by: Andrey Pleskach <ples@aiven.io>
(cherry picked from commit cd45e78)
Signed-off-by: Andrey Pleskach <ples@aiven.io>
  • Loading branch information
willyborankin committed Sep 5, 2023
1 parent d8a30d4 commit 2bf955e
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 33 deletions.
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ jacocoTestReport {
}

checkstyle {
configFile file("checkstyle/sun_checks.xml")
configFile file("checkstyle/println_checks.xml")
toolVersion "10.3.3"
configDirectory.set(rootProject.file("checkstyle/"))
}

opensearchplugin {
Expand Down
18 changes: 16 additions & 2 deletions checkstyle/sun_checks.xml → checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
To suppress certain violations please review suppression filters.
Finally, it is worth reading the documentation.
-->

<!-- exSUN codestyle check with the additional check for System.out.ptintln -->
<module name="Checker">
<!--
If you set the basedir property below, then all reported file
Expand All @@ -36,6 +36,13 @@
<module name="BeforeExecutionExclusionFileFilter">
<property name="fileNamePattern" value="module\-info\.java$"/>
</module>
<!-- System.out.ptintln -->
<module name="BeforeExecutionExclusionFileFilter">
<property name="fileNamePattern" value="src/main/java/org/opensearch/security/tools/*"/>
</module>
<module name="BeforeExecutionExclusionFileFilter">
<property name="fileNamePattern" value="src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java"/>
</module>

<!-- https://checkstyle.org/config_filters.html#SuppressionFilter -->
<module name="SuppressionFilter">
Expand Down Expand Up @@ -188,6 +195,14 @@
<property name="optional" value="true"/>
</module>

<!-- System.out.println -->
<module name="RegexpSinglelineJava">
<property name="format" value="System.out.println"/>
<property name="ignoreCase" value="true"/>
<property name="message" value="Do not use System.out.println" />
<property name="severity" value="error"/>
</module>

</module>

<module name="RegexpSingleline">
Expand Down Expand Up @@ -215,5 +230,4 @@
<property name="checkFormat" value="$1"/>
</module>

<module name="PrintlnModule"/>
</module>
21 changes: 0 additions & 21 deletions checkstyle/println_checks.xml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.ExceptionsHelper;
import org.opensearch.core.action.ActionListener;
import org.opensearch.action.index.IndexRequest;
import org.opensearch.action.index.IndexResponse;
import org.opensearch.action.support.WriteRequest.RefreshPolicy;
Expand All @@ -43,7 +42,6 @@
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.action.configupdate.ConfigUpdateAction;
import org.opensearch.security.action.configupdate.ConfigUpdateRequest;
import org.opensearch.security.action.configupdate.ConfigUpdateResponse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import com.google.common.collect.ImmutableList;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.core.action.ActionListener;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
import org.opensearch.core.action.ActionListener;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@
// CS-SUPPRESS-SINGLE: RegexpSingleline https://github.com/opensearch-project/OpenSearch/issues/3663

import com.google.common.collect.ImmutableList;

import org.opensearch.LegacyESVersion;
import org.opensearch.Version;
import org.opensearch.core.action.ActionListener;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.LegacyESVersion;
import org.opensearch.Version;
import org.opensearch.action.admin.indices.create.CreateIndexResponse;
import org.opensearch.action.bulk.BulkRequestBuilder;
import org.opensearch.action.bulk.BulkResponse;
Expand All @@ -29,7 +27,6 @@
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.Settings;
Expand Down

0 comments on commit 2bf955e

Please sign in to comment.