Java checkstyle (#1563) (#1577)

* Java checkstyle config (#1563) * use Google Java Style Guide (with some modifications) * Gradle plugin allowing running checkstyle locally * running checkstyle on CI with GitHub Action * using reviewdog to publish checkstyle warns in PR diffs * fix Hermes codebase to meet checkstyle requirements (#1563) * fix imports order * shorten lines longer than 140 characters * rename methods, fields, variables to meet naming conventions * fix missing whitespaces, braces around statements * fix maximum distance before variables declaration and usage * move trailing operators "+", "&&", "||" etc. to new lines * etc. * testing checkstyle on CI + reviewdog config (#1563) * revert renaming class/method/vars with abbreviations (#1563) * fix checkstyle after rebase (#1563) * suppress files copied from external libraries (#1563) * fix checkstyle (#1563)
allegro · Oct 19, 2022 · 8707e95 · 8707e95
1 parent 66e5d3f
commit 8707e95
Show file tree

Hide file tree

Showing 391 changed files with 2,937 additions and 1,959 deletions.
diff --git a/.github/workflows/checkstyle.yml b/.github/workflows/checkstyle.yml
@@ -0,0 +1,33 @@
+name: Checkstyle
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+jobs:
+  checkstyle:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+      - uses: reviewdog/action-setup@v1
+        with:
+          reviewdog_version: latest
+      - name: Set up JDK 11
+        uses: actions/setup-java@v2
+        with:
+          java-version: 11
+          distribution: 'adopt'
+      - name: Grant execute permission for gradlew
+        run: chmod +x gradlew
+      - name: Run check style
+        run: ./gradlew clean checkstyleMain
+      - name: Run reviewdog
+        env:
+          REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          find . -name 'main.xml' \
+            -exec bash -c 'reviewdog -f=checkstyle -reporter=github-check -filter-mode=nofilter -level=warning < "$1"' reviewdog-report {} \;
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -37,4 +37,4 @@ jobs:
       - name: Build with Gradle
         run: ./gradlew assemble
       - name: Run tests with Gradle
-        run: ./gradlew ${{ matrix.tests }}
+        run: ./gradlew ${{ matrix.tests }}
diff --git a/build.gradle b/build.gradle
@@ -39,6 +39,7 @@ nexusPublishing {
 allprojects {
     apply plugin: 'java'
     apply plugin: 'groovy'
+    apply plugin: 'checkstyle'
 
     group = 'pl.allegro.tech.hermes'
     version = scmVersion.version
@@ -176,6 +177,17 @@ subprojects {
             events 'passed', 'skipped', 'failed'
         }
     }
+
+    tasks.withType(Checkstyle) {
+        reports {
+            xml.required = true
+            html.required = false
+        }
+    }
+
+    checkstyle {
+        toolVersion '10.3.4'
+    }
 }
 
 def getIntProperty(String name, int defaultValue) {

diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml
diff --git a/config/checkstyle/suppressions.xml b/config/checkstyle/suppressions.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <!DOCTYPE suppressions PUBLIC
-        "-//Puppy Crawl//DTD Suppressions 1.1//EN"
-        "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
+        "-//Puppy Crawl//DTD Suppressions 1.2//EN"
+        "https://checkstyle.org/dtds/suppressions_1_2.dtd">
 
 <suppressions>
     <!-- Suppress all checks in test helpers -->
@@ -14,7 +14,7 @@
               files="[/\\]src[/\\](main)[/\\]java[/\\].+Properties\.java$"
             />
 
-    <!-- Suppressions for unit (test|integration)ing code -->
+    <!-- Suppressions for unit testing code -->
     <suppress checks="MagicNumber"
               files="[/\\]src[/\\](test|integration|jmh)[/\\]java[/\\]"
             />
@@ -36,4 +36,12 @@
     <suppress checks="VisibilityModifier"
               files="[/\\]src[/\\](test|integration)[/\\]java[/\\]"
             />
+
+    <!-- Build dirs -->
+    <suppress checks="[a-zA-Z0-9]*"
+              files="[/\\]build[/\\]generated[/\\]java[/\\]" />
+
+    <!-- Classes repackaged from external libraries -->
+    <suppress checks="[a-zA-Z0-9]*"
+              files="(DirectBufferPool|LinkedHashSetBlockingQueue)\.java" />
 </suppressions>
diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/AvroMediaType.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/AvroMediaType.java
@@ -2,7 +2,7 @@
 
 public class AvroMediaType {
 
-    public final static String AVRO_BINARY = "avro/binary";
+    public static final String AVRO_BINARY = "avro/binary";
 
-    public final static String AVRO_JSON = "avro/json";
+    public static final String AVRO_JSON = "avro/json";
 }
diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/BatchSubscriptionPolicy.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/BatchSubscriptionPolicy.java
@@ -4,9 +4,9 @@
 import com.google.common.base.MoreObjects;
 import pl.allegro.tech.hermes.api.helpers.Patch;
 
-import javax.validation.constraints.Min;
 import java.util.Map;
 import java.util.Objects;
+import javax.validation.constraints.Min;
 
 public class BatchSubscriptionPolicy {
 

diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/Constraints.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/Constraints.java
@@ -3,8 +3,8 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import javax.validation.constraints.Min;
 import java.util.Objects;
+import javax.validation.constraints.Min;
 
 public class Constraints {
 
@@ -22,8 +22,12 @@ public int getConsumersNumber() {
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
         Constraints that = (Constraints) o;
         return consumersNumber == that.consumersNumber;
     }

diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/ConsumerGroup.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/ConsumerGroup.java
@@ -43,17 +43,21 @@ public Set<ConsumerGroupMember> getMembers() {
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
         ConsumerGroup that = (ConsumerGroup) o;
-        return Objects.equals(clusterName, that.clusterName) &&
-                Objects.equals(groupId, that.groupId) &&
-                Objects.equals(state, that.state) &&
-                Objects.equals(members, that.members);
+        return Objects.equals(clusterName, that.clusterName)
+                && Objects.equals(groupId, that.groupId)
+                && Objects.equals(state, that.state)
+                && Objects.equals(members, that.members);
     }
 
     @Override
     public int hashCode() {
         return Objects.hash(clusterName, groupId, state, members);
     }
-}
+}
diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/ConsumerGroupMember.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/ConsumerGroupMember.java
@@ -42,13 +42,17 @@ public Set<TopicPartition> getPartitions() {
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
         ConsumerGroupMember that = (ConsumerGroupMember) o;
-        return Objects.equals(consumerId, that.consumerId) &&
-                Objects.equals(clientId, that.clientId) &&
-                Objects.equals(host, that.host) &&
-                Objects.equals(partitions, that.partitions);
+        return Objects.equals(consumerId, that.consumerId)
+                && Objects.equals(clientId, that.clientId)
+                && Objects.equals(host, that.host)
+                && Objects.equals(partitions, that.partitions);
     }
 
     @Override

diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/DatacenterReadiness.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/DatacenterReadiness.java
@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+
 import java.util.Objects;
 
 public class DatacenterReadiness {
@@ -25,19 +26,23 @@ public boolean isReady() {
 
     @Override
     public String toString() {
-        return "DatacenterReadiness{" +
-                "datacenter='" + datacenter + '\'' +
-                ", isReady=" + isReady +
-                '}';
+        return "DatacenterReadiness{"
+                + "datacenter='" + datacenter + '\''
+                + ", isReady=" + isReady
+                + '}';
     }
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (!(o instanceof DatacenterReadiness)) return false;
+        if (this == o) {
+            return true;
+        }
+        if (!(o instanceof DatacenterReadiness)) {
+            return false;
+        }
         DatacenterReadiness that = (DatacenterReadiness) o;
-        return isReady == that.isReady &&
-                Objects.equals(datacenter, that.datacenter);
+        return isReady == that.isReady
+                && Objects.equals(datacenter, that.datacenter);
     }
 
     @Override

diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/EndpointAddress.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/EndpointAddress.java
@@ -47,16 +47,15 @@ public EndpointAddress(String endpoint) {
         this.rawEndpoint = endpoint;
 
         Matcher matcher = URL_PATTERN.matcher(endpoint);
-        if(matcher.matches()) {
+        if (matcher.matches()) {
             this.protocol = matcher.group(PROTOCOL_GROUP);
             this.containsCredentials = !Strings.isNullOrEmpty(matcher.group(USER_INFO_GROUP));
 
             this.username = containsCredentials ? matcher.group(USERNAME_GROUP) : null;
             this.password = containsCredentials ? matcher.group(PASSWORD_GROUP) : null;
 
             this.endpoint = containsCredentials ? protocol + "://" + matcher.group(ADDRESS_GROUP) : endpoint;
-        }
-        else {
+        } else {
             this.protocol = null;
             this.containsCredentials = false;
             this.username = null;
@@ -75,6 +74,20 @@ private EndpointAddress(String protocol, String endpoint, String username) {
         this.rawEndpoint = protocol + "://" + username + ":" + password + "@" + endpoint.replace(protocol + "://", "");
     }
 
+    public static EndpointAddress of(String endpoint) {
+        return new EndpointAddress(endpoint);
+    }
+
+    public static EndpointAddress of(URI endpoint) {
+        return new EndpointAddress(endpoint.toString());
+    }
+
+    public static String extractProtocolFromAddress(String endpoint) {
+        Preconditions.checkArgument(endpoint.indexOf(':') != -1);
+
+        return endpoint.substring(0, endpoint.indexOf(':'));
+    }
+
     public String getEndpoint() {
         return endpoint;
     }
@@ -87,14 +100,6 @@ public URI getUri() {
         return URI.create(endpoint);
     }
 
-    public static EndpointAddress of(String endpoint) {
-        return new EndpointAddress(endpoint);
-    }
-
-    public static EndpointAddress of(URI endpoint) {
-        return new EndpointAddress(endpoint.toString());
-    }
-
     public String getProtocol() {
         return protocol;
     }
@@ -123,12 +128,6 @@ public String toString() {
                 .toString();
     }
 
-    public static String extractProtocolFromAddress(String endpoint) {
-        Preconditions.checkArgument(endpoint.indexOf(':') != -1);
-
-        return endpoint.substring(0, endpoint.indexOf(':'));
-    }
-
     public boolean containsCredentials() {
         return containsCredentials;
     }
@@ -142,7 +141,7 @@ public String getUsername() {
     }
 
     public EndpointAddress anonymize() {
-        if(containsCredentials) {
+        if (containsCredentials) {
             return new EndpointAddress(protocol, endpoint, username);
         }
         return this;

diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/EndpointAddressResolverMetadata.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/EndpointAddressResolverMetadata.java
@@ -7,13 +7,13 @@
 import com.fasterxml.jackson.databind.ser.std.StdSerializer;
 import com.google.common.collect.ImmutableMap;
 
-import javax.validation.constraints.NotNull;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import javax.validation.constraints.NotNull;
 
 @JsonSerialize(using = EndpointAddressResolverMetadata.EndpointAddressResolverMetadataSerializer.class)
 public class EndpointAddressResolverMetadata {
@@ -28,6 +28,14 @@ public EndpointAddressResolverMetadata(Map<String, Object> entries) {
         this.entries = ImmutableMap.copyOf(entries);
     }
 
+    public static EndpointAddressResolverMetadata empty() {
+        return EMPTY_INSTANCE;
+    }
+
+    public static Builder endpointAddressResolverMetadata() {
+        return new Builder();
+    }
+
     public Optional<Object> get(String key) {
         return Optional.ofNullable(entries.get(key));
     }
@@ -71,14 +79,6 @@ public void serialize(EndpointAddressResolverMetadata metadata, JsonGenerator jg
         }
     }
 
-    public static EndpointAddressResolverMetadata empty() {
-        return EMPTY_INSTANCE;
-    }
-
-    public static Builder endpointAddressResolverMetadata() {
-        return new Builder();
-    }
-
     public static class Builder {
 
         private Map<String, Object> entries = new HashMap<>();
@@ -92,4 +92,4 @@ public EndpointAddressResolverMetadata build() {
             return new EndpointAddressResolverMetadata(entries);
         }
     }
-}
+}
diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/ErrorCode.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/ErrorCode.java
@@ -2,7 +2,12 @@
 
 import javax.ws.rs.core.Response;
 
-import static javax.ws.rs.core.Response.Status.*;
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static javax.ws.rs.core.Response.Status.FORBIDDEN;
+import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR;
+import static javax.ws.rs.core.Response.Status.NOT_ACCEPTABLE;
+import static javax.ws.rs.core.Response.Status.NOT_FOUND;
+import static javax.ws.rs.core.Response.Status.REQUEST_TIMEOUT;
 
 public enum ErrorCode {
     TIMEOUT(REQUEST_TIMEOUT),

diff --git a/hermes-api/src/main/java/pl/allegro/tech/hermes/api/Group.java b/hermes-api/src/main/java/pl/allegro/tech/hermes/api/Group.java
@@ -3,14 +3,13 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-
-import javax.validation.constraints.NotNull;
 import java.util.Objects;
+import javax.validation.constraints.NotNull;
 
 public class Group {
 
     @NotNull
-    private String groupName;
+    private final String groupName;
 
     @JsonCreator
     public Group(@JsonProperty("groupName") String groupName) {