Enable most IntelliJ 'Probable bugs' inspections (apache#4353)

* Enable most IntelliJ 'Probable bugs' inspections

* Fix in RemoteTestNG

* Fix IndexSpec's equals() and hashCode() to include longEncoding

* Fix inspection errors

* Extract global isntance of natural().nullsFirst(); address comments

* Fix

* Use noinspection comments instead of SuppressWarnings on method for IntelliJ-specific inspections

* Prohibit Ordering.natural().nullsFirst() using Checkstyle

api/src/main/java/io/druid/data/input/impl/TimestampSpec.java

@@ -47,12 +47,13 @@ private static class ParseCtx
 
   private final String timestampColumn;
   private final String timestampFormat;
-  private final Function<Object, DateTime> timestampConverter;
   // this value should never be set for production data
   private final DateTime missingValue;
+  /** This field is a derivative of {@link #timestampFormat}; not checked in {@link #equals} and {@link #hashCode} */
+  private final Function<Object, DateTime> timestampConverter;
 
   // remember last value parsed
-  private ParseCtx parseCtx = new ParseCtx();
+  private transient ParseCtx parseCtx = new ParseCtx();
 
   @JsonCreator
   public TimestampSpec(
@@ -141,6 +142,16 @@ public int hashCode()
     return result;
   }
 
+  @Override
+  public String toString()
+  {
+    return "TimestampSpec{" +
+           "timestampColumn='" + timestampColumn + '\'' +
+           ", timestampFormat='" + timestampFormat + '\'' +
+           ", missingValue=" + missingValue +
+           '}';
+  }
+
   //simple merge strategy on timestampSpec that checks if all are equal or else
   //returns null. this can be improved in future but is good enough for most use-cases.
   public static TimestampSpec mergeTimestampSpec(List<TimestampSpec> toMerge) {

codestyle/checkstyle.xml

@@ -29,10 +29,18 @@
     <property name="fileExtensions" value="java"/>
   </module>
 
+  <module name="SuppressionCommentFilter">
+    <property name="offCommentFormat" value="CHECKSTYLE.OFF\: ([\w\|]+)"/>
+    <property name="onCommentFormat" value="CHECKSTYLE.ON\: ([\w\|]+)"/>
+    <property name="checkFormat" value="$1"/>
+  </module>
+
   <module name="NewlineAtEndOfFile"/>
   <module name="FileTabCharacter"/>
 
   <module name="TreeWalker">
+    <module name="FileContentsHolder"/>
+
     <!-- See http://checkstyle.sourceforge.net/checks.html for examples -->
 
     <!--<module name="LineLength">-->
@@ -44,19 +52,24 @@
     <module name="UnusedImports" />
     <module name="NeedBraces"/>
     <module name="Regexp">
-      <!-- Prohibit using Guava's Closer, see comment in io.druid.java.util.common.io.Closer -->
       <property name="format" value="com\.google\.common\.io\.Closer"/>
       <property name="illegalPattern" value="true"/>
+      <property name="message" value="Use io.druid.java.util.common.io.Closer instead of Guava's Closer"/>
     </module>
     <module name="Regexp">
-      <!-- Prohibit IntelliJ-style commented code lines from being committed -->
       <property name="format" value="^// {2}"/>
       <property name="illegalPattern" value="true"/>
+      <property name="message" value="Don't commit IntelliJ-style commented code lines"/>
     </module>
     <module name="Regexp">
-      <!-- Force comments to classes and methods to be Javadoc comments -->
       <property name="format" value="/\*[^\*].*?\n(\s*\*.*?\n)*\s+\*/[\s\n]*(transient|volatile|strictfp|synchronized|native|abstract|class|interface|enum|static|private|public|protected|default|void|byte|char|short|int|float|long|double|[A-Z])"/>
       <property name="illegalPattern" value="true"/>
+      <property name="message" value="Comments to classes and methods must be Javadoc comments"/>
+    </module>
+    <module name="Regexp">
+      <property name="format" value="natural\(\)[\s\n]*\.[\s\n]*nullsFirst\(\)"/>
+      <property name="illegalPattern" value="true"/>
+      <property name="message" value="Use Comparators.naturalNullsFirst() instead of Ordering.natural().nullsFirst()"/>
     </module>
   </module>
 </module>

common/src/main/java/io/druid/timeline/partition/IntegerPartitionChunk.java

@@ -19,16 +19,10 @@
 
 package io.druid.timeline.partition;
 
-import com.google.common.collect.Ordering;
-
-import java.util.Comparator;
-
 /**
  */
 public class IntegerPartitionChunk<T> implements PartitionChunk<T>
 {
-  Comparator<Integer> comparator = Ordering.<Integer>natural().nullsFirst();
-
   private final Integer start;
   private final Integer end;
   private final int chunkNumber;
@@ -96,7 +90,7 @@ public int compareTo(PartitionChunk<T> chunk)
   {
     if (chunk instanceof IntegerPartitionChunk) {
       IntegerPartitionChunk<T> intChunk = (IntegerPartitionChunk<T>) chunk;
-      return comparator.compare(chunkNumber, intChunk.chunkNumber);
+      return Integer.compare(chunkNumber, intChunk.chunkNumber);
     } else {
       throw new IllegalArgumentException("Cannot compare against something that is not an IntegerPartitionChunk.");
     }

common/src/main/java/io/druid/timeline/partition/LinearPartitionChunk.java

@@ -19,14 +19,8 @@
 
 package io.druid.timeline.partition;
 
-import com.google.common.collect.Ordering;
-
-import java.util.Comparator;
-
 public class LinearPartitionChunk <T> implements PartitionChunk<T>
 {
-  Comparator<Integer> comparator = Ordering.<Integer>natural().nullsFirst();
-
   private final int chunkNumber;
   private final T object;
 
@@ -81,7 +75,7 @@ public int compareTo(PartitionChunk<T> chunk)
     if (chunk instanceof LinearPartitionChunk) {
       LinearPartitionChunk<T> linearChunk = (LinearPartitionChunk<T>) chunk;
 
-      return comparator.compare(chunkNumber, linearChunk.chunkNumber);
+      return Integer.compare(chunkNumber, linearChunk.chunkNumber);
     }
     throw new IllegalArgumentException("Cannot compare against something that is not a LinearPartitionChunk.");
   }

common/src/main/java/io/druid/timeline/partition/StringPartitionChunk.java

@@ -19,16 +19,10 @@
 
 package io.druid.timeline.partition;
 
-import com.google.common.collect.Ordering;
-
-import java.util.Comparator;
-
 /**
  */
 public class StringPartitionChunk<T> implements PartitionChunk<T>
 {
-  private static final Comparator<Integer> comparator = Ordering.<Integer>natural().nullsFirst();
-
   private final String start;
   private final String end;
   private final int chunkNumber;
@@ -95,7 +89,7 @@ public int compareTo(PartitionChunk<T> chunk)
     if (chunk instanceof StringPartitionChunk) {
       StringPartitionChunk<T> stringChunk = (StringPartitionChunk<T>) chunk;
 
-      return comparator.compare(chunkNumber, stringChunk.chunkNumber);
+      return Integer.compare(chunkNumber, stringChunk.chunkNumber);
     }
     throw new IllegalArgumentException("Cannot compare against something that is not a StringPartitionChunk.");
   }

common/src/test/java/io/druid/common/guava/ComplexSequenceTest.java

@@ -19,15 +19,13 @@
 
 package io.druid.common.guava;
 
-import com.google.common.collect.Ordering;
 import com.google.common.primitives.Ints;
-
+import io.druid.java.util.common.guava.Comparators;
 import io.druid.java.util.common.guava.Sequence;
 import io.druid.java.util.common.guava.Sequences;
 import io.druid.java.util.common.guava.Yielder;
 import io.druid.java.util.common.guava.YieldingAccumulator;
 import io.druid.java.util.common.guava.nary.BinaryFn;
-
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -82,23 +80,14 @@ private Sequence<Integer> simple(int... values)
 
   private Sequence<Integer> combine(Sequence<Integer> sequence)
   {
-    return CombiningSequence.create(sequence, alwaysSame, plus);
+    return CombiningSequence.create(sequence, Comparators.alwaysEqual(), plus);
   }
 
   private Sequence<Integer> concat(Sequence<Integer>... sequences)
   {
     return Sequences.concat(Arrays.asList(sequences));
   }
 
-  private final Ordering<Integer> alwaysSame = new Ordering<Integer>()
-  {
-    @Override
-    public int compare(Integer left, Integer right)
-    {
-      return 0;
-    }
-  };
-
   private final BinaryFn<Integer, Integer, Integer> plus = new BinaryFn<Integer, Integer, Integer>()
   {
     @Override