From 887c6456755bcd8d1bab96b41c9e569f00e9123e Mon Sep 17 00:00:00 2001
From: Roman Leventov <leventov.ru@gmail.com>
Date: Mon, 26 Nov 2018 16:55:42 +0100
Subject: [PATCH] Find duplicate lines with checkstyle; enable some duplicate
 inspections in IntelliJ (#6558)

Not putting this to 0.13 milestone because the found bugs are not critical (one is a harmless DI config duplicate, and another is in a benchmark.

Change in `DumpSegment` is just an indentation change.
---
 .idea/inspectionProfiles/Druid.xml            |  8 +++
 .../benchmark/query/SearchBenchmark.java      |  7 +-
 codestyle/checkstyle-suppressions.xml         |  7 +-
 codestyle/checkstyle.xml                      |  8 +++
 .../jackson/CommaListJoinDeserializer.java    |  3 +-
 .../jackson/CommaListJoinSerializer.java      |  4 +-
 .../druid/indexer/TaskStatusPlusTest.java     |  4 +-
 .../ArrayOfDoublesSketchJsonSerializer.java   |  3 +-
 .../cache/JdbcExtractionNamespaceTest.java    |  7 +-
 .../druid/guice/NullHandlingModule.java       |  1 -
 .../DruidDefaultSerializersModule.java        |  8 +--
 .../org/apache/druid/jackson/JodaStuff.java   |  6 +-
 .../org/apache/druid/cli/DumpSegment.java     | 69 +++++++++----------
 13 files changed, 68 insertions(+), 67 deletions(-)

diff --git a/.idea/inspectionProfiles/Druid.xml b/.idea/inspectionProfiles/Druid.xml
index 97f079fc0fa..6bb556d69ac 100644
--- a/.idea/inspectionProfiles/Druid.xml
+++ b/.idea/inspectionProfiles/Druid.xml
@@ -29,6 +29,11 @@
     <inspection_tool class="Contract" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="CopyConstructorMissesField" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="CovariantEquals" enabled="true" level="ERROR" enabled_by_default="true" />
+    <inspection_tool class="DuplicateBooleanBranch" enabled="true" level="ERROR" enabled_by_default="true" />
+    <inspection_tool class="DuplicateCondition" enabled="true" level="ERROR" enabled_by_default="true">
+      <option name="ignoreSideEffectConditions" value="true" />
+    </inspection_tool>
+    <inspection_tool class="DuplicateThrows" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="EmptyInitializer" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="EmptyStatementBody" enabled="true" level="WARNING" enabled_by_default="true">
       <option name="m_reportEmptyBlocks" value="true" />
@@ -71,6 +76,7 @@
     <inspection_tool class="InvalidComparatorMethodReference" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="IteratorHasNextCallsIteratorNext" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="IteratorNextDoesNotThrowNoSuchElementException" enabled="true" level="WARNING" enabled_by_default="true" />
+    <inspection_tool class="JsonDuplicatePropertyKeys" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="JsonStandardCompliance" enabled="true" level="WARNING" enabled_by_default="true" />
     <inspection_tool class="LengthOneStringInIndexOf" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ListIndexOfReplaceableByContains" enabled="true" level="ERROR" enabled_by_default="true" />
@@ -80,6 +86,8 @@
     </inspection_tool>
     <inspection_tool class="MalformedRegex" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="MathRandomCastToInt" enabled="true" level="ERROR" enabled_by_default="true" />
+    <inspection_tool class="MavenDuplicateDependenciesInspection" enabled="true" level="ERROR" enabled_by_default="true" />
+    <inspection_tool class="MavenDuplicatePluginInspection" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="MavenModelInspection" enabled="true" level="WARNING" enabled_by_default="true" />
     <inspection_tool class="MismatchedArrayReadWrite" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="MismatchedCollectionQueryUpdate" enabled="true" level="ERROR" enabled_by_default="true">
diff --git a/benchmarks/src/main/java/org/apache/druid/benchmark/query/SearchBenchmark.java b/benchmarks/src/main/java/org/apache/druid/benchmark/query/SearchBenchmark.java
index 3b2f6679c9d..c7008fcf556 100644
--- a/benchmarks/src/main/java/org/apache/druid/benchmark/query/SearchBenchmark.java
+++ b/benchmarks/src/main/java/org/apache/druid/benchmark/query/SearchBenchmark.java
@@ -282,7 +282,9 @@ public class SearchBenchmark
 
   private static SearchQueryBuilder basicD(final BenchmarkSchemaInfo basicSchema)
   {
-    final QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(Collections.singletonList(basicSchema.getDataInterval()));
+    final QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(
+        Collections.singletonList(basicSchema.getDataInterval())
+    );
 
     final List<String> dimUniformFilterVals = new ArrayList<>();
     final int resultNum = (int) (100000 * 0.1);
@@ -296,9 +298,6 @@ public class SearchBenchmark
     dimFilters.add(new InDimFilter(dimName, dimUniformFilterVals, null));
     dimFilters.add(new SelectorDimFilter(dimName, "3", null));
     dimFilters.add(new BoundDimFilter(dimName, "100", "10000", true, true, true, null, null));
-    dimFilters.add(new InDimFilter(dimName, dimUniformFilterVals, null));
-    dimFilters.add(new InDimFilter(dimName, dimUniformFilterVals, null));
-    dimFilters.add(new InDimFilter(dimName, dimUniformFilterVals, null));
 
     return Druids.newSearchQueryBuilder()
                  .dataSource("blah")
diff --git a/codestyle/checkstyle-suppressions.xml b/codestyle/checkstyle-suppressions.xml
index bf5087c9b00..0c8300aedc6 100644
--- a/codestyle/checkstyle-suppressions.xml
+++ b/codestyle/checkstyle-suppressions.xml
@@ -50,13 +50,16 @@
 
   <suppress checks="Indentation" files="[\\/]target[\\/]generated-test-sources[\\/]" />
   <suppress checks="Indentation" files="ProtoTestEventWrapper.java" />
-  <suppress checks="Regex" files="ProtoTestEventWrapper.java" />
+  <suppress checks="Regexp" id="argumentLineBreaking" files="ProtoTestEventWrapper.java" />
 
   <suppress checks="OneStatementPerLine" files="[\\/]target[\\/]generated-test-sources[\\/]" />
 
-  <!-- extendedset is a fork of Alessandro Colantonio's CONCISE (COmpressed 'N' Composable Integer SEt) repository and licensed to ASF under a CLA is not true. -->
+  <!-- extendedset is a fork of Alessandro Colantonio's CONCISE (COmpressed 'N' Composable Integer SEt) repository
+       and licensed to ASF under a CLA is not true. -->
   <suppress checks="Header" files="[\\/]extendedset[\\/]" />
 
+  <suppress checks="Regexp" id="duplicateLine" files="[\\/]src[\\/]test[\\/]" />
+
   <!-- See https://github.com/checkstyle/checkstyle/issues/5510 and the ImportOrder definition in checkstyle.xml -->
   <suppress checks="ImportOrder" message="^'java\..*'.*" />
 
diff --git a/codestyle/checkstyle.xml b/codestyle/checkstyle.xml
index 16e8c14c8c7..d6ef3e8f997 100644
--- a/codestyle/checkstyle.xml
+++ b/codestyle/checkstyle.xml
@@ -221,6 +221,7 @@
     </module>
 
     <module name="Regexp">
+      <property name="id" value="argumentLineBreaking"/>
       <property
           name="format"
           value='(?&lt;!ImmutableMap.of|Types.mapOf|orderedMap|makeSelectResults|makeListOfPairs)\(\s*\n +([^,\n\(\{"&lt;/]+|[^,\n\(\{" /]+&gt; [a-zA-Z0-9_]+)\, ++[^,\n/]+'
@@ -277,5 +278,12 @@ If you encouter a map-like or a pair-accepting method that is reported by this&#
 checkstyle rule, you should add it as an exception in the corresponding rule in&#10;
 codestyle/checkstyle.xml.&#10;"/>
     </module>
+
+    <module name="Regexp">
+      <property name="id" value="duplicateLine"/>
+      <property name="format" value="^(.*;)(\r?\n\1)+$"/>
+      <property name="illegalPattern" value="true"/>
+      <property name="message" value="Duplicate line"/>
+    </module>
   </module>
 </module>
diff --git a/core/src/main/java/org/apache/druid/jackson/CommaListJoinDeserializer.java b/core/src/main/java/org/apache/druid/jackson/CommaListJoinDeserializer.java
index bda1f36fe43..b9910e61540 100644
--- a/core/src/main/java/org/apache/druid/jackson/CommaListJoinDeserializer.java
+++ b/core/src/main/java/org/apache/druid/jackson/CommaListJoinDeserializer.java
@@ -20,7 +20,6 @@
 package org.apache.druid.jackson;
 
 import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer;
 
@@ -39,7 +38,7 @@ public class CommaListJoinDeserializer extends StdScalarDeserializer<List<String
 
   @Override
   public List<String> deserialize(JsonParser jsonParser, DeserializationContext deserializationContext)
-      throws IOException, JsonProcessingException
+      throws IOException
   {
     return Arrays.asList(jsonParser.getText().split(","));
   }
diff --git a/core/src/main/java/org/apache/druid/jackson/CommaListJoinSerializer.java b/core/src/main/java/org/apache/druid/jackson/CommaListJoinSerializer.java
index 8d62da41d37..05c733ceb56 100644
--- a/core/src/main/java/org/apache/druid/jackson/CommaListJoinSerializer.java
+++ b/core/src/main/java/org/apache/druid/jackson/CommaListJoinSerializer.java
@@ -19,7 +19,6 @@
 
 package org.apache.druid.jackson;
 
-import com.fasterxml.jackson.core.JsonGenerationException;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.ser.std.StdScalarSerializer;
@@ -40,8 +39,7 @@ public class CommaListJoinSerializer extends StdScalarSerializer<List<String>>
   }
 
   @Override
-  public void serialize(List<String> value, JsonGenerator jgen, SerializerProvider provider)
-      throws IOException, JsonGenerationException
+  public void serialize(List<String> value, JsonGenerator jgen, SerializerProvider provider) throws IOException
   {
     jgen.writeString(joiner.join(value));
   }
diff --git a/core/src/test/java/org/apache/druid/indexer/TaskStatusPlusTest.java b/core/src/test/java/org/apache/druid/indexer/TaskStatusPlusTest.java
index f99ea0acbd5..9d4817226bd 100644
--- a/core/src/test/java/org/apache/druid/indexer/TaskStatusPlusTest.java
+++ b/core/src/test/java/org/apache/druid/indexer/TaskStatusPlusTest.java
@@ -20,7 +20,6 @@
 package org.apache.druid.indexer;
 
 import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.JsonToken;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -109,8 +108,7 @@ public class TaskStatusPlusTest
     }
 
     @Override
-    public DateTime deserialize(JsonParser jp, DeserializationContext ctxt)
-        throws IOException, JsonProcessingException
+    public DateTime deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException
     {
       JsonToken t = jp.getCurrentToken();
       if (t == JsonToken.VALUE_NUMBER_INT) {
diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchJsonSerializer.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchJsonSerializer.java
index b1637068b92..841ef78d9c0 100644
--- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchJsonSerializer.java
+++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchJsonSerializer.java
@@ -20,7 +20,6 @@
 package org.apache.druid.query.aggregation.datasketches.tuple;
 
 import com.fasterxml.jackson.core.JsonGenerator;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonSerializer;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.yahoo.sketches.tuple.ArrayOfDoublesSketch;
@@ -35,7 +34,7 @@ public class ArrayOfDoublesSketchJsonSerializer extends JsonSerializer<ArrayOfDo
       final ArrayOfDoublesSketch sketch,
       final JsonGenerator generator,
       final SerializerProvider provider
-  ) throws IOException, JsonProcessingException
+  ) throws IOException
   {
     generator.writeBinary(sketch.toByteArray());
   }
diff --git a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java
index fd89b7bac68..deef14e70fd 100644
--- a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java
+++ b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java
@@ -276,12 +276,7 @@ public class JdbcExtractionNamespaceTest
         }
     );
 
-    Closeable closeable = () -> {
-      if (!setupFuture.isDone() && !setupFuture.cancel(true) && !setupFuture.isDone()) {
-        throw new IOException("Unable to stop future");
-      }
-    };
-    try (final Closeable c = closeable) {
+    try (final Closeable ignore = () -> setupFuture.cancel(true)) {
       handleRef = setupFuture.get(10, TimeUnit.SECONDS);
     }
     Assert.assertNotNull(handleRef);
diff --git a/processing/src/main/java/org/apache/druid/guice/NullHandlingModule.java b/processing/src/main/java/org/apache/druid/guice/NullHandlingModule.java
index 99819805e4c..b8be2e851a8 100644
--- a/processing/src/main/java/org/apache/druid/guice/NullHandlingModule.java
+++ b/processing/src/main/java/org/apache/druid/guice/NullHandlingModule.java
@@ -33,6 +33,5 @@ public class NullHandlingModule implements Module
   {
     JsonConfigProvider.bind(binder, "druid.generic", NullValueHandlingConfig.class);
     binder.requestStaticInjection(NullHandling.class);
-    binder.requestStaticInjection(NullHandling.class);
   }
 }
diff --git a/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java b/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java
index 55a39378eef..26de3ecb153 100644
--- a/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java
+++ b/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java
@@ -21,7 +21,6 @@ package org.apache.druid.jackson;
 
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonDeserializer;
 import com.fasterxml.jackson.databind.JsonSerializer;
@@ -70,8 +69,7 @@ public class DruidDefaultSerializersModule extends SimpleModule
               DateTimeZone dateTimeZone,
               JsonGenerator jsonGenerator,
               SerializerProvider serializerProvider
-          )
-              throws IOException, JsonProcessingException
+          ) throws IOException
           {
             jsonGenerator.writeString(dateTimeZone.getID());
           }
@@ -83,7 +81,7 @@ public class DruidDefaultSerializersModule extends SimpleModule
         {
           @Override
           public void serialize(Sequence value, final JsonGenerator jgen, SerializerProvider provider)
-              throws IOException, JsonProcessingException
+              throws IOException
           {
             jgen.writeStartArray();
             value.accumulate(
@@ -113,7 +111,7 @@ public class DruidDefaultSerializersModule extends SimpleModule
         {
           @Override
           public void serialize(Yielder yielder, final JsonGenerator jgen, SerializerProvider provider)
-              throws IOException, JsonProcessingException
+              throws IOException
           {
             try {
               jgen.writeStartArray();
diff --git a/processing/src/main/java/org/apache/druid/jackson/JodaStuff.java b/processing/src/main/java/org/apache/druid/jackson/JodaStuff.java
index b7af383c915..1830ef99b84 100644
--- a/processing/src/main/java/org/apache/druid/jackson/JodaStuff.java
+++ b/processing/src/main/java/org/apache/druid/jackson/JodaStuff.java
@@ -20,7 +20,6 @@
 package org.apache.druid.jackson;
 
 import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.JsonToken;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonDeserializer;
@@ -71,7 +70,7 @@ class JodaStuff
 
     @Override
     public Interval deserialize(JsonParser jsonParser, DeserializationContext deserializationContext)
-        throws IOException, JsonProcessingException
+        throws IOException
     {
       return Intervals.of(jsonParser.getText());
     }
@@ -94,8 +93,7 @@ class JodaStuff
     }
 
     @Override
-    public DateTime deserialize(JsonParser jp, DeserializationContext ctxt)
-        throws IOException, JsonProcessingException
+    public DateTime deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException
     {
       JsonToken t = jp.getCurrentToken();
       if (t == JsonToken.VALUE_NUMBER_INT) {
diff --git a/services/src/main/java/org/apache/druid/cli/DumpSegment.java b/services/src/main/java/org/apache/druid/cli/DumpSegment.java
index 06f90adef4e..66ebd542d2d 100644
--- a/services/src/main/java/org/apache/druid/cli/DumpSegment.java
+++ b/services/src/main/java/org/apache/druid/cli/DumpSegment.java
@@ -343,50 +343,49 @@ public class DumpSegment extends GuiceRunnable
           @Override
           public Object apply(final OutputStream out)
           {
-            try {
-              final JsonGenerator jg = objectMapper.getFactory().createGenerator(out);
-
-              jg.writeStartObject();
-              jg.writeObjectField("bitmapSerdeFactory", bitmapSerdeFactory);
-              jg.writeFieldName("bitmaps");
+            try (final JsonGenerator jg = objectMapper.getFactory().createGenerator(out)) {
               jg.writeStartObject();
+              {
+                jg.writeObjectField("bitmapSerdeFactory", bitmapSerdeFactory);
+                jg.writeFieldName("bitmaps");
+                jg.writeStartObject();
+                {
+                  for (final String columnName : columnNames) {
+                    final ColumnHolder columnHolder = index.getColumnHolder(columnName);
+                    final BitmapIndex bitmapIndex = columnHolder.getBitmapIndex();
 
-              for (final String columnName : columnNames) {
-                final ColumnHolder columnHolder = index.getColumnHolder(columnName);
-                final BitmapIndex bitmapIndex = columnHolder.getBitmapIndex();
-
-                if (bitmapIndex == null) {
-                  jg.writeNullField(columnName);
-                } else {
-                  jg.writeFieldName(columnName);
-                  jg.writeStartObject();
-                  for (int i = 0; i < bitmapIndex.getCardinality(); i++) {
-                    String val = NullHandling.nullToEmptyIfNeeded(bitmapIndex.getValue(i));
-                    if (val != null) {
-                      final ImmutableBitmap bitmap = bitmapIndex.getBitmap(i);
-                      if (decompressBitmaps) {
-                        jg.writeStartArray();
-                        final IntIterator iterator = bitmap.iterator();
-                        while (iterator.hasNext()) {
-                          final int rowNum = iterator.next();
-                          jg.writeNumber(rowNum);
-                        }
-                        jg.writeEndArray();
-                      } else {
-                        byte[] bytes = bitmapSerdeFactory.getObjectStrategy().toBytes(bitmap);
-                        if (bytes != null) {
-                          jg.writeBinary(bytes);
+                    if (bitmapIndex == null) {
+                      jg.writeNullField(columnName);
+                    } else {
+                      jg.writeFieldName(columnName);
+                      jg.writeStartObject();
+                      for (int i = 0; i < bitmapIndex.getCardinality(); i++) {
+                        String val = NullHandling.nullToEmptyIfNeeded(bitmapIndex.getValue(i));
+                        if (val != null) {
+                          final ImmutableBitmap bitmap = bitmapIndex.getBitmap(i);
+                          if (decompressBitmaps) {
+                            jg.writeStartArray();
+                            final IntIterator iterator = bitmap.iterator();
+                            while (iterator.hasNext()) {
+                              final int rowNum = iterator.next();
+                              jg.writeNumber(rowNum);
+                            }
+                            jg.writeEndArray();
+                          } else {
+                            byte[] bytes = bitmapSerdeFactory.getObjectStrategy().toBytes(bitmap);
+                            if (bytes != null) {
+                              jg.writeBinary(bytes);
+                            }
+                          }
                         }
                       }
+                      jg.writeEndObject();
                     }
                   }
-                  jg.writeEndObject();
                 }
+                jg.writeEndObject();
               }
-
               jg.writeEndObject();
-              jg.writeEndObject();
-              jg.close();
             }
             catch (IOException e) {
               throw Throwables.propagate(e);