Skip to content

Commit b1308fe

Browse files
committed
[CALCITE-6552] Enable CheckerFramework in 'server' module
In [CALCITE-4199] we enabled CheckerFramework (via annotations and CI jobs) in the 'core' and 'linq4j' modules; this change further enables CheckerFramework in the 'server' module, and fixes all violations. There are also a large number of 'cosmetic' modifications to improve code quality without changing behavior, including: * Replace `this.x = x; assert x != null;` with `this.x = requireNonNull(x);` in constructors * Replace `assert` in other code locations where it implements an invariant. We don't use `requireNonNull` because it throws `NullPointerException`; we would prefer to throw `AssertionError` or `IllegalStateException` * Replace `x.equals("")` and `x.size() == 0` with `x.isEmpty()` * Make fields `final` where possible * Make private methods and inner classes `static` where possible * In class `Pair<K, V>` make the type variables `K` and `V` no longer nullable by default (you can make each of them nullable if you need) * Always import `Objects.requireNonNull`, `Integer.parseInt`, `Float.parseFloat`, `Double.parseDouble`, `Byte.parseByte`, `Short.parseShort`, `Boolean.parseBoolean`, `Long.parseLong` via static import, and change autostyle rules to enforce this automatically * Remove redundant 'throws' clauses * Add `@Nullable` annotations for method parameters and return values so that types are nullable only if they need to be; the goal is that you should not pass null or a nullable value as an argument that has non-nullable type, and after this change we are nearer to that goal In Util, optimize methods `transform` and `transformIndexed` for the case where list is empty and immutable. Close #3939
1 parent 8771e3f commit b1308fe

File tree

606 files changed

+4276
-3707
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

606 files changed

+4276
-3707
lines changed

Diff for: .github/workflows/main.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -377,9 +377,9 @@ jobs:
377377
with:
378378
job-id: checkerframework-jdk11
379379
remote-build-cache-proxy-enabled: false
380-
arguments: --scan --no-parallel --no-daemon -PenableCheckerframework :linq4j:classes :core:classes
380+
arguments: --scan --no-parallel --no-daemon -PenableCheckerframework :linq4j:classes :core:classes :server:classes
381381

382-
linux-checkerframework-guava29:
382+
linux-checkerframework-oldest-guava:
383383
name: 'CheckerFramework (JDK 11), oldest Guava'
384384
runs-on: ubuntu-latest
385385
env:
@@ -398,7 +398,7 @@ jobs:
398398
with:
399399
job-id: checkerframework-jdk11
400400
remote-build-cache-proxy-enabled: false
401-
arguments: --scan --no-parallel --no-daemon -Pguava.version=${{ env.GUAVA }} -PenableCheckerframework :linq4j:classes :core:classes
401+
arguments: --scan --no-parallel --no-daemon -Pguava.version=${{ env.GUAVA }} -PenableCheckerframework :linq4j:classes :core:classes :server:classes
402402

403403
linux-slow:
404404
# Run slow tests when the commit is on main or it is requested explicitly by adding an

Diff for: arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowFilterEnumerator.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,20 @@
2929
import org.apache.arrow.vector.ipc.ArrowFileReader;
3030
import org.apache.arrow.vector.ipc.message.ArrowRecordBatch;
3131

32+
import org.checkerframework.checker.nullness.qual.Nullable;
33+
3234
import java.io.IOException;
3335

36+
import static java.util.Objects.requireNonNull;
37+
3438
/**
3539
* Enumerator that reads from a filtered collection of Arrow value-vectors.
3640
*/
3741
class ArrowFilterEnumerator extends AbstractArrowEnumerator {
3842
private final BufferAllocator allocator;
3943
private final Filter filter;
40-
private ArrowBuf buf;
41-
private SelectionVector selectionVector;
44+
private @Nullable ArrowBuf buf;
45+
private @Nullable SelectionVector selectionVector;
4246
private int selectionVectorIndex;
4347

4448
ArrowFilterEnumerator(ArrowFileReader arrowFileReader, ImmutableIntList fields, Filter filter) {
@@ -71,7 +75,7 @@ class ArrowFilterEnumerator extends AbstractArrowEnumerator {
7175
selectionVectorIndex = 0;
7276
this.valueVectors.clear();
7377
loadNextArrowBatch();
74-
assert selectionVector != null;
78+
requireNonNull(selectionVector, "selectionVector");
7579
if (selectionVectorIndex >= selectionVector.getRecordCount()) {
7680
// the "filtered" batch is empty, but there may be more batches to fetch
7781
continue;

Diff for: arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTable.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@
5555
import java.util.ArrayList;
5656
import java.util.List;
5757

58+
import static java.lang.Double.parseDouble;
59+
import static java.lang.Float.parseFloat;
60+
import static java.lang.Integer.parseInt;
61+
import static java.lang.Long.parseLong;
5862
import static java.util.Objects.requireNonNull;
5963

6064
/**
@@ -183,13 +187,13 @@ private static RelDataType deduceRowType(Schema schema,
183187
private static TreeNode makeLiteralNode(String literal, String type) {
184188
switch (type) {
185189
case "integer":
186-
return TreeBuilder.makeLiteral(Integer.parseInt(literal));
190+
return TreeBuilder.makeLiteral(parseInt(literal));
187191
case "long":
188-
return TreeBuilder.makeLiteral(Long.parseLong(literal));
192+
return TreeBuilder.makeLiteral(parseLong(literal));
189193
case "float":
190-
return TreeBuilder.makeLiteral(Float.parseFloat(literal));
194+
return TreeBuilder.makeLiteral(parseFloat(literal));
191195
case "double":
192-
return TreeBuilder.makeLiteral(Double.parseDouble(literal));
196+
return TreeBuilder.makeLiteral(parseDouble(literal));
193197
case "string":
194198
return TreeBuilder.makeStringLiteral(literal.substring(1, literal.length() - 1));
195199
default:

Diff for: arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowAdapterTest.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@
4343
import java.nio.file.Path;
4444
import java.sql.SQLException;
4545
import java.util.Map;
46-
import java.util.Objects;
4746

4847
import static org.junit.jupiter.api.Assertions.assertEquals;
4948

49+
import static java.util.Objects.requireNonNull;
50+
5051
/**
5152
* Tests for the Apache Arrow adapter.
5253
*/
@@ -57,9 +58,11 @@ class ArrowAdapterTest {
5758
private static File arrowDataDirectory;
5859

5960
@BeforeAll
60-
static void initializeArrowState(@TempDir Path sharedTempDir) throws IOException, SQLException {
61+
static void initializeArrowState(@TempDir Path sharedTempDir)
62+
throws IOException, SQLException {
6163
URL modelUrl =
62-
Objects.requireNonNull(ArrowAdapterTest.class.getResource("/arrow-model.json"), "url");
64+
requireNonNull(
65+
ArrowAdapterTest.class.getResource("/arrow-model.json"), "url");
6366
Path sourceModelFilePath = Sources.of(modelUrl).file().toPath();
6467
Path modelFileTarget = sharedTempDir.resolve("arrow-model.json");
6568
Files.copy(sourceModelFilePath, modelFileTarget);

Diff for: babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import net.hydromatic.quidem.CommandHandler;
4141
import net.hydromatic.quidem.Quidem;
4242

43+
import org.checkerframework.checker.nullness.qual.Nullable;
4344
import org.junit.jupiter.api.AfterEach;
4445
import org.junit.jupiter.api.BeforeEach;
4546

@@ -167,13 +168,11 @@ public static void main(String[] args) throws Exception {
167168
static class ExplainValidatedCommand extends AbstractCommand {
168169
private final ImmutableList<String> lines;
169170
private final ImmutableList<String> content;
170-
private final Set<String> productSet;
171171

172172
ExplainValidatedCommand(List<String> lines, List<String> content,
173-
Set<String> productSet) {
173+
Set<String> unusedProductSet) {
174174
this.lines = ImmutableList.copyOf(lines);
175175
this.content = ImmutableList.copyOf(content);
176-
this.productSet = ImmutableSet.copyOf(productSet);
177176
}
178177

179178
@Override public void execute(Context x, boolean execute) throws Exception {
@@ -214,7 +213,7 @@ static class ExplainValidatedCommand extends AbstractCommand {
214213
/** Command handler that adds a "!explain-validated-on dialect..." command
215214
* (see {@link ExplainValidatedCommand}). */
216215
private static class BabelCommandHandler implements CommandHandler {
217-
@Override public Command parseCommand(List<String> lines,
216+
@Override public @Nullable Command parseCommand(List<String> lines,
218217
List<String> content, String line) {
219218
final String prefix = "explain-validated-on";
220219
if (line.startsWith(prefix)) {

Diff for: build.gradle.kts

+11
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,14 @@ allprojects {
666666
replace("hamcrest: sameInstance", "org.hamcrest.core.IsSame.sameInstance", "org.hamcrest.CoreMatchers.sameInstance")
667667
replace("hamcrest: startsWith", "org.hamcrest.core.StringStartsWith.startsWith", "org.hamcrest.CoreMatchers.startsWith")
668668
replaceRegex("hamcrest: size", "\\.size\\(\\), (is|equalTo)\\(", ", hasSize\\(")
669+
replaceRegex("use static import: parseBoolean", "Boolean\\.(parseBoolean\\()", "$1")
670+
replaceRegex("use static import: parseByte", "Byte\\.(parseByte\\()", "$1")
671+
replaceRegex("use static import: parseDouble", "Double\\.(parseDouble\\()", "$1")
672+
replaceRegex("use static import: parseFloat", "Float\\.(parseFloat\\()", "$1")
673+
replaceRegex("use static import: parseInt", "Integer\\.(parseInt\\()", "$1")
674+
replaceRegex("use static import: parseLong", "Long\\.(parseLong\\()", "$1")
675+
replaceRegex("use static import: parseLong", "Short\\.(parseShort\\()", "$1")
676+
replaceRegex("use static import: requireNonNull", "Objects\\.(requireNonNull\\()", "$1")
669677
replaceRegex("use static import: toImmutableList", "ImmutableList\\.(toImmutableList\\(\\))", "$1")
670678
replaceRegex("use static import: checkArgument", "Preconditions\\.(checkArgument\\()", "$1")
671679
replaceRegex("use static import: checkArgument", "Preconditions\\.(checkState\\()", "$1")
@@ -778,6 +786,9 @@ allprojects {
778786
if (project.path == ":core") {
779787
extraJavacArgs.add("-AskipDefs=^org\\.apache\\.calcite\\.sql\\.parser\\.impl\\.")
780788
}
789+
if (project.path == ":server") {
790+
extraJavacArgs.add("-AskipDefs=^org\\.apache\\.calcite\\.sql\\.parser\\.ddl\\.")
791+
}
781792
}
782793
}
783794

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraEnumerator.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import java.util.Objects;
4343
import java.util.stream.IntStream;
4444

45+
import static java.util.Objects.requireNonNull;
46+
4547
/** Enumerator that reads from a Cassandra column family. */
4648
class CassandraEnumerator implements Enumerator<Object> {
4749
private final Iterator<Row> iterator;
@@ -86,7 +88,7 @@ class CassandraEnumerator implements Enumerator<Object> {
8688
* @param index Index of the field within the Row object
8789
*/
8890
private @Nullable Object currentRowField(int index) {
89-
assert current != null;
91+
requireNonNull(current, "current");
9092
final Object o =
9193
current.get(index,
9294
CodecRegistry.DEFAULT.codecFor(

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,13 @@
4646
import java.util.Collections;
4747
import java.util.HashSet;
4848
import java.util.List;
49-
import java.util.Objects;
5049
import java.util.Set;
5150

5251
import static org.apache.calcite.util.DateTimeStringUtils.ISO_DATETIME_FRACTIONAL_SECOND_FORMAT;
5352
import static org.apache.calcite.util.DateTimeStringUtils.getDateFormatter;
5453

54+
import static java.util.Objects.requireNonNull;
55+
5556
/**
5657
* Implementation of a {@link org.apache.calcite.rel.core.Filter}
5758
* relational expression in Cassandra.
@@ -92,7 +93,8 @@ public CassandraFilter(
9293

9394
@Override public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner,
9495
RelMetadataQuery mq) {
95-
return super.computeSelfCost(planner, mq).multiplyBy(0.1);
96+
final RelOptCost cost = requireNonNull(super.computeSelfCost(planner, mq));
97+
return cost.multiplyBy(0.1);
9698
}
9799

98100
@Override public CassandraFilter copy(RelTraitSet traitSet, RelNode input,
@@ -297,7 +299,7 @@ private String translateOp2(String op, String name, RexLiteral right) {
297299
String valueString = value.toString();
298300
if (value instanceof String) {
299301
RelDataTypeField field =
300-
Objects.requireNonNull(rowType.getField(name, true, false));
302+
requireNonNull(rowType.getField(name, true, false));
301303
SqlTypeName typeName = field.getType().getSqlTypeName();
302304
if (typeName != SqlTypeName.CHAR) {
303305
valueString = "'" + valueString + "'";

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraProject.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import java.util.List;
3737
import java.util.Map;
3838

39+
import static java.util.Objects.requireNonNull;
40+
3941
/**
4042
* Implementation of {@link org.apache.calcite.rel.core.Project}
4143
* relational expression in Cassandra.
@@ -56,7 +58,8 @@ public CassandraProject(RelOptCluster cluster, RelTraitSet traitSet,
5658

5759
@Override public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner,
5860
RelMetadataQuery mq) {
59-
return super.computeSelfCost(planner, mq).multiplyBy(0.1);
61+
final RelOptCost cost = super.computeSelfCost(planner, mq);
62+
return requireNonNull(cost, "cost").multiplyBy(0.1);
6063
}
6164

6265
@Override public void implement(Implementor implementor) {
@@ -66,9 +69,9 @@ public CassandraProject(RelOptCluster cluster, RelTraitSet traitSet,
6669
CassandraRules.cassandraFieldNames(getInput().getRowType()));
6770
final Map<String, String> fields = new LinkedHashMap<>();
6871
for (Pair<RexNode, String> pair : getNamedProjects()) {
69-
assert pair.left != null;
72+
final RexNode node = pair.left;
7073
final String name = pair.right;
71-
final String originalName = pair.left.accept(translator);
74+
final String originalName = node.accept(translator);
7275
fields.put(originalName, name);
7376
}
7477
implementor.add(fields, null);

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@
6262
import java.util.Collection;
6363
import java.util.List;
6464
import java.util.Map;
65-
import java.util.Objects;
6665
import java.util.Optional;
6766
import java.util.stream.Collectors;
6867
import java.util.stream.IntStream;
6968

69+
import static java.util.Objects.requireNonNull;
70+
7071
/**
7172
* Schema mapped onto a Cassandra column family.
7273
*/
@@ -297,7 +298,7 @@ private void addMaterializedViews() {
297298
+ "WHERE keyspace_name='" + keyspace + "' AND view_name='"
298299
+ view.getName().asInternal() + "'";
299300

300-
Row whereClauseRow = Objects.requireNonNull(session.execute(whereQuery).one());
301+
Row whereClauseRow = requireNonNull(session.execute(whereQuery).one());
301302

302303
queryBuilder.append(" WHERE ")
303304
.append(whereClauseRow.getString(0));

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchemaFactory.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
import java.util.concurrent.ConcurrentHashMap;
3535
import java.util.stream.Collectors;
3636

37+
import static java.lang.Integer.parseInt;
38+
3739
/**
3840
* Factory that creates a {@link CassandraSchema}.
3941
*/
@@ -107,7 +109,7 @@ private static int getPort(Map<String, Object> map) {
107109
if (map.containsKey("port")) {
108110
Object portObj = map.get("port");
109111
if (portObj instanceof String) {
110-
return Integer.parseInt((String) portObj);
112+
return parseInt((String) portObj);
111113
} else {
112114
return (int) portObj;
113115
}

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSort.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import java.util.ArrayList;
3434
import java.util.List;
3535

36+
import static java.util.Objects.requireNonNull;
37+
3638
/**
3739
* Implementation of {@link org.apache.calcite.rel.core.Sort}
3840
* relational expression in Cassandra.
@@ -48,7 +50,7 @@ public CassandraSort(RelOptCluster cluster, RelTraitSet traitSet,
4850

4951
@Override public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner,
5052
RelMetadataQuery mq) {
51-
RelOptCost cost = super.computeSelfCost(planner, mq);
53+
RelOptCost cost = requireNonNull(super.computeSelfCost(planner, mq));
5254
if (!collation.getFieldCollations().isEmpty()) {
5355
return cost.multiplyBy(0.05);
5456
} else {

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraTable.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@
5757
public class CassandraTable extends AbstractQueryableTable
5858
implements TranslatableTable {
5959
final RelProtoDataType protoRowType;
60-
List<String> partitionKeys;
61-
List<String> clusteringKeys;
62-
List<RelFieldCollation> clusteringOrder;
60+
final List<String> partitionKeys;
61+
final List<String> clusteringKeys;
62+
final List<RelFieldCollation> clusteringOrder;
6363
private final Optional<String> keyspace;
6464
private final String columnFamily;
6565

Diff for: cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraToEnumerableConverter.java

+9-4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.calcite.plan.RelOptCluster;
3131
import org.apache.calcite.plan.RelOptCost;
3232
import org.apache.calcite.plan.RelOptPlanner;
33+
import org.apache.calcite.plan.RelOptTable;
3334
import org.apache.calcite.plan.RelTraitSet;
3435
import org.apache.calcite.rel.RelNode;
3536
import org.apache.calcite.rel.convert.ConverterImpl;
@@ -46,7 +47,8 @@
4647
import java.util.ArrayList;
4748
import java.util.List;
4849
import java.util.Map;
49-
import java.util.Objects;
50+
51+
import static java.util.Objects.requireNonNull;
5052

5153
/**
5254
* Relational expression representing a scan of a table in a Cassandra data source.
@@ -68,7 +70,8 @@ protected CassandraToEnumerableConverter(
6870

6971
@Override public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner,
7072
RelMetadataQuery mq) {
71-
return super.computeSelfCost(planner, mq).multiplyBy(.1);
73+
final RelOptCost cost = requireNonNull(super.computeSelfCost(planner, mq));
74+
return cost.multiplyBy(.1);
7275
}
7376

7477
@Override public Result implement(EnumerableRelImplementor implementor, Prefer pref) {
@@ -103,10 +106,12 @@ protected CassandraToEnumerableConverter(
103106
}
104107
final Expression selectFields =
105108
list.append("selectFields", constantArrayList(selectList, Pair.class));
109+
final RelOptTable cassandraTable =
110+
requireNonNull(cassandraImplementor.table);
106111
final Expression table =
107112
list.append("table",
108-
Objects.requireNonNull(
109-
cassandraImplementor.table.getExpression(
113+
requireNonNull(
114+
cassandraTable.getExpression(
110115
CassandraTable.CassandraQueryable.class)));
111116
final Expression predicates =
112117
list.append("predicates",

Diff for: cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,13 @@
4141
import java.io.IOException;
4242
import java.io.UncheckedIOException;
4343
import java.lang.reflect.Field;
44+
import java.net.URL;
4445
import java.time.Duration;
4546
import java.util.Locale;
46-
import java.util.Objects;
4747
import java.util.concurrent.ExecutionException;
4848

49+
import static java.util.Objects.requireNonNull;
50+
4951
/**
5052
* JUnit5 extension to start and stop embedded Cassandra server.
5153
*
@@ -83,9 +85,9 @@ class CassandraExtension implements ParameterResolver, ExecutionCondition {
8385
}
8486

8587
static ImmutableMap<String, String> getDataset(String resourcePath) {
88+
URL u = CassandraExtension.class.getResource(resourcePath);
8689
return ImmutableMap.of("model",
87-
Sources.of(Objects.requireNonNull(CassandraExtension.class.getResource(resourcePath)))
88-
.file().getAbsolutePath());
90+
Sources.of(requireNonNull(u, "u")).file().getAbsolutePath());
8991
}
9092

9193
/** Registers a Cassandra resource in root context, so it can be shared with

0 commit comments

Comments
 (0)