This is an automated email from the ASF dual-hosted git repository. leventov pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-druid.git
The following commit(s) were added to refs/heads/master by this push: new 887c645 Find duplicate lines with checkstyle; enable some duplicate inspections in IntelliJ (#6558) 887c645 is described below commit 887c6456755bcd8d1bab96b41c9e569f00e9123e Author: Roman Leventov <leventov...@gmail.com> AuthorDate: Mon Nov 26 16:55:42 2018 +0100 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 +++ .../druid/benchmark/query/SearchBenchmark.java | 7 +-- codestyle/checkstyle-suppressions.xml | 7 ++- codestyle/checkstyle.xml | 8 +++ .../druid/jackson/CommaListJoinDeserializer.java | 3 +- .../druid/jackson/CommaListJoinSerializer.java | 4 +- .../apache/druid/indexer/TaskStatusPlusTest.java | 4 +- .../tuple/ArrayOfDoublesSketchJsonSerializer.java | 3 +- .../cache/JdbcExtractionNamespaceTest.java | 7 +-- .../org/apache/druid/guice/NullHandlingModule.java | 1 - .../jackson/DruidDefaultSerializersModule.java | 8 +-- .../java/org/apache/druid/jackson/JodaStuff.java | 6 +- .../java/org/apache/druid/cli/DumpSegment.java | 71 +++++++++++----------- 13 files changed, 69 insertions(+), 68 deletions(-) diff --git a/.idea/inspectionProfiles/Druid.xml b/.idea/inspectionProfiles/Druid.xml index 97f079f..6bb556d 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 3b2f667..c7008fc 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 bf5087c..0c8300a 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 16e8c14..d6ef3e8 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='(?<!ImmutableMap.of|Types.mapOf|orderedMap|makeSelectResults|makeListOfPairs)\(\s*\n +([^,\n\(\{"</]+|[^,\n\(\{" /]+> [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 codestyle/checkstyle.xml. "/> </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 bda1f36..b9910e6 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 8d62da4..05c733c 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 f99ea0a..9d48172 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 b163706..841ef78 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 fd89b7b..deef14e 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 9981980..b8be2e8 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 55a3937..26de3ec 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 b7af383..1830ef9 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 06f90ad..66ebd54 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(); - - 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); + { + 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(); + + 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); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org