Skip to content

Commit c85b66e

Browse files
armandinomsridhar
andauthored
Minor cleanup, no behavior changes (#1487)
Cleanup a few things I noticed while browsing the source. No logic changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Simplified conditional logic across multiple core modules to reduce nesting and improve readability. * Modernized type handling with newer language constructs to remove unnecessary casts and streamline control flow. * Consistent generic initialization improvements for clearer, more concise code. * **Style** * Minor null-safety and stylistic updates to improve robustness and maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Manu Sridharan <msridhar@gmail.com>
1 parent 5593abf commit c85b66e

File tree

9 files changed

+38
-52
lines changed

9 files changed

+38
-52
lines changed

nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import com.uber.nullaway.fixserialization.SerializationService;
5959
import java.util.Iterator;
6060
import java.util.List;
61+
import java.util.Objects;
6162
import java.util.Set;
6263
import java.util.stream.StreamSupport;
6364
import javax.lang.model.element.Element;
@@ -178,8 +179,8 @@ private static boolean canHaveSuppressWarningsAnnotation(Tree tree) {
178179
private boolean hasPathSuppression(TreePath treePath, String subcheckerName) {
179180
return StreamSupport.stream(treePath.spliterator(), false)
180181
.filter(ErrorBuilder::canHaveSuppressWarningsAnnotation)
181-
.map(tree -> ASTHelpers.getSymbol(tree))
182-
.filter(symbol -> symbol != null)
182+
.map(ASTHelpers::getSymbol)
183+
.filter(Objects::nonNull)
183184
.anyMatch(
184185
symbol ->
185186
symbolHasSuppressWarningsAnnotation(symbol, subcheckerName)

nullaway/src/main/java/com/uber/nullaway/NullAway.java

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,10 +1580,8 @@ private ImmutableMultimap<Tree, Element> computeTree2Init(
15801580
nullnessAnalysis.getNonnullFieldsOfReceiverAtExit(memberPath, state.context));
15811581
}
15821582
}
1583-
if (memberTree instanceof MethodTree methodTree) {
1584-
if (isConstructor(methodTree)) {
1585-
constructors.add(methodTree);
1586-
}
1583+
if (memberTree instanceof MethodTree methodTree && isConstructor(methodTree)) {
1584+
constructors.add(methodTree);
15871585
}
15881586
}
15891587
// all the initializer blocks have run before any code inside a constructor
@@ -2476,14 +2474,12 @@ private Set<Element> getSafeInitMethods(
24762474
// as "top level" for the purposes of finding initialization methods. Any exception happening
24772475
// there is also an
24782476
// exception of the full method.
2479-
if (stmt instanceof TryTree tryTree) {
2480-
if (tryTree.getCatches().isEmpty()) {
2481-
if (tryTree.getBlock() != null) {
2482-
result.addAll(getSafeInitMethods(tryTree.getBlock(), classSymbol, state));
2483-
}
2484-
if (tryTree.getFinallyBlock() != null) {
2485-
result.addAll(getSafeInitMethods(tryTree.getFinallyBlock(), classSymbol, state));
2486-
}
2477+
if (stmt instanceof TryTree tryTree && tryTree.getCatches().isEmpty()) {
2478+
if (tryTree.getBlock() != null) {
2479+
result.addAll(getSafeInitMethods(tryTree.getBlock(), classSymbol, state));
2480+
}
2481+
if (tryTree.getFinallyBlock() != null) {
2482+
result.addAll(getSafeInitMethods(tryTree.getFinallyBlock(), classSymbol, state));
24872483
}
24882484
}
24892485
}

nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -267,17 +267,17 @@ static AccessPath switchRoot(AccessPath origAP, Element newRoot) {
267267
}
268268

269269
private static Node stripCasts(Node node) {
270-
while (node instanceof TypeCastNode) {
271-
node = ((TypeCastNode) node).getOperand();
270+
while (node instanceof TypeCastNode typeCastNode) {
271+
node = typeCastNode.getOperand();
272272
}
273273
return node;
274274
}
275275

276276
private static @Nullable MapKey argumentToMapKeySpecifier(
277277
Node argument, VisitorState state, AccessPathContext apContext) {
278278
// Required to have Node type match Tree type in some instances.
279-
if (argument instanceof WideningConversionNode) {
280-
argument = ((WideningConversionNode) argument).getOperand();
279+
if (argument instanceof WideningConversionNode node) {
280+
argument = node.getOperand();
281281
}
282282
// A switch at the Tree level should be faster than multiple if checks at the Node level.
283283
switch (castToNonNull(argument.getTree()).getKind()) {
@@ -456,9 +456,8 @@ private static boolean isBoxingMethod(Symbol.MethodSymbol methodSymbol) {
456456
Tree tree = argumentNode.getTree();
457457
if (tree == null) {
458458
return null; // Not an AP
459-
} else if (tree instanceof MethodInvocationTree) {
459+
} else if (tree instanceof MethodInvocationTree methodInvocationTree) {
460460
// Check for boxing call
461-
MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree;
462461
if (methodInvocationTree.getArguments().size() == 1
463462
&& isBoxingMethod(ASTHelpers.getSymbol(methodInvocationTree))) {
464463
tree = methodInvocationTree.getArguments().get(0);
@@ -561,8 +560,7 @@ && isBoxingMethod(ASTHelpers.getSymbol(methodInvocationTree))) {
561560
*/
562561
public static @Nullable AccessPath mapWithIteratorContentsKey(
563562
Node mapNode, LocalVariableNode iterVar, AccessPathContext apContext) {
564-
IteratorContentsKey iterContentsKey =
565-
new IteratorContentsKey((VariableElement) iterVar.getElement());
563+
IteratorContentsKey iterContentsKey = new IteratorContentsKey(iterVar.getElement());
566564
return buildAccessPathRecursive(mapNode, new ArrayDeque<>(), apContext, iterContentsKey);
567565
}
568566

nullaway/src/main/java/com/uber/nullaway/dataflow/DataFlow.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,24 +94,21 @@ public final class DataFlow {
9494
CacheBuilder.newBuilder()
9595
.maximumSize(MAX_CACHE_SIZE)
9696
.build(
97-
new CacheLoader<CfgParams, ControlFlowGraph>() {
97+
new CacheLoader<>() {
9898
@Override
9999
public ControlFlowGraph load(CfgParams key) {
100100
TreePath codePath = key.codePath();
101101
TreePath bodyPath;
102102
UnderlyingAST ast;
103103
ProcessingEnvironment env = key.environment();
104-
if (codePath.getLeaf() instanceof LambdaExpressionTree) {
105-
LambdaExpressionTree lambdaExpressionTree =
106-
(LambdaExpressionTree) codePath.getLeaf();
104+
if (codePath.getLeaf() instanceof LambdaExpressionTree lambdaExpressionTree) {
107105
MethodTree enclMethod =
108106
ASTHelpers.findEnclosingNode(codePath, MethodTree.class);
109107
ClassTree enclClass =
110108
castToNonNull(ASTHelpers.findEnclosingNode(codePath, ClassTree.class));
111109
ast = new UnderlyingAST.CFGLambda(lambdaExpressionTree, enclClass, enclMethod);
112110
bodyPath = new TreePath(codePath, lambdaExpressionTree.getBody());
113-
} else if (codePath.getLeaf() instanceof MethodTree) {
114-
MethodTree method = (MethodTree) codePath.getLeaf();
111+
} else if (codePath.getLeaf() instanceof MethodTree method) {
115112
ClassTree enclClass =
116113
castToNonNull(ASTHelpers.findEnclosingNode(codePath, ClassTree.class));
117114
ast = new UnderlyingAST.CFGMethod(method, enclClass);

nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -293,13 +293,11 @@ public Type visitMethodType(Type.MethodType t, Type other) {
293293

294294
@Override
295295
public Type visitClassType(Type.ClassType t, Type other) {
296-
if (other instanceof Type.WildcardType wt) {
297-
if (wt.kind == BoundKind.EXTENDS) {
298-
// As a temporary measure, we restore nullability annotations from the upper bound of the
299-
// wildcard.
300-
// TODO revisit this decision when we add fuller support for inference and wildcards.
301-
return visit(t, wt.getExtendsBound());
302-
}
296+
if (other instanceof Type.WildcardType wt && wt.kind == BoundKind.EXTENDS) {
297+
// As a temporary measure, we restore nullability annotations from the upper bound of the
298+
// wildcard.
299+
// TODO revisit this decision when we add fuller support for inference and wildcards.
300+
return visit(t, wt.getExtendsBound());
303301
}
304302

305303
Type updated = updateDirectNullabilityAnnotationsForType(t, other);

nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,10 @@ protected boolean validateAnnotationSyntax(
184184
return false;
185185
}
186186
VariableElement field = getFieldOfClass(classSymbol, fieldName);
187-
if (field != null) {
188-
if (field.getModifiers().contains(Modifier.STATIC)) {
189-
continue;
190-
}
187+
if (field != null && field.getModifiers().contains(Modifier.STATIC)) {
188+
continue;
191189
}
190+
192191
if (fieldName.contains(".")) {
193192
if (!fieldName.startsWith(THIS_NOTATION)) {
194193
message =
@@ -264,13 +263,12 @@ protected boolean validateAnnotationSyntax(
264263
}
265264

266265
protected boolean isThisDotStaticField(Symbol.ClassSymbol classSymbol, String expression) {
267-
if (expression.contains(".")) {
268-
if (expression.startsWith(THIS_NOTATION)) {
269-
String fieldName = expression.substring(THIS_NOTATION.length());
270-
VariableElement field = getFieldOfClass(classSymbol, fieldName);
271-
return field != null && field.getModifiers().contains(Modifier.STATIC);
272-
}
266+
if (expression.contains(".") && expression.startsWith(THIS_NOTATION)) {
267+
String fieldName = expression.substring(THIS_NOTATION.length());
268+
VariableElement field = getFieldOfClass(classSymbol, fieldName);
269+
return field != null && field.getModifiers().contains(Modifier.STATIC);
273270
}
271+
274272
return false;
275273
}
276274
}

nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,13 +255,12 @@ private static String getErrorMessageForViolatedContract(
255255
private static void collectNestedReturnedExpressions(
256256
TreePath expressionPath, List<TreePath> output) {
257257
ExpressionTree expression = (ExpressionTree) expressionPath.getLeaf();
258-
while (expression instanceof ParenthesizedTree) {
259-
ExpressionTree nestedExpression = ((ParenthesizedTree) expression).getExpression();
258+
while (expression instanceof ParenthesizedTree parenthesizedTree) {
259+
ExpressionTree nestedExpression = parenthesizedTree.getExpression();
260260
expressionPath = new TreePath(expressionPath, nestedExpression);
261261
expression = nestedExpression;
262262
}
263-
if (expression instanceof ConditionalExpressionTree) {
264-
ConditionalExpressionTree conditionalExpression = (ConditionalExpressionTree) expression;
263+
if (expression instanceof ConditionalExpressionTree conditionalExpression) {
265264
TreePath truePath = new TreePath(expressionPath, conditionalExpression.getTrueExpression());
266265
TreePath falsePath = new TreePath(expressionPath, conditionalExpression.getFalseExpression());
267266
collectNestedReturnedExpressions(truePath, output);

nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ protected boolean validateAnnotationSemantics(
9494
.stream()
9595
.map(e -> e.getSimpleName().toString())
9696
.collect(Collectors.toSet());
97-
Set<String> nonnullFieldsAtExit = new HashSet<String>();
97+
Set<String> nonnullFieldsAtExit = new HashSet<>();
9898
nonnullFieldsAtExit.addAll(nonnullFieldsOfReceiverAtExit);
9999
nonnullFieldsAtExit.addAll(nonnullStaticFieldsAtExit);
100100
Set<String> fieldNames = getAnnotationValueArray(methodSymbol, annotName, false);

nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@ public void onDataflowVisitReturn(
195195
// Whether the return true expression evaluates to a boolean literal or not. If null, then not
196196
// a boolean literal.
197197
Boolean expressionAsBoolean = null;
198-
if (returnTree.getExpression() instanceof LiteralTree) {
199-
LiteralTree expressionAsLiteral = (LiteralTree) returnTree.getExpression();
198+
if (returnTree.getExpression() instanceof LiteralTree expressionAsLiteral) {
200199
if (expressionAsLiteral.getValue() instanceof Boolean) {
201200
expressionAsBoolean = (Boolean) expressionAsLiteral.getValue();
202201
}

0 commit comments

Comments
 (0)