Skip to content

Commit

Permalink
[Backport 2.x] Migrating from checkstyle to spotless (#648 and #651) (#…
Browse files Browse the repository at this point in the history
…661)

* Migrating from checkstyle to spotless for formatting (#648)

* Adding spotless for checking and fixing formatting and removing checkstyle

Signed-off-by: Vacha Shah <[email protected]>

* Making build task depend on spotlessJavaCheck

Signed-off-by: Vacha Shah <[email protected]>

* Removing resources and documentation related to checkstyle

Signed-off-by: Vacha Shah <[email protected]>

* Running spotless for json, transport and util folders

Signed-off-by: Vacha Shah <[email protected]>

* Running spotless for samples

Signed-off-by: Vacha Shah <[email protected]>

* Removing commented code

Signed-off-by: Vacha Shah <[email protected]>

---------

Signed-off-by: Vacha Shah <[email protected]>

* Fixing merge conflicts and build

Signed-off-by: Vacha Shah <[email protected]>

* Enabling spotless for all modules from PR 651

Signed-off-by: Vacha Shah <[email protected]>

* Applying spotless to all the remaining files

Signed-off-by: Vacha Shah <[email protected]>

---------

Signed-off-by: Vacha Shah <[email protected]>
  • Loading branch information
VachaShah authored Oct 12, 2023
1 parent 53489bc commit 119cabc
Show file tree
Hide file tree
Showing 1,660 changed files with 272,379 additions and 272,548 deletions.
24 changes: 0 additions & 24 deletions .github/workflows/checkstyle.yml

This file was deleted.

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### Changed
- Moved "software.amazon.awssdk" dependencies to the compileOnly scope. ([#628](https://github.com/opensearch-project/opensearch-java/pull/628))
- Migrated from checkstyle to spotless ([#648](https://github.com/opensearch-project/opensearch-java/pull/648))

### Deprecated

Expand Down
25 changes: 21 additions & 4 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,28 @@ Follow links in the [Java Tutorial](https://code.visualstudio.com/docs/java/java

## Java Language Formatting Guidelines

Java files in the opensearch-java codebase are formatted with the [checkstyle plugin](https://docs.gradle.org/current/userguide/checkstyle_plugin.html). This plugin is configured using [checkstyle.xml](config/checkstyle/checkstyle.xml). To run the formatting checks:
Java files in the OpenSearch codebase are formatted with the Eclipse JDT formatter, using the [Spotless Gradle](https://github.com/diffplug/spotless/tree/master/plugin-gradle) plugin. This plugin is configured on a project-by-project basis, via `build.gradle.kts`. So long as at least one project is configured, the formatting check can be run explicitly with:

```
./gradlew checkstyleMain checkstyleTest
```
./gradlew spotlessJavaCheck

The code can be formatted with:

./gradlew spotlessApply

These tasks can also be run for specific subprojects, e.g.

./gradlew :java-client:spotlessJavaCheck
./gradlew :samples:spotlessJavaCheck

Please follow these formatting guidelines:

* Java indent is 4 spaces
* Line width is 140 characters
* Lines of code surrounded by `// tag::NAME` and `// end::NAME` comments are included in the documentation and should only be 76 characters wide not counting leading indentation. Such regions of code are not formatted automatically as it is not possible to change the line length rule of the formatter for part of a file. Please format such sections sympathetically with the rest of the code, while keeping lines to maximum length of 76 characters.
* Wildcard imports (`import foo.bar.baz.*`) are forbidden and will cause the build to fail.
* If *absolutely* necessary, you can disable formatting for regions of code with the `// tag::NAME` and `// end::NAME` directives, but note that these are intended for use in documentation, so please make it clear what you have done, and only do this where the benefit clearly outweighs the decrease in consistency.
* Note that JavaDoc and block comments i.e. `/* ... */` are not formatted, but line comments i.e `// ...` are.
* There is an implicit rule that negative boolean expressions should use the form `foo == false` instead of `!foo` for better readability of the code. While this isn't strictly enforced, if might get called out in PR reviews as something to change.

## Submitting Changes

Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
[![Checkstyle](https://github.com/opensearch-project/opensearch-java/actions/workflows/checkstyle.yml/badge.svg?branch=main)](https://github.com/opensearch-project/opensearch-java/actions/workflows/checkstyle.yml)
[![Build](https://github.com/opensearch-project/opensearch-java/actions/workflows/build.yml/badge.svg?branch=main)](https://github.com/opensearch-project/opensearch-java/actions/workflows/build.yml)
[![Integration Tests](https://github.com/opensearch-project/opensearch-java/actions/workflows/test-integration.yml/badge.svg?branch=main)](https://github.com/opensearch-project/opensearch-java/actions/workflows/test-integration.yml)
![Maven Central](https://img.shields.io/maven-central/v/org.opensearch.client/opensearch-java)
Expand Down
2 changes: 0 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ allprojects {
mavenCentral()
maven(url = "https://plugins.gradle.org/m2/")
}

apply(plugin = "checkstyle")
}

// Find git information.
Expand Down
362 changes: 362 additions & 0 deletions buildSrc/formatterConfig.xml

Large diffs are not rendered by default.

156 changes: 0 additions & 156 deletions config/checkstyle/checkstyle.xml

This file was deleted.

44 changes: 0 additions & 44 deletions config/checkstyle/checkstyle_suppressions.xml

This file was deleted.

7 changes: 0 additions & 7 deletions config/checkstyle/header.java.txt

This file was deleted.

26 changes: 21 additions & 5 deletions java-client/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ buildscript {
plugins {
java
`java-library`
checkstyle
`maven-publish`
id("com.github.jk1.dependency-license-report") version "2.5"
id("org.owasp.dependencycheck") version "8.4.0"
id("com.diffplug.spotless") version "6.22.0"
}

apply(plugin = "org.owasp.dependencycheck")
Expand All @@ -60,10 +60,6 @@ configurations {
}
}

checkstyle {
toolVersion = "10.12.3"
}

java {
targetCompatibility = JavaVersion.VERSION_11
sourceCompatibility = JavaVersion.VERSION_11
Expand Down Expand Up @@ -113,6 +109,10 @@ tasks.withType<Jar> {
}
}

tasks.build {
dependsOn("spotlessJavaCheck")
}

tasks.test {
systemProperty("tests.security.manager", "false")

Expand Down Expand Up @@ -265,6 +265,22 @@ tasks.withType<Jar> {
}
}

spotless {
java {

target("**/*.java")

// Use the default importOrder configuration
importOrder()
removeUnusedImports()

eclipse().configFile("../buildSrc/formatterConfig.xml")

trimTrailingWhitespace()
endWithNewline()
}
}

publishing {
repositories{
if (version.toString().endsWith("SNAPSHOT")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,11 @@

package org.opensearch.client;

import org.opensearch.client.transport.TransportOptions;
import org.opensearch.client.transport.Transport;
import javax.annotation.Nullable;
import org.opensearch.client.json.JsonpDeserializer;
import org.opensearch.client.json.JsonpMapperBase;

import javax.annotation.Nullable;
import org.opensearch.client.transport.Transport;
import org.opensearch.client.transport.TransportOptions;

public abstract class ApiClient<T extends Transport, Self extends ApiClient<T, Self>> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public boolean ignoreUnknownFields() {
@SuppressWarnings("unchecked")
public <T> T attribute(String name) {
if (this.name.equals(name)) {
return (T)this.value;
return (T) this.value;
} else {
return mapper.attribute(name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
package org.opensearch.client.json;

import jakarta.json.stream.JsonParser;

import java.util.function.Function;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
package org.opensearch.client.json;

import jakarta.json.stream.JsonParser;

import java.util.EnumSet;

public abstract class DelegatingDeserializer<T, U> implements JsonpDeserializer<T> {
Expand Down Expand Up @@ -68,7 +67,7 @@ public T deserialize(JsonParser parser, JsonpMapper mapper, JsonParser.Event eve
*/
public static JsonpDeserializer<?> unwrap(JsonpDeserializer<?> deserializer) {
while (deserializer instanceof DelegatingDeserializer) {
deserializer = ((DelegatingDeserializer<?,?>) deserializer).unwrap();
deserializer = ((DelegatingDeserializer<?, ?>) deserializer).unwrap();
}
return deserializer;
}
Expand Down
Loading

0 comments on commit 119cabc

Please sign in to comment.