Add Checkstyle framework (apache#3551)

* Add Checkstyle framework

* Avoid star import

* Need braces for control flow statements

* Redundant imports

* Add NewLineAtEndOfFile check

benchmarks/src/main/java/io/druid/benchmark/datagen/BenchmarkSchemaInfo.java

@@ -19,7 +19,6 @@
 
 package io.druid.benchmark.datagen;
 
-import io.druid.benchmark.datagen.BenchmarkColumnSchema;
 import io.druid.query.aggregation.AggregatorFactory;
 import org.joda.time.Interval;
 

codestyle/checkstyle-suppressions.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE suppressions PUBLIC
+    "-//Puppy Crawl//DTD Suppressions 1.0//EN"
+    "http://www.puppycrawl.com/dtds/suppressions_1_0.dtd">
+
+<!--
+  ~ Licensed to Metamarkets Group Inc. (Metamarkets) under one
+  ~ or more contributor license agreements. See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership. Metamarkets licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License. You may obtain a copy of the License at
+  ~
+  ~ http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied. See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+  -->
+
+<suppressions>
+  <!-- See http://checkstyle.sourceforge.net/config_filters.html#SuppressionFilter for examples -->
+</suppressions>

codestyle/checkstyle.xml

@@ -0,0 +1,39 @@
+<?xml version="1.0" ?>
+
+<!DOCTYPE module PUBLIC
+    "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
+    "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+
+<!--
+  ~ Licensed to Metamarkets Group Inc. (Metamarkets) under one
+  ~ or more contributor license agreements. See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership. Metamarkets licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License. You may obtain a copy of the License at
+  ~
+  ~ http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied. See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+  -->
+
+<module name="Checker">
+  <module name="NewlineAtEndOfFile"/>
+
+  <module name="TreeWalker">
+    <!-- See http://checkstyle.sourceforge.net/checks.html for examples -->
+
+    <!--<module name="LineLength">-->
+      <!--<property name="max" value="120"/>-->
+    <!--</module>-->
+    <module name="AvoidStarImport"/>
+    <module name="RedundantImport"/>
+    <module name="NeedBraces"/>
+  </module>
+</module>

extensions-contrib/azure-extensions/src/main/java/io/druid/storage/azure/AzureTaskLogs.java

@@ -73,7 +73,9 @@ public Optional<ByteSource> streamTaskLog(final String taskid, final long offset
     final String taskKey = getTaskLogKey(taskid);
 
     try {
-      if (!azureStorage.getBlobExists(container, taskKey)) return Optional.absent();
+      if (!azureStorage.getBlobExists(container, taskKey)) {
+        return Optional.absent();
+      }
 
       return Optional.<ByteSource>of(
           new ByteSource() {

extensions-contrib/druid-rocketmq/src/main/java/io/druid/firehose/rocketmq/RocketMQFirehoseFactory.java

@@ -34,16 +34,23 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.collect.Sets;
+import com.metamx.common.logger.Logger;
 import com.metamx.common.parsers.ParseException;
 import io.druid.data.input.ByteBufferInputRowParser;
 import io.druid.data.input.Firehose;
 import io.druid.data.input.FirehoseFactory;
 import io.druid.data.input.InputRow;
-import com.metamx.common.logger.Logger;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.CountDownLatch;

extensions-contrib/orc-extensions/src/main/java/io/druid/data/input/orc/OrcHadoopInputRowParser.java

@@ -199,16 +199,19 @@ public String apply(@Nullable TypeInfo typeInfo) {
   @Override
   public boolean equals(Object o)
   {
-    if (!(o instanceof OrcHadoopInputRowParser))
+    if (!(o instanceof OrcHadoopInputRowParser)) {
       return false;
+    }
 
     OrcHadoopInputRowParser other = (OrcHadoopInputRowParser)o;
 
-    if (!parseSpec.equals(other.parseSpec))
+    if (!parseSpec.equals(other.parseSpec)) {
       return false;
+    }
 
-    if (!typeString.equals(other.typeString))
+    if (!typeString.equals(other.typeString)) {
       return false;
+    }
 
     return true;
   }

extensions-contrib/orc-extensions/src/test/java/io/druid/data/input/orc/OrcHadoopInputRowParserTest.java

@@ -25,7 +25,13 @@
 import com.google.inject.Injector;
 import com.google.inject.Module;
 import com.google.inject.name.Names;
-import io.druid.data.input.impl.*;
+import io.druid.data.input.impl.DimensionSchema;
+import io.druid.data.input.impl.DimensionsSpec;
+import io.druid.data.input.impl.InputRowParser;
+import io.druid.data.input.impl.ParseSpec;
+import io.druid.data.input.impl.StringDimensionSchema;
+import io.druid.data.input.impl.TimeAndDimsParseSpec;
+import io.druid.data.input.impl.TimestampSpec;
 import io.druid.guice.GuiceInjectors;
 import io.druid.initialization.Initialization;
 import io.druid.jackson.DefaultObjectMapper;

extensions-core/lookups-cached-single/src/main/java/io/druid/server/lookup/LoadingLookupFactory.java

@@ -96,7 +96,9 @@ public boolean replaces(
       @Nullable LookupExtractorFactory lookupExtractorFactory
   )
   {
-    if (lookupExtractorFactory == null) return true;
+    if (lookupExtractorFactory == null) {
+      return true;
+    }
     return !this.equals(lookupExtractorFactory);
   }
 

extensions-core/s3-extensions/src/main/java/io/druid/storage/s3/AWSSessionCredentialsAdapter.java

@@ -27,10 +27,11 @@ public class AWSSessionCredentialsAdapter extends AWSSessionCredentials {
 
   public AWSSessionCredentialsAdapter(AWSCredentialsProvider provider) {
     super(null, null, null);
-    if(provider.getCredentials() instanceof com.amazonaws.auth.AWSSessionCredentials)
+    if(provider.getCredentials() instanceof com.amazonaws.auth.AWSSessionCredentials) {
       this.provider = provider;
-    else
+    } else {
       throw new IllegalArgumentException("provider does not contain session credentials");
+    }
   }
 
   @Override

pom.xml

@@ -690,6 +690,30 @@
                 <artifactId>coveralls-maven-plugin</artifactId>
                 <version>4.0.0</version>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-checkstyle-plugin</artifactId>
+                <version>2.17</version>
+                <configuration>
+                    <sourceDirectory>${project.build.sourceDirectory}</sourceDirectory>
+                    <includeTestSourceDirectory>true</includeTestSourceDirectory>
+                    <configLocation>codestyle/checkstyle.xml</configLocation>
+                    <suppressionsLocation>codestyle/checkstyle-suppressions.xml</suppressionsLocation>
+                    <suppressionsFileExpression>checkstyle.suppressions.file</suppressionsFileExpression>
+                    <encoding>UTF-8</encoding>
+                    <consoleOutput>true</consoleOutput>
+                    <failsOnError>true</failsOnError>
+                </configuration>
+                <executions>
+                    <execution>
+                        <id>validate</id>
+                        <phase>validate</phase>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
         <pluginManagement>
             <plugins>

processing/src/main/java/io/druid/granularity/PeriodGranularity.java

@@ -111,8 +111,11 @@ public long truncate(long t)
         y -= y % years;
         long tt = chronology.years().add(origin, y);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.years().add(tt, -years);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.years().add(tt, -years);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -130,8 +133,11 @@ public long truncate(long t)
         m -= m % months;
         long tt = chronology.months().add(origin, m);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.months().add(tt, -months);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.months().add(tt, -months);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -150,8 +156,11 @@ public long truncate(long t)
         w -= w % weeks;
         long tt = chronology.weeks().add(origin, w);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.weeks().add(tt, -weeks);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.weeks().add(tt, -weeks);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -172,8 +181,11 @@ public long truncate(long t)
         d -= d % days;
         long tt = chronology.days().add(origin, d);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.days().add(tt, -days);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.days().add(tt, -days);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -193,8 +205,11 @@ public long truncate(long t)
         h -= h % hours;
         long tt = chronology.hours().add(origin, h);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.hours().add(tt, -hours);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.hours().add(tt, -hours);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -214,8 +229,11 @@ public long truncate(long t)
         m -=  m % minutes;
         long tt = chronology.minutes().add(origin, m);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.minutes().add(tt, -minutes);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.minutes().add(tt, -minutes);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -235,8 +253,11 @@ public long truncate(long t)
         s -= s % seconds;
         long tt = chronology.seconds().add(origin, s);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.seconds().add(tt, -seconds);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.seconds().add(tt, -seconds);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else
@@ -254,8 +275,11 @@ public long truncate(long t)
         ms -= ms % millis;
         long tt = chronology.millis().add(origin, ms);
         // always round down to the previous period (for timestamps prior to origin)
-        if(t < tt) t = chronology.millis().add(tt, -millis);
-        else t = tt;
+        if(t < tt) {
+          t = chronology.millis().add(tt, -millis);
+        } else {
+          t = tt;
+        }
         return t;
       }
       else {
@@ -274,7 +298,9 @@ private static boolean isCompoundPeriod(Period period)
     {
       if(v > 0)
       {
-        if(single) return true;
+        if(single) {
+          return true;
+        }
         single = true;
       }
     }

processing/src/main/java/io/druid/query/aggregation/DoubleMaxAggregatorFactory.java

@@ -172,13 +172,21 @@ public String toString()
   @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;
+    }
 
     DoubleMaxAggregatorFactory that = (DoubleMaxAggregatorFactory) o;
 
-    if (fieldName != null ? !fieldName.equals(that.fieldName) : that.fieldName != null) return false;
-    if (name != null ? !name.equals(that.name) : that.name != null) return false;
+    if (fieldName != null ? !fieldName.equals(that.fieldName) : that.fieldName != null) {
+      return false;
+    }
+    if (name != null ? !name.equals(that.name) : that.name != null) {
+      return false;
+    }
 
     return true;
   }

processing/src/main/java/io/druid/query/aggregation/DoubleMinAggregatorFactory.java

@@ -172,13 +172,21 @@ public String toString()
   @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;
+    }
 
     DoubleMinAggregatorFactory that = (DoubleMinAggregatorFactory) o;
 
-    if (fieldName != null ? !fieldName.equals(that.fieldName) : that.fieldName != null) return false;
-    if (name != null ? !name.equals(that.name) : that.name != null) return false;
+    if (fieldName != null ? !fieldName.equals(that.fieldName) : that.fieldName != null) {
+      return false;
+    }
+    if (name != null ? !name.equals(that.name) : that.name != null) {
+      return false;
+    }
 
     return true;
   }