Skip to content

Commit

Permalink
[CALCITE-4422] Add MethodCanBeStatic check via ErrorProne
Browse files Browse the repository at this point in the history
private and final methods can be made static, so it is clear they do not access instance fields
  • Loading branch information
vlsi committed Dec 8, 2020
1 parent 0327135 commit 404f968
Show file tree
Hide file tree
Showing 170 changed files with 451 additions and 440 deletions.
3 changes: 3 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,9 @@ allprojects {
options.errorprone {
disableWarningsInGeneratedCode.set(true)
errorproneArgs.add("-XepExcludedPaths:.*/javacc/.*")
enable(
"MethodCanBeStatic"
)
disable(
"ComplexBooleanConstant",
"EqualsGetClass",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ protected CassandraFilterRule(Config config) {
* @param clusteringKeys Names of primary key columns
* @return True if the node represents an equality predicate on a primary key
*/
private boolean isEqualityOnKey(RexNode node, List<String> fieldNames,
private static boolean isEqualityOnKey(RexNode node, List<String> fieldNames,
Set<String> partitionKeys, List<String> clusteringKeys) {
if (node.getKind() != SqlKind.EQUALS) {
return false;
Expand All @@ -182,7 +182,8 @@ private boolean isEqualityOnKey(RexNode node, List<String> fieldNames,
* @param fieldNames Names of all columns in the table
* @return The field being compared or null if there is no key equality
*/
private String compareFieldWithLiteral(RexNode left, RexNode right, List<String> fieldNames) {
private static String compareFieldWithLiteral(RexNode left, RexNode right,
List<String> fieldNames) {
// FIXME Ignore casts for new and assume they aren't really necessary
if (left.isA(SqlKind.CAST)) {
left = ((RexCall) left).getOperands().get(0);
Expand Down Expand Up @@ -305,7 +306,7 @@ public RelNode convert(Sort sort, CassandraFilter filter) {
*
* @return True if it is possible to achieve this sort in Cassandra
*/
private boolean collationsCompatible(RelCollation sortCollation,
private static boolean collationsCompatible(RelCollation sortCollation,
RelCollation implicitCollation) {
List<RelFieldCollation> sortFieldCollations = sortCollation.getFieldCollations();
List<RelFieldCollation> implicitFieldCollations = implicitCollation.getFieldCollations();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ ArrayTable.Representation chooseRep(int ordinal) {
return new ArrayTable.ObjectArray(ordinal);
}

private long toLong(Object o) {
private static long toLong(Object o) {
// We treat Boolean and Character as if they were subclasses of
// Number but actually they are not.
if (o instanceof Boolean) {
Expand All @@ -382,7 +382,7 @@ private long toLong(Object o) {
}

@EnsuresNonNullIf(result = true, expression = "#1")
private boolean canBeLong(@Nullable Object o) {
private static boolean canBeLong(@Nullable Object o) {
return o instanceof Boolean
|| o instanceof Character
|| o instanceof Number;
Expand All @@ -397,7 +397,7 @@ private boolean canBeLong(@Nullable Object o) {
* @param min Minimum value to be encoded
* @param max Maximum value to be encoded (inclusive)
*/
private ArrayTable.Representation chooseFixedRep(
private static ArrayTable.Representation chooseFixedRep(
int ordinal, Primitive p, long min, long max) {
if (min == max) {
return new ArrayTable.Constant(ordinal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -975,15 +975,15 @@ private void initialize() {
}
}

private boolean isOverlapped(Pair<Long, Long> a, Pair<Long, Long> b) {
private static boolean isOverlapped(Pair<Long, Long> a, Pair<Long, Long> b) {
return !(b.left >= a.right);
}

private Pair<Long, Long> mergeWindows(Pair<Long, Long> a, Pair<Long, Long> b) {
private static Pair<Long, Long> mergeWindows(Pair<Long, Long> a, Pair<Long, Long> b) {
return new Pair<>(a.left <= b.left ? a.left : b.left, a.right >= b.right ? a.right : b.right);
}

private Pair<Long, Long> computeInitWindow(long ts, long gap) {
private static Pair<Long, Long> computeInitWindow(long ts, long gap) {
return new Pair<>(ts, ts + gap);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public static EnumerableLimitSort create(
return cost;
}

private double getValue(@Nullable RexNode r, double defaultValue) {
private static double getValue(@Nullable RexNode r, double defaultValue) {
if (r == null || r instanceof RexDynamicParam) {
return defaultValue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private Expression implementEmitter(EnumerableRelImplementor implementor,
builder.toBlock())));
}

private Expression implementMeasure(RexToLixTranslator translator,
private static Expression implementMeasure(RexToLixTranslator translator,
ParameterExpression rows_, ParameterExpression symbols_,
ParameterExpression i_, ParameterExpression row_, RexNode value) {
final SqlMatchFunction matchFunction;
Expand Down Expand Up @@ -331,7 +331,7 @@ private Expression implementMatcher(EnumerableRelImplementor implementor,
}

/** Generates code for a predicate. */
private Expression implementPredicate(PhysType physType,
private static Expression implementPredicate(PhysType physType,
ParameterExpression rows_, BlockStatement body) {
final List<MemberDeclaration> memberDeclarations = new ArrayList<>();
ParameterExpression row_ = Expressions.parameter(
Expand Down Expand Up @@ -384,8 +384,8 @@ private Expression implementPredicate(PhysType physType,
*
* <p>For example, for the pattern {@code (A B)}, generates
* {@code patternBuilder.symbol("A").symbol("B").seq()}. */
private Expression implementPattern(Expression patternBuilder_,
RexNode pattern) {
private static Expression implementPattern(Expression patternBuilder_,
RexNode pattern) {
switch (pattern.getKind()) {
case LITERAL:
final String symbol = ((RexLiteral) pattern).getValueAs(String.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ private Mappings.TargetMapping buildMapping(boolean left2Right) {
/**
* This function extends collation by appending new collation fields defined on keys.
*/
private RelCollation extendCollation(RelCollation collation, List<Integer> keys) {
private static RelCollation extendCollation(RelCollation collation, List<Integer> keys) {
List<RelFieldCollation> fieldsForNewCollation = new ArrayList<>(keys.size());
fieldsForNewCollation.addAll(collation.getFieldCollations());

Expand Down Expand Up @@ -360,7 +360,7 @@ private RelCollation extendCollation(RelCollation collation, List<Integer> keys)
* @param collation collation defined on the JOIN
* @param joinKeys the join keys
*/
private RelCollation intersectCollationAndJoinKey(
private static RelCollation intersectCollationAndJoinKey(
RelCollation collation, ImmutableIntList joinKeys) {
List<RelFieldCollation> fieldCollations = new ArrayList<>();
for (RelFieldCollation rf : collation.getFieldCollations()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public ClassDeclaration implementRoot(EnumerableRel rootRel,
memberDeclarations);
}

private ClassDeclaration classDecl(
private static ClassDeclaration classDecl(
JavaTypeFactoryImpl.SyntheticRecordType type) {
ClassDeclaration classDeclaration =
Expressions.classDecl(
Expand Down Expand Up @@ -545,7 +545,7 @@ static class TypeFinder extends VisitorImpl<Void> {
}

/** Adds a declaration of each synthetic type found in a code block. */
private class TypeRegistrar {
private static class TypeRegistrar {
private final List<MemberDeclaration> memberDeclarations;
private final Set<Type> seen = new HashSet<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public EnumerableTableFunctionScan(RelOptCluster cluster,
}
}

private boolean isImplementorDefined(RexCall call) {
private static boolean isImplementorDefined(RexCall call) {
if (call.getOperator() instanceof SqlWindowTableFunction
&& RexImpTable.INSTANCE.get((SqlWindowTableFunction) call.getOperator()) != null) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ private Expression getExpression(PhysType physType) {
return toRows(physType, expression2);
}

private Expression toEnumerable(Expression expression) {
private static Expression toEnumerable(Expression expression) {
final Type type = expression.getType();
if (Types.isArray(type)) {
if (requireNonNull(toClass(type).getComponentType()).isPrimitive()) {
Expand Down Expand Up @@ -309,7 +309,7 @@ private JavaRowFormat format() {
return JavaRowFormat.CUSTOM;
}

private boolean hasCollectionField(RelDataType rowType) {
private static boolean hasCollectionField(RelDataType rowType) {
for (RelDataTypeField field : rowType.getFieldList()) {
switch (field.getType().getSqlTypeName()) {
case ARRAY:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private WindowRelInputGetter(Expression row,
}

@SuppressWarnings({"unused", "nullness"})
private void sampleOfTheGeneratedWindowedAggregate() {
private static void sampleOfTheGeneratedWindowedAggregate() {
// Here's overview of the generated code
// For each list of rows that have the same partitioning key, evaluate
// all of the windowed aggregate functions.
Expand Down Expand Up @@ -536,7 +536,7 @@ private void sampleOfTheGeneratedWindowedAggregate() {
return implementor.result(inputPhysType, builder.toBlock());
}

private Function<BlockBuilder, WinAggFrameResultContext>
private static Function<BlockBuilder, WinAggFrameResultContext>
getBlockBuilderWinAggFrameResultContextFunction(
final JavaTypeFactory typeFactory, final SqlConformance conformance,
final Result result, final List<Expression> translatedConstants,
Expand Down Expand Up @@ -647,7 +647,7 @@ public Expression getRow(Expression rowIndex) {
};
}

private Pair<Expression, Expression> getPartitionIterator(
private static Pair<Expression, Expression> getPartitionIterator(
BlockBuilder builder,
Expression source_,
PhysType inputPhysType,
Expand Down Expand Up @@ -754,7 +754,7 @@ private Pair<Expression, Expression> getPartitionIterator(
comparator_)));
}

private Pair<@Nullable Expression, @Nullable Expression> getRowCollationKey(
private static Pair<@Nullable Expression, @Nullable Expression> getRowCollationKey(
BlockBuilder builder, PhysType inputPhysType,
Group group, int windowIdx) {
if (!(group.isRows
Expand Down Expand Up @@ -859,7 +859,7 @@ private void declareAndResetState(final JavaTypeFactory typeFactory,
}
}

private void implementAdd(List<AggImpState> aggs,
private static void implementAdd(List<AggImpState> aggs,
final BlockBuilder builder7,
final Function<BlockBuilder, WinAggFrameResultContext> frame,
final Function<AggImpState, List<RexNode>> rexArguments,
Expand All @@ -883,7 +883,7 @@ private void implementAdd(List<AggImpState> aggs,
}
}

private boolean implementResult(List<AggImpState> aggs,
private static boolean implementResult(List<AggImpState> aggs,
final BlockBuilder builder,
final Function<BlockBuilder, WinAggFrameResultContext> frame,
final Function<AggImpState, List<RexNode>> rexArguments,
Expand Down Expand Up @@ -918,13 +918,13 @@ private boolean implementResult(List<AggImpState> aggs,
return nonEmpty;
}

private Expression translateBound(RexToLixTranslator translator,
ParameterExpression i_, Expression row_, Expression min_,
Expression max_, Expression rows_, Group group,
boolean lower,
PhysType physType,
@SuppressWarnings("unused") Expression rowComparator, // TODO: remove or use
@Nullable Expression keySelector, @Nullable Expression keyComparator) {
private static Expression translateBound(RexToLixTranslator translator,
ParameterExpression i_, Expression row_, Expression min_,
Expression max_, Expression rows_, Group group,
boolean lower,
PhysType physType,
@SuppressWarnings("unused") Expression rowComparator, // TODO: remove or use
@Nullable Expression keySelector, @Nullable Expression keyComparator) {
RexWindowBound bound = lower ? group.lowerBound : group.upperBound;
if (bound.isUnbounded()) {
return bound.isPreceding() ? min_ : max_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public ReflectiveCallNotNullImplementor(Method method) {
return translator.handleMethodCheckedExceptions(callExpr);
}

private boolean containsCheckedException(Method method) {
private static boolean containsCheckedException(Method method) {
Class[] exceptions = method.getExceptionTypes();
if (exceptions == null || exceptions.length == 0) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ public class RexImpTable {
tvfImplementorMap.put(SESSION, SessionImplementor::new);
}

private <T> Supplier<T> constructorSupplier(Class<T> klass) {
private static <T> Supplier<T> constructorSupplier(Class<T> klass) {
final Constructor<T> constructor;
try {
constructor = klass.getDeclaredConstructor();
Expand Down Expand Up @@ -2208,7 +2208,7 @@ private static class BinaryImplementor extends AbstractRexCallImplementor {
}

/** Returns whether any of a call's operands have ANY type. */
private boolean anyAnyOperands(RexCall call) {
private static boolean anyAnyOperands(RexCall call) {
for (RexNode operand : call.operands) {
if (operand.getType().getSqlTypeName() == SqlTypeName.ANY) {
return true;
Expand All @@ -2228,7 +2228,7 @@ private Expression callBackupMethodAnyType(List<Expression> expressions) {
expression0, expression1);
}

private Expression maybeBox(Expression expression) {
private static Expression maybeBox(Expression expression) {
final Primitive primitive = Primitive.of(expression.getType());
if (primitive != null) {
expression = Expressions.box(expression, primitive);
Expand Down Expand Up @@ -2461,7 +2461,7 @@ private static class CoalesceImplementor extends AbstractRexCallImplementor {
return implementRecurse(translator, argValueList);
}

private Expression implementRecurse(RexToLixTranslator translator,
private static Expression implementRecurse(RexToLixTranslator translator,
final List<Expression> argValueList) {
if (argValueList.size() == 1) {
return argValueList.get(0);
Expand Down Expand Up @@ -2508,7 +2508,7 @@ private static class CastImplementor extends AbstractRexCallImplementor {
targetType, argValueList.get(0));
}

private RelDataType nullifyType(JavaTypeFactory typeFactory,
private static RelDataType nullifyType(JavaTypeFactory typeFactory,
final RelDataType type, final boolean nullable) {
if (type instanceof RelDataTypeFactoryImpl.JavaType) {
Class<?> javaClass = ((RelDataTypeFactoryImpl.JavaType) type).getJavaClass();
Expand Down Expand Up @@ -2809,7 +2809,7 @@ private static class DatetimeArithmeticImplementor
}

/** Normalizes a TIME value into 00:00:00..23:59:39. */
private Expression normalize(SqlTypeName typeName, Expression e) {
private static Expression normalize(SqlTypeName typeName, Expression e) {
switch (typeName) {
case TIME:
return Expressions.call(BuiltInMethod.FLOOR_MOD.method, e,
Expand Down Expand Up @@ -2878,7 +2878,7 @@ private static class LastImplementor implements MatchImplementor {
}
}

private void setInputGetterIndex(RexToLixTranslator translator, @Nullable Expression o) {
private static void setInputGetterIndex(RexToLixTranslator translator, @Nullable Expression o) {
requireNonNull((EnumerableMatch.PassedRowsInputGetter) translator.inputGetter,
"inputGetter").setIndex(o);
}
Expand Down Expand Up @@ -3011,7 +3011,7 @@ private ParameterExpression genIsNullStatement(
}

/** Ensures that operands have identical type. */
private List<Expression> harmonize(final List<Expression> argValueList,
private static List<Expression> harmonize(final List<Expression> argValueList,
final RexToLixTranslator translator, final RexCall call) {
int nullCount = 0;
final List<RelDataType> types = new ArrayList<>();
Expand Down Expand Up @@ -3057,7 +3057,7 @@ private List<Expression> unboxIfNecessary(final List<Expression> argValueList) {
if (nullPolicy == NullPolicy.STRICT || nullPolicy == NullPolicy.ANY
|| nullPolicy == NullPolicy.SEMI_STRICT) {
unboxValueList = argValueList.stream()
.map(this::unboxExpression)
.map(AbstractRexCallImplementor::unboxExpression)
.collect(Collectors.toList());
}
if (nullPolicy == NullPolicy.ARG0 && argValueList.size() > 0) {
Expand All @@ -3067,7 +3067,7 @@ private List<Expression> unboxIfNecessary(final List<Expression> argValueList) {
return unboxValueList;
}

private Expression unboxExpression(final Expression argValue) {
private static Expression unboxExpression(final Expression argValue) {
Primitive fromBox = Primitive.ofBox(argValue.getType());
if (fromBox == null || fromBox == Primitive.VOID) {
return argValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ private Result implementCaseWhen(RexCall call) {
* }
* }</pre></blockquote>
*/
private void implementRecursively(final RexToLixTranslator currentTranslator,
private static void implementRecursively(final RexToLixTranslator currentTranslator,
final List<RexNode> operandList, final ParameterExpression valueVariable, int pos) {
final BlockBuilder currentBlockBuilder = currentTranslator.getBlockBuilder();
final List<@Nullable Type> storageTypes = EnumUtils.internalTypes(operandList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected final int getStateSize() {
return stateSize;
}

protected final void accAdvance(AggAddContext add, Expression acc,
protected static void accAdvance(AggAddContext add, Expression acc,
Expression next) {
add.currentBlock().add(
Expressions.statement(
Expand All @@ -71,7 +71,7 @@ protected final void accAdvance(AggAddContext add, Expression acc,
return res;
}

private boolean anyNullable(List<? extends RelDataType> types) {
private static boolean anyNullable(List<? extends RelDataType> types) {
for (RelDataType type : types) {
if (type.isNullable()) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ protected JdbcJoinRule(Config config) {
* @param node Condition
* @return Whether condition is supported
*/
private boolean canJoinOnCondition(RexNode node) {
private static boolean canJoinOnCondition(RexNode node) {
final List<RexNode> operands;
switch (node.getKind()) {
case AND:
Expand Down
Loading

0 comments on commit 404f968

Please sign in to comment.