Enable PatternMatchingInstanceOf check and fix all warnings #1412
Conversation
WalkthroughRemoved an explicit disable of the PatternMatchingInstanceof Error Prone check in build.gradle and refactored many Java source files to use Java's pattern matching for Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-28T04:54:20.953ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (31)
build.gradlejar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.javajar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.javanullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.javanullaway/src/main/java/com/uber/nullaway/ErrorBuilder.javanullaway/src/main/java/com/uber/nullaway/InvocationArguments.javanullaway/src/main/java/com/uber/nullaway/NullAway.javanullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.javanullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.javanullaway/src/main/java/com/uber/nullaway/dataflow/DataFlow.javanullaway/src/main/java/com/uber/nullaway/dataflow/EnclosingEnvironmentNullness.javanullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.javanullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.javanullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.javanullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/MethodAnalysisContext.javanullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.javanullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.javanullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.javanullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.javanullaway/src/test/java/com/uber/nullaway/tools/FieldInitDisplay.javanullaway/src/test/java/com/uber/nullaway/tools/version1/ErrorDisplayV1.java
💤 Files with no reviewable changes (1)
- build.gradle
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
📚 Learning: 2025-08-28T04:54:20.953Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/dataflow/DataFlow.javanullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.javanullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.javajar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.javanullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/NullAway.javanullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.javanullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.javanullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.javanullaway/src/main/java/com/uber/nullaway/ErrorBuilder.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.javanullaway/src/main/java/com/uber/nullaway/dataflow/EnclosingEnvironmentNullness.javanullaway/src/main/java/com/uber/nullaway/InvocationArguments.javanullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.javanullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/test/java/com/uber/nullaway/tools/version1/ErrorDisplayV1.javanullaway/src/main/java/com/uber/nullaway/NullAway.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java
📚 Learning: 2025-10-29T23:56:18.236Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.javajar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.javanullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.javanullaway/src/main/java/com/uber/nullaway/NullAway.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java
📚 Learning: 2025-08-29T18:41:43.584Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/NullAway.java
🧬 Code graph analysis (1)
nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java (1)
nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java (1)
AccessPath(78-805)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build spring-framework with snapshot
- GitHub Check: Build caffeine with snapshot
- GitHub Check: Build and test on ubuntu-latest
🔇 Additional comments (51)
nullaway/src/main/java/com/uber/nullaway/dataflow/EnclosingEnvironmentNullness.java (1)
54-55: LGTM! Clean pattern matching refactor.The migration from explicit cast to pattern matching is correct and improves readability without changing behavior.
nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java (1)
57-57: LGTM! Pattern matching refactoring improves code clarity.The migration from explicit instanceof checks and casts to pattern matching instanceof syntax is correct and improves readability. Both the toString method (line 57) and equals method (lines 63-64) preserve the original logic while eliminating redundant type casts.
Also applies to: 63-64
nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java (1)
104-106: LGTM! Clean pattern matching refactor.The refactoring from explicit instanceof check and cast to pattern matching instanceof is correct. The pattern variable
varSymbolis properly scoped to the conditional block and eliminates the need for an explicit cast while preserving the same logic.nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java (1)
42-43: LGTM! Clean pattern matching refactor.The migration to pattern matching instanceof eliminates the need for an explicit cast while preserving identical behavior. The refactor follows modern Java best practices and improves code readability.
nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.java (1)
215-219: LGTM! Clean pattern matching refactor.The migration from explicit instanceof check and casts to pattern matching is correct and idiomatic. The pattern variable
cfgMethodproperly binds when the instanceof check succeeds (after the early return), and eliminates the redundant explicit casts on lines 218-219.nullaway/src/main/java/com/uber/nullaway/dataflow/DataFlow.java (2)
289-289: LGTM! Clean pattern matching refactor.The instanceof check with pattern matching correctly replaces the explicit cast, making the code more concise while preserving the exact same logic.
318-318: LGTM! Clean pattern matching refactor.The instanceof check with pattern matching correctly replaces the explicit cast, improving readability while maintaining the same null body check for abstract methods.
nullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.java (2)
132-137: LGTM! Clean pattern matching refactor.The instanceof check with pattern matching correctly eliminates the explicit cast while preserving the exact same logic. The pattern variable
iteratorContentsKeyis appropriately scoped and used.
210-214: LGTM! Proper equals() pattern matching refactor.The negated instanceof with pattern matching correctly combines the type check and cast, eliminating boilerplate while maintaining identical behavior. The pattern variable
otheris properly used for the comparison.nullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.java (1)
72-75: LGTM! Clean pattern matching refactoring.The migration from explicit instanceof + cast to pattern matching is correct and idiomatic. The pattern variable
methodSymbolis properly scoped after the early return guard, eliminating the need for the redundant explicit cast while maintaining identical behavior.nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java (1)
83-86: LGTM! Pattern matching simplifies the type check.The refactor correctly uses Java pattern matching for instanceof, eliminating the explicit cast while preserving identical equality logic. The bound variable
thatis properly used in all subsequent field comparisons.nullaway/src/test/java/com/uber/nullaway/tools/version1/ErrorDisplayV1.java (1)
79-82: LGTM! Pattern matching simplifies the type check.The refactor correctly uses Java pattern matching for instanceof, eliminating the explicit cast while preserving identical equality logic. The bound variable
thatis properly used in all subsequent field comparisons.nullaway/src/test/java/com/uber/nullaway/tools/FieldInitDisplay.java (1)
54-57: LGTM! Pattern matching simplifies the type check.The refactor correctly uses Java pattern matching for instanceof, eliminating the explicit cast while preserving identical equality logic. The bound variable
thatis properly used in all subsequent field comparisons.nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java (1)
302-303: LGTM! Clean pattern matching refactor.The instanceof check has been correctly refactored to use Java pattern matching, eliminating the redundant explicit cast while preserving identical functionality. The pattern variable
methodInvocationNodeis properly scoped and used within the if-block.nullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.java (1)
61-62: LGTM! Clean pattern matching refactor.The refactoring correctly adopts pattern matching for
instanceof, eliminating the redundant cast while preserving identical behavior and scope.jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java (1)
115-116: LGTM! Clean refactor to pattern matching.The refactor from explicit instanceof cast to pattern matching simplifies the code and improves readability with a more descriptive variable name. The logic and behavior are preserved.
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (3)
125-128: LGTM! Clean pattern matching refactor.The conversion from explicit cast to pattern matching is correct and makes the code more concise while preserving the filtering logic.
253-254: LGTM! Concise pattern matching usage.The pattern variable
stringeliminates the explicit cast and improves readability.
699-700: LGTM! Pattern matching in ternary operator.The pattern variable
jcMethodInvocationcleanly eliminates the explicit cast while accessing thevarargsElementfield.nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java (1)
147-148: LGTM! Clean pattern matching refactoring.All 18 instanceof checks have been correctly modernized to use Java pattern matching with inline variable bindings. Each pattern variable is properly scoped and used immediately in its corresponding code block. The refactorings maintain identical control flow and logic—no behavioral changes introduced.
Also applies to: 235-236, 323-323, 536-537, 540-541, 568-569, 570-571, 686-686, 873-873, 1091-1092, 1543-1544, 1570-1571, 1576-1576, 1668-1669, 1693-1694, 1812-1812, 1826-1827, 2050-2051
jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.java (3)
200-204: LGTM! Pattern matching correctly applied.The refactoring from explicit casts to pattern matching for
SSAGetInstructionandSSAPutInstructionis correct and improves code readability. The compound conditions with&&properly leverage short-circuit evaluation.
205-219: LGTM! Consistent use of pattern variable throughout the block.The pattern matching refactoring for
SSAAbstractInvokeInstructioneliminates multiple casts and uses the boundcallInstvariable consistently throughout the conditional block. The logic for handling static vs. non-static invocations remains unchanged.
261-263: LGTM! Pattern matching simplifies the return instruction handling.The refactoring correctly applies pattern matching for
SSAReturnInstruction, eliminating the need for an explicit cast when checking the return value.nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java (4)
135-143: LGTM! Clean refactoring to pattern matching.The instanceof check and cast are correctly replaced with pattern matching syntax. The pattern variable
superArrayTypeis properly scoped and used within the if block.
276-286: LGTM! Pattern matching correctly applied.The refactoring properly eliminates the explicit cast while maintaining the same logic. The pattern variable
tvis used correctly within the if block for the capture check and annotation checks.
304-314: LGTM! Pattern matching modernization.The instanceof check for
Symbol.MethodSymbolis correctly refactored to use pattern matching. The pattern variablemethodSymbolis appropriately used within the conditional block.
315-323: LGTM! Pattern matching consistently applied.The instanceof check for
Symbol.ClassSymbolfollows the same clean pattern matching refactoring. The pattern variableclassSymbolis correctly scoped and used within the else-if block.nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java (1)
258-259: LGTM! Clean refactoring to pattern matching instanceof.All conversions from explicit instanceof checks with casts to pattern matching are correct and maintain identical behavior. The changes improve code readability by eliminating redundant cast statements while preserving all logic and control flow.
Also applies to: 267-269, 276-277, 304-308, 321-322, 347-350, 401-401, 426-427, 511-512, 572-573, 578-579
nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java (6)
203-207: LGTM! Clean pattern matching conversion.The conversion from explicit cast to pattern matching in
unwrapAssignExpris correct and improves code readability. The bound variableassignmentNodeis used appropriately.
523-549: LGTM! Excellent pattern matching refactoring in visitAssignment.All three pattern matching conversions (
LocalVariableNode,ArrayAccessNode, andFieldAccessNode) are correct and improve code clarity. The bound variables are used consistently throughout their respective code blocks, eliminating redundant casts.
594-617: LGTM! Well-executed nested pattern matching.The pattern matching conversions in
handleEnhancedForOverKeySetare correct, including the nestedLocalVariableNodecheck. The bound variables (methodInvandlocalVariableNode) are used appropriately throughout the enhanced-for key set tracking logic.
627-635: LGTM! Clear pattern matching for nested invocation handling.The pattern matching conversion in
getMapNodeForKeySetIteratorCallcorrectly bindsbaseInvocationand uses it to extract the map node from the keySet invocation chain.
935-941: LGTM! Pattern matching simplifies assertion condition handling.The pattern matching conversion in
visitAssertionErroris correct. The boundnotEqualNodevariable is used appropriately to check for null comparison patterns and extract the left operand.
1021-1024: LGTM! Pattern matching streamlines type pattern variable extraction.The pattern matching conversion in
getVariableNodeForTypePatternis correct. The boundlocalVariableNodeis returned when the operand matches the pattern variable symbol, making the code more concise.nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java (3)
43-44: LGTM! Clean pattern matching refactoring.The conversion to pattern matching eliminates the explicit cast while preserving the ternary logic. The pattern variable
declaredTypeis appropriately scoped to the true branch.
67-69: LGTM! Consistent pattern matching usage.The refactoring correctly applies pattern matching to eliminate the explicit cast, consistent with the
asSupermethod above.
159-164: LGTM! Proper pattern matching with negated check.The negated pattern matching with early return is idiomatic, and the pattern variable
wildcardTypecorrectly replaces the explicit cast at line 164.nullaway/src/main/java/com/uber/nullaway/handlers/MethodAnalysisContext.java (1)
57-63: The pattern matching refactor is idiomatic and safe. A search of the codebase confirms no subclasses ofMethodAnalysisContextexist, making the semantic difference betweengetClass()comparison andinstanceofchecking non-existent in practice. The change poses no behavioral risk and aligns with modern Java best practices.nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java (1)
165-177: Pattern match inisSymbolUnannotatedis behavior‑preservingThe new
symbol instanceof Symbol.ClassSymbol csblock is equivalent to the oldinstanceof+cast; the primitive.classspecial‑case and enclosing‑class fallback remain unchanged. Looks good.nullaway/src/main/java/com/uber/nullaway/InvocationArguments.java (1)
47-55: ConstructorinvocationTreepattern matching is a mechanical refactorSwitching to
instanceof MethodInvocationTree methodInvocationTree/NewClassTree newClassTreepreserves the previous branching and exception behavior while simplifying casts. No functional change.nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java (1)
103-116: Inline binding ofreceiver_methodis equivalentThe
receiver instanceof MethodInvocationNode receiver_methodcheck is a direct replacement for the oldinstanceof+cast and is only used inside the guarded block. Recursive call and access‑path extraction remain the same.nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java (2)
175-204: Method‑invocation guard now uses pattern binding without changing behaviorThe new condition:
if (!(expr instanceof MethodInvocationTree && exprSymbol instanceof Symbol.MethodSymbol methodSymbol)) { return exprMayBeNull; }preserves the original requirement that both
exprbe aMethodInvocationTreeandexprSymbolbe aMethodSymbolbefore proceeding, while avoiding a separate cast. No change to the subsequent unannotated/return‑nullability logic.
285-303: VarSymbol pattern match in nullable‑field check is safe
symbol instanceof Symbol.VarSymbol varSymbol && symbol.getKind().isField()mirrors the oldinstanceof+field‑kind+cast sequence; usingsymbol.getKind()instead ofvarSymbol.getKind()does not affect semantics. Primitive.classand non‑field symbols are still excluded as before.nullaway/src/main/java/com/uber/nullaway/NullAway.java (5)
333-346: Helper‑method pattern matching (isCallToUnmarkedMethod,matchReturn,getSymbolOfSuperConstructor) is mechanicalAcross these helpers, each
instanceof+cast has been replaced with a pattern variable:
exprTree instanceof MethodInvocationTree methodInvokeleaf instanceof MethodTree enclosingMethodstmt instanceof ExpressionStatementTree expressionStatementTreeandexpression instanceof MethodInvocationTree methodInvocationTreeIn all cases, the conditions guarding use of the bound variables are unchanged, and nullability checks (
methodSymbol == nullpaths, etc.) are preserved. No behavioral change here.Also applies to: 421-431, 510-526
1249-1286: Field read‑before‑init checks retain semantics under pattern matching
In
checkForReadBeforeInit, the new patterns:
enclosing instanceof MethodTree methodTree && !ASTHelpers.getSymbol(methodTree).isStatic()enclosing instanceof BlockTree blockTree && !blockTree.isStatic()still implement the same “static field must be in a static context” filter.
In
okToReadBeforeInitialized, pattern bindings forAssignmentTree,BinaryTree, andMethodInvocationTreekeep the previous logic:
- Writes (
assignment.getVariable().equals(leaf)) are still exempt.- Null‑comparison binary checks are unchanged.
castToNonNull/ handler‑driven cast positions are still recognized by matching on the sameargumentsindex.Overall, the behavior of read‑before‑init detection is maintained.
Also applies to: 1525-1565
1288-1325: Initializer and safe‑init analysis logic unchanged with new patterns
relevantInitializerMethodOrBlocknow usesmethodLambdaOrBlock instanceof MethodTree methodTreebut preserves the existing constructor/initializer selection and the partially‑marked class check viacodeAnnotationInfo.isClassNullAnnotated(...).In
safeInitByCalleeBefore,BlockTreeandTryTreeare pattern‑bound (BlockTree bt,TryTree tryTree) but the algorithm—scan statements up to the read, collect safe‑init method invocations, and recurse intotry/finallyblocks without catches—remains identical.In
computeTree2Init, pattern bindings forBlockTreeandMethodTreeleave the ordering and accumulation of initialized fields exactly as before.No new control‑flow paths or dataflow decisions were introduced; just cast elimination.
Also applies to: 1368-1432
2373-2403: Safe‑initializer detection still matches the original criteria
getSafeInitMethodsnow pattern‑matchesTryTree tryTree, but still treats try/finally without catches as “top‑level” for purposes of discovering safe init methods in bothtryandfinallybodies.
getInvokeOfSafeInitMethod’s matcher usesexpressionTree instanceof MethodInvocationTree methodInvocationTreeand the outerstmt instanceof ExpressionStatementTree expressionStatementTreepattern; it keeps the same constraints: private/final/non‑native, non‑static methods on the same enclosing class withthis(or implicit) receiver.The refactor doesn’t broaden or narrow what counts as a safe initializer.
Also applies to: 2414-2446
2395-2403:isThisIdentifierand its matcher refactor are safeThe helper now uses:
return expressionTree instanceof IdentifierTree identifierTree && identifierTree.getName().toString().equals("this");and
isThisIdentifierMatchersimply delegates to it. This is equivalent to the previous implementation and continues to correctly recognizethisreceivers.Also applies to: 2795-2803
nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java (1)
161-165: ErrorBuilder pattern uses are straightforward and behavior‑neutral
canHaveSuppressWarningsAnnotationnow bindsClassTree classTreeinline before checking that its simple name is non‑empty; the set of trees considered suppressible is the same.
addSuppressWarningsFix’s modifiers selection usessuggestTree instanceof MethodTree methodTreeto avoid a separate cast, still falling back toVariableTreeotherwise; callers continue to pass only methods/fields.
symbolIsExcludedClassSymbolnow bindsSymbol.ClassSymbol classSymbolbefore streaming annotation mirrors; the exclusion condition is identical.These are all mechanical refactors without semantic impact.
Also applies to: 290-308, 438-446
nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java (1)
331-359: AccessPath pattern‑matching refactor keeps dataflow semantics intact
getAccessPathForNodenow dispatches via pattern‑matchedLocalVariableNode,FieldAccessNode,MethodInvocationNode, andArrayAccessNode, but the mapping tofromLocal/fromFieldAccess/fromMethodCall/fromArrayAccessis unchanged.
getElementFromArrayNodebindsLocalVariableNode/FieldAccessNodedirectly, returning the same elements as before; non‑array roots still yieldnull.In
buildAccessPathRecursive, theFieldAccessNode,ArrayAccessNode,MethodInvocationNode, andLocalVariableNodebranches use bound variables (fieldAccess,arrayAccess,invocation,localVariableNode) in place of casts. The logic for static vs instance fields, array index handling (integer vs variable index), constant‑argument extraction, and AP root selection remains the same.
StringMapKey.equalsandNumericMapKey.equalsnow compare against pattern‑bound keys while preserving the same equality condition.Given the centrality of
AccessPathto nullness tracking, it’s good that these changes are strictly syntactic.Also applies to: 407-554, 654-674, 677-703
nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java (1)
92-98: OptionalEmptinessHandler pattern bindings are safe and align with intent
onOverrideMayBeNullExprandonExpressionDereferencenow bindSymbol.MethodSymbol methodSymboldirectly whenexpr/symbolis a method invocation, but still require the same shapes before callingoptionalIsGetCall.
getNodeWrappedByAssertThatandmaybeUnwrapBooleanValueOfuse pattern‑matchedMethodInvocationNodeinstances while preserving their original control flow (single‑argument checks,assertThatrecognition, Boolean.valueOf unwrapping).These refactors keep the Optional emptiness reasoning exactly as before.
Also applies to: 139-152, 232-244, 251-261
nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java (1)
275-287: Fix inconsistent variable naming.The variable name
annonClassBodyon line 276 has inconsistent spelling compared to the same variable at lines 259 and 320, which useanonClassBody(single 'n'). This should be corrected for consistency.🔎 Proposed fix
if (argTree instanceof NewClassTree newClassTree) { - ClassTree annonClassBody = newClassTree.getClassBody(); + ClassTree anonClassBody = newClassTree.getClassBody(); // Ensure that this `new B() ...` has a custom class body, otherwise, we skip for now. - if (annonClassBody != null) { + if (anonClassBody != null) { MapLikeMethodRecord methodRecord = streamType.getMaplikeMethodRecord(methodSymbol); handleMapOrCollectAnonClassBody( - methodRecord, annonClassBody, t -> observableCallToInnerMethodOrLambda.put(tree, t)); + methodRecord, anonClassBody, t -> observableCallToInnerMethodOrLambda.put(tree, t)); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (23)
jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.javanullaway/src/main/java/com/uber/nullaway/NullAway.javanullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.javanullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.javanullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.javanullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.javanullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.javanullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/MethodAnalysisContext.javanullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.javanullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.javanullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.javanullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.javanullaway/src/test/java/com/uber/nullaway/tools/FieldInitDisplay.javanullaway/src/test/java/com/uber/nullaway/tools/version1/ErrorDisplayV1.java
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.
📚 Learning: 2025-08-28T04:54:20.953Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.javanullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.javanullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.javanullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.javajar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.javanullaway/src/main/java/com/uber/nullaway/handlers/MethodAnalysisContext.javanullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.javanullaway/src/main/java/com/uber/nullaway/NullAway.java
📚 Learning: 2025-10-29T23:56:18.236Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.javajar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.javanullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.javanullaway/src/main/java/com/uber/nullaway/NullAway.java
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.javanullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/NullAway.javanullaway/src/test/java/com/uber/nullaway/tools/version1/ErrorDisplayV1.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build spring-framework with snapshot
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build caffeine with snapshot
- GitHub Check: Build and test on ubuntu-latest
🔇 Additional comments (68)
nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java (1)
42-42: LGTM! Clean refactoring to pattern matching instanceof.The refactoring correctly applies Java pattern matching, eliminating the explicit cast while preserving the same logic. The bound variable
otheris appropriately named and used correctly in the subsequent comparisons.nullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.java (1)
61-61: LGTM! Clean modernization to pattern matching.The refactoring correctly uses Java pattern matching for
instanceof, eliminating the explicit cast and reducing boilerplate. The variable scope and all downstream uses remain unchanged, ensuring no behavioral impact.nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java (1)
103-105: LGTM! Pattern matching correctly applied.The refactoring correctly replaces the explicit cast with inline pattern matching. The variable
varSymbolis properly scoped to the if block and maintains identical behavior.nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java (1)
106-106: LGTM! Clean adoption of pattern matching forinstanceof.The refactor correctly replaces the traditional
instanceof+ cast idiom with Java's pattern matching syntax, improving readability and eliminating the redundant cast operation.nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java (2)
57-57: LGTM! Pattern matching improves readability.The refactor to use pattern matching for
instanceofin the ternary expression is correct and eliminates the need for an explicit cast.
63-64: LGTM! Pattern matching simplifies the equals implementation.The refactor correctly combines the
instanceofcheck and cast into a single pattern-matching expression, improving code clarity without changing behavior.nullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.java (2)
132-132: LGTM! Clean pattern matching refactor.The traditional
instanceofcheck with explicit cast has been correctly replaced with pattern matching, eliminating boilerplate and improving readability. The pattern variableiteratorContentsKeyis properly scoped and used on line 133.
209-209: LGTM! Clean pattern matching refactor in equals().The traditional
instanceofcheck with explicit cast has been correctly replaced with pattern matching. The pattern variableotheris idiomatic forequals()methods and properly used on line 212.nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java (1)
83-85: LGTM!Clean use of pattern matching for
instanceof. The pattern variablethatis correctly scoped for the subsequent field comparisons, and this is semantically equivalent to the traditional check-then-cast idiom.nullaway/src/test/java/com/uber/nullaway/tools/version1/ErrorDisplayV1.java (1)
79-81: LGTM!Consistent application of pattern matching for
instanceof, matching the style used inErrorDisplay.java. The refactor is correct and maintains equivalent semantics.nullaway/src/test/java/com/uber/nullaway/tools/FieldInitDisplay.java (1)
54-56: LGTM!Correct application of pattern matching for
instanceof. The pattern variablethatis properly scoped and used in the subsequentObjects.equalscomparisons.nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java (4)
93-94: LGTM! Pattern matching correctly applied.The instanceof check with pattern binding eliminates the need for an explicit cast and improves code readability.
144-145: LGTM! Pattern matching correctly applied.The instanceof check with pattern binding eliminates the need for an explicit cast and improves code readability.
234-234: LGTM! Pattern matching correctly applied.The instanceof check with pattern binding eliminates the need for an explicit cast and improves code readability.
254-255: LGTM! Pattern matching correctly applied.The instanceof check with pattern binding eliminates the need for an explicit cast and improves code readability.
nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java (3)
135-135: LGTM! Clean pattern matching refactor.The pattern matching eliminates the explicit cast while maintaining identical control flow and logic.
275-275: LGTM! Clean pattern matching refactor.The pattern matching eliminates the explicit cast while maintaining identical control flow and logic.
302-319: LGTM! Clean pattern matching refactor.The pattern matching eliminates explicit casts in both the if and else-if branches while maintaining identical control flow and logic.
nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java (2)
178-179: LGTM! Clean pattern matching refactor.The combined instanceof checks with pattern binding for
methodSymbolcorrectly preserve the original logic while improving readability. The pattern variable is properly scoped and used throughout the method.
289-289: LGTM! Clean pattern matching refactor.The pattern matching instanceof for
Symbol.VarSymbolcorrectly eliminates the explicit cast while preserving the original logic. The pattern variablevarSymbolis properly used within its scope.nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java (1)
302-302: LGTM! Clean refactoring to pattern matching instanceof.The change correctly applies Java pattern matching for
instanceof, eliminating the explicit cast and making the code more concise. No behavioral changes.nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java (5)
333-344: LGTM!The pattern matching refactor is semantically equivalent to the previous
instanceof+ cast pattern. The pattern variables (localVariableNode,fieldAccessNode,methodInvocationNode,arrayAccessNode) are correctly scoped within their respective branches.
352-358: LGTM!Clean pattern matching transformation for extracting elements from array nodes.
413-528: LGTM!The pattern matching transformations in this complex recursive method are all correct:
- Compound condition at line 484-485 (
symbol instanceof Symbol.VarSymbol varSymbol && ...) correctly scopesvarSymbolfor use throughout theifblock.- The remaining traditional cast at line 461 is appropriate since it operates on a
Treetype rather than aNodetype.
664-668: LGTM!The pattern matching in the
equalsmethod is idiomatic and correctly compares thekeyfield.
693-697: LGTM!The pattern matching transformation correctly handles the primitive
longkey comparison.nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java (1)
147-147: LGTM! Clean refactoring to pattern matching for instanceof.All instanceof checks have been correctly refactored to use Java pattern matching, eliminating explicit casts and improving code readability. The pattern-bound variables are appropriately scoped and named, and there are no behavioral changes—this is purely a syntactic modernization that aligns with the PR objective of enabling the PatternMatchingInstanceOf check.
Also applies to: 234-234, 321-323, 534-539, 564-566, 681-684, 868-868, 1085-1085, 1536-1536, 1562-1563, 1568-1568, 1659-1659, 1683-1683, 1801-1806, 1815-1816, 2039-2040
jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.java (2)
200-218: LGTM! Clean refactoring to pattern matching.The transformation from traditional instanceof+cast to pattern matching is correct and improves readability. All bound variables (
ssaGetInstruction,ssaPutInstruction,callInst) are used consistently within their scopes, and the control flow logic is preserved exactly.
260-260: LGTM! Correct pattern matching transformation.The refactoring eliminates the explicit cast while preserving the exact same logic. The bound variable
retInstris used correctly on line 261.nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (3)
125-125: LGTM!The pattern matching conversion correctly eliminates the need for an explicit cast. The pattern variable
msymis properly scoped for use after the negated guard condition.
252-253: LGTM!The pattern matching simplifies the String cast, eliminating the redundant explicit cast while preserving the original logic.
698-699: LGTM!The pattern matching in the ternary operator correctly eliminates the explicit cast for the
JCMethodInvocationcase while preserving the existing logic for both branches.nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java (8)
203-207: LGTM! Clean pattern matching refactor.The conversion from traditional instanceof + cast to pattern matching is correct and improves readability.
523-527: LGTM! Pattern matching correctly applied.The bound variable
localVariableNodeis used consistently throughout the block, correctly replacing the previous cast-based pattern.
529-531: LGTM! Correct pattern matching usage.
533-547: LGTM! Pattern matching refactor preserves logic.The bound variable
fieldAccessNodeis used consistently throughout the conditional block, correctly replacing multiple implicit casts.
592-614: LGTM! Multiple pattern matching refactors correctly applied.Both instanceof checks (for
MethodInvocationNodeat line 592 andLocalVariableNodeat line 597) are correctly converted to pattern matching with appropriate variable bindings used throughout their respective scopes.
624-631: LGTM! Pattern matching correctly replaces cast.The bound variable
baseInvocationis appropriately used throughout the conditional block.
931-937: LGTM! Pattern matching with negation correctly applied.The pattern matching variable
notEqualNodeis correctly bound and used only when the instanceof check succeeds (after the early return condition).
1017-1020: LGTM! Pattern matching simplifies return logic.The bound variable
localVariableNodeis correctly returned without needing an explicit cast.nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java (7)
258-269: LGTM! Clean pattern matching refactor.The conversion to pattern matching for
NewClassTreeandLambdaExpressionTreeis correct and improves readability.
303-340: LGTM! Nested pattern matching handled correctly.The pattern matching refactor correctly handles both the outer
MethodInvocationTreecheck and the nestedNewClassTreecheck. Logic is preserved.
345-350: LGTM! Chained pattern matching refactor is correct.The nested pattern matching for
MemberSelectTreeandMethodInvocationTreecorrectly preserves the original logic.
399-404: LGTM! Pattern matching refactor is correct.The conversion to pattern matching for
MethodTreeis correct and preserves the original logic.
424-428: LGTM! Pattern matching with compound condition is correct.The pattern matching refactor correctly handles the compound condition check for
MethodTree.
509-520: LGTM! Pattern matching refactor is correct.The conversion to pattern matching for
LiteralTreeis correct and preserves the original logic.
569-580: LGTM! Pattern matching refactor is correct.The two pattern matching conversions for
MethodTreeare correct and preserve the original logic. The reuse of the variable namemethodTreein different scopes is appropriate.nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java (4)
43-45: LGTM: Pattern matching refactoring is correct.The conversion from explicit cast to pattern matching properly binds
declaredTypeand uses it within the ternary expression's true branch.
67-69: LGTM: Pattern matching refactoring is correct.The conversion eliminates the explicit cast by binding
declaredTypein the instanceof check, properly using it in the subsequent method call.
159-164: LGTM: Pattern matching refactoring is correct.The negated instanceof check with pattern binding properly eliminates the explicit cast, and
wildcardTypeis correctly used to access the type field.
229-233: LGTM: Pattern matching refactoring is correct.The conversion properly binds
otherArrayTypein the negated instanceof check and uses it to access the elemtype field, eliminating the explicit cast.Note: A previous review comment flagged a minor formatting inconsistency regarding blank line spacing compared to
visitWildcardType. Refer to that comment if the formatting concern remains.nullaway/src/main/java/com/uber/nullaway/handlers/MethodAnalysisContext.java (1)
57-57: LGTM! Clean refactor to pattern matching instanceof.The refactor correctly combines the type check with pattern binding, eliminating the need for an explicit cast. The pattern variable
thatis appropriately scoped and used in the subsequent equality checks.nullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.java (1)
72-72: LGTM! Pattern matching simplifies type check and eliminates redundant cast.The refactor correctly binds the pattern variable
methodSymbolin the instanceof check, which is then used throughout the method body without requiring an explicit cast.nullaway/src/main/java/com/uber/nullaway/NullAway.java (15)
335-346: LGTM! Pattern matching refactor correctly eliminates cast.The pattern variable
methodInvokeis properly scoped and used for the method symbol retrieval.
422-429: LGTM! Clear pattern matching in conditional branch.The pattern binding correctly handles the case where the enclosing element is a MethodTree, eliminating the need for an explicit cast.
517-525: LGTM! Nested pattern matching improves readability.The nested pattern matching correctly handles the type checks for both the statement and its expression, eliminating redundant casts and making the intent clearer.
1249-1254: LGTM! Pattern matching with compound conditions.The refactor correctly combines instanceof checks with additional boolean conditions, maintaining the original logic while eliminating explicit casts.
1292-1318: LGTM! Pattern matching simplifies conditional logic.The instanceof check with pattern binding is properly used within the conditional branch, eliminating the need for an explicit cast.
1381-1383: LGTM! Pattern matching in ternary expression.The pattern variable
btis correctly scoped to the true branch of the ternary, eliminating one cast while the false branch appropriately retains the explicit MethodTree cast.
1406-1423: LGTM! Pattern matching in loop body.The pattern variable
tryTreeis properly scoped and used within the conditional block.
1474-1489: LGTM! Pattern matching in iteration.Both pattern bindings (
blockTreeandmethodTree) are correctly scoped to their respective conditional blocks.
1524-1556: LGTM! Pattern matching across multiple conditional branches.The refactoring correctly uses pattern variables (
assignment,binaryTree,methodInvoke) in each respective branch, eliminating redundant casts throughout the method.
1632-1638: LGTM! Pattern matching with negated instanceof.The early return pattern with negated instanceof correctly uses pattern matching, making the positive case clearer below.
2382-2391: LGTM! Pattern matching in loop.The pattern variable
tryTreeis appropriately scoped and used within the conditional block.
2409-2433: LGTM! Pattern matching in lambda expression.The pattern variable
methodInvocationTreeis correctly scoped to the lambda body and eliminates an explicit cast.
2429-2434: LGTM! Pattern matching simplifies statement check.The pattern variable
expressionStatementTreeis properly used to access the expression.
2439-2442: LGTM! Pattern matching in helper method.The pattern variable
expressionStatementTreeis correctly scoped and used.
2786-2787: LGTM! Elegant pattern matching in boolean expression.The pattern variable
identifierTreeis used within a compound boolean expression in a return statement, demonstrating clean use of pattern matching to eliminate redundant instanceof checks.
Should be no behavior changes
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.