Skip to content

Support method-based conditional library model postconditions #1677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Shankar-v27 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support method-based conditional library model postconditions #1677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/LibraryModels.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ public interface LibraryModels {
*/
ImmutableSetMultimap<MethodRef, Integer> nullImpliesFalseParameters();

/**
* Get (method, target method) pairs where returning <code>true</code> implies the target method
* on the receiver is non-null.
*
* @return map from querying methods to target receiver methods that are non-null when returning
* true.
*/
default ImmutableSetMultimap<MethodRef, MethodRef> ensuresNonNullIfTrueMethodCalls() {
return ImmutableSetMultimap.of();
}
Comment on lines +90 to +99

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the conditional-model contract.

The Javadoc states that a false result implies that the target method returns null. The model name and implementation encode the opposite implication: a null target result implies false. The true-branch non-null result follows by contrapositive.

Proposed fix 
- * Get (method, target method) pairs where returning <code>false</code> implies the target method
- * on the receiver is <code>null</code> (and returning <code>true</code> implies the target method
- * on the receiver is non-null).
+ * Get (querying method, target receiver method) pairs where a <code>null</code> target result
+ * implies that the querying method returns <code>false</code>.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Get (method, target method) pairs where returning <code>false</code> implies the target method
* on the receiver is <code>null</code> (and returning <code>true</code> implies the target method
* on the receiver is non-null).
*
* @return map from querying methods to target receiver methods that are non-null when returning
* true.
*/
default ImmutableSetMultimap<MethodRef, MethodRef> nullImpliesFalseMethodCalls() {
return ImmutableSetMultimap.of();
}
/**
* Get (querying method, target receiver method) pairs where a <code>null</code> target result
* implies that the querying method returns <code>false</code>.
*
* `@return` map from querying methods to target receiver methods that are non-null when returning
* true.
*/
default ImmutableSetMultimap<MethodRef, MethodRef> nullImpliesFalseMethodCalls() {
return ImmutableSetMultimap.of();
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nullaway/src/main/java/com/uber/nullaway/LibraryModels.java` around lines 90
- 100, Correct the contract for nullImpliesFalseMethodCalls: update its Javadoc
to describe that a null target-method result implies the querying method returns
false, with the non-null result implying true by contrapositive. Keep the method
name and default empty ImmutableSetMultimap implementation unchanged.


/**
* Get (method, parameter) pairs that cause the method to return <code>null</code> when passed
* <code>null</code> on that parameter.
Expand Down
135 changes: 117 additions & 18 deletions nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,7 @@ public NullnessHint onDataflowVisitMethodInvocation(
boolean isMethodAnnotated =
!getCodeAnnotationInfo(state.context).isSymbolUnannotated(callee, this.config, mainHandler);
setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee, state, apContext);
setConditionalArgumentNullness(
thenUpdates, elseUpdates, node.getArguments(), callee, state, apContext);
setConditionalArgumentNullness(thenUpdates, elseUpdates, node, callee, state, apContext);
OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
ImmutableSet<Integer> nullImpliesNullIndexes =
optLibraryModels.nullImpliesNullParameters(callee);
Expand Down Expand Up @@ -350,10 +349,11 @@ private boolean isNullableFieldInLibraryModels(@Nullable Symbol symbol) {
private void setConditionalArgumentNullness(
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
List<Node> arguments,
MethodInvocationNode node,
Symbol.MethodSymbol callee,
VisitorState state,
AccessPath.AccessPathContext apContext) {
Comment on lines 349 to 355

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add Javadoc for the changed non-trivial private methods.

Both methods implement conditional library-model dataflow behavior without method documentation.

  • nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java#L350-L356: document branch updates, inputs, and receiver-method refinement.
  • nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java#L390-L404: document hierarchy lookup behavior and nullable return semantics.

As per coding guidelines, “Add Javadoc for every non-trivial method, including private methods.”

📍 Affects 1 file
  • nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java#L350-L356 (this comment)
  • nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java#L390-L404
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java`
around lines 350 - 356, Add Javadoc for the private method
setConditionalArgumentNullness in
nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
lines 350-356, documenting its inputs, branch updates, and receiver-method
refinement. Also document the non-trivial private method at lines 390-404 in the
same file, describing its hierarchy lookup behavior and nullable return
semantics.

Source: Coding guidelines

List<Node> arguments = node.getArguments();
OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
ImmutableSet<Integer> nullImpliesTrueParameters =
optLibraryModels.nullImpliesTrueParameters(callee);
Expand All @@ -367,6 +367,68 @@ private void setConditionalArgumentNullness(
accessPathsAtIndexes(nullImpliesFalseParameters, arguments, state, apContext)) {
thenUpdates.set(accessPath, NONNULL);
}
applyConditionalMethodCallUpdates(thenUpdates, node, callee, state, apContext);
}

/**
* Applies conditional updates for method calls on the receiver object when the method invocation
* returns {@code true} (e.g. for methods like {@code Class.isArray()}).
*
* @param thenUpdates updates for the then-branch
* @param node the method invocation node
* @param callee the method symbol of the callee
* @param state the visitor state
* @param apContext access path context
*/
private void applyConditionalMethodCallUpdates(
AccessPathNullnessPropagation.Updates thenUpdates,
MethodInvocationNode node,
Symbol.MethodSymbol callee,
VisitorState state,
AccessPath.AccessPathContext apContext) {
ImmutableSet<MethodRef> ensuresNonNullIfTrueMethodCalls =
getOptLibraryModels(state.context).ensuresNonNullIfTrueMethodCalls(callee);
if (!ensuresNonNullIfTrueMethodCalls.isEmpty()) {
Node receiver = node.getTarget().getReceiver();
if (receiver != null && callee.owner instanceof Symbol.ClassSymbol classSymbol) {
for (MethodRef targetRef : ensuresNonNullIfTrueMethodCalls) {
Symbol.MethodSymbol targetMethod = lookupMethodSymbol(classSymbol, targetRef, state);
if (targetMethod != null) {
AccessPath accessPath =
AccessPath.fromBaseAndElement(receiver, targetMethod, apContext);
if (accessPath != null) {
thenUpdates.set(accessPath, NONNULL);
}
}
}
}
}
}

/**
* Looks up a method symbol matching the given target method reference in the class or its
* supertypes.
*
* @param classSymbol class symbol in which to search for the method
* @param targetRef reference to the target method being searched
* @param state visitor state
* @return matching method symbol, or {@code null} if not found
*/
private static Symbol.@Nullable MethodSymbol lookupMethodSymbol(
Symbol.ClassSymbol classSymbol, MethodRef targetRef, VisitorState state) {
Name name = state.getName(targetRef.methodName);
Types types = state.getTypes();
for (Type s : types.closure(classSymbol.type)) {
for (Symbol m : s.tsym.members().getSymbolsByName(name)) {
if (!(m instanceof Symbol.MethodSymbol msym)) {
continue;
}
if (MethodRef.fromSymbol(msym).equals(targetRef)) {
return msym;
}
}
}
return null;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

private static List<AccessPath> accessPathsAtIndexes(
Expand Down Expand Up @@ -974,6 +1036,14 @@ private static class DefaultLibraryModels implements LibraryModels {
0)
.build();

private static final ImmutableSetMultimap<MethodRef, MethodRef>
ENSURES_NONNULL_IF_TRUE_METHOD_CALLS =
new ImmutableSetMultimap.Builder<MethodRef, MethodRef>()
.put(
methodRef("java.lang.Class", "isArray()"),
methodRef("java.lang.Class", "getComponentType()"))
.build();

private static final ImmutableSetMultimap<MethodRef, Integer> NULL_IMPLIES_NULL_PARAMETERS =
new ImmutableSetMultimap.Builder<MethodRef, Integer>()
.put(methodRef("java.lang.Class", "cast(java.lang.Object)"), 0)
Expand All @@ -992,6 +1062,7 @@ private static class DefaultLibraryModels implements LibraryModels {

private static final ImmutableSet<MethodRef> ALWAYS_NULLABLE_RETURNS =
new ImmutableSet.Builder<MethodRef>()
.add(methodRef("java.lang.Class", "getComponentType()"))
.add(methodRef("com.sun.source.tree.CompilationUnitTree", "getPackageName()"))
.add(methodRef("java.lang.Throwable", "getMessage()"))
.add(methodRef("java.lang.Throwable", "getLocalizedMessage()"))
Expand Down Expand Up @@ -1180,6 +1251,11 @@ public ImmutableSetMultimap<MethodRef, Integer> nullImpliesFalseParameters() {
return NULL_IMPLIES_FALSE_PARAMETERS;
}

@Override
public ImmutableSetMultimap<MethodRef, MethodRef> ensuresNonNullIfTrueMethodCalls() {
return ENSURES_NONNULL_IF_TRUE_METHOD_CALLS;
}

@Override
public ImmutableSetMultimap<MethodRef, Integer> nullImpliesNullParameters() {
return NULL_IMPLIES_NULL_PARAMETERS;
Expand Down Expand Up @@ -1242,6 +1318,8 @@ private static class CombinedLibraryModels implements LibraryModels {

private final ImmutableSetMultimap<MethodRef, Integer> nullImpliesFalseParameters;

private final ImmutableSetMultimap<MethodRef, MethodRef> ensuresNonNullIfTrueMethodCalls;

private final ImmutableSetMultimap<MethodRef, Integer> nullImpliesNullParameters;

private final ImmutableSet<MethodRef> nullableReturns;
Expand Down Expand Up @@ -1281,6 +1359,8 @@ private static class CombinedLibraryModels implements LibraryModels {
new ImmutableSetMultimap.Builder<>();
ImmutableSetMultimap.Builder<MethodRef, Integer> nullImpliesFalseParametersBuilder =
new ImmutableSetMultimap.Builder<>();
ImmutableSetMultimap.Builder<MethodRef, MethodRef> ensuresNonNullIfTrueMethodCallsBuilder =
new ImmutableSetMultimap.Builder<>();
ImmutableSetMultimap.Builder<MethodRef, Integer> nullImpliesNullParametersBuilder =
new ImmutableSetMultimap.Builder<>();
ImmutableSet.Builder<MethodRef> nullableReturnsBuilder = new ImmutableSet.Builder<>();
Expand Down Expand Up @@ -1326,6 +1406,13 @@ private static class CombinedLibraryModels implements LibraryModels {
}
nullImpliesFalseParametersBuilder.put(entry);
}
for (Map.Entry<MethodRef, MethodRef> entry :
libraryModels.ensuresNonNullIfTrueMethodCalls().entries()) {
if (shouldSkipModel(entry.getKey())) {
continue;
}
ensuresNonNullIfTrueMethodCallsBuilder.put(entry);
}
for (Map.Entry<MethodRef, Integer> entry :
libraryModels.nullImpliesNullParameters().entries()) {
if (shouldSkipModel(entry.getKey())) {
Expand Down Expand Up @@ -1380,6 +1467,7 @@ private static class CombinedLibraryModels implements LibraryModels {
nonNullParameters = nonNullParametersBuilder.build();
nullImpliesTrueParameters = nullImpliesTrueParametersBuilder.build();
nullImpliesFalseParameters = nullImpliesFalseParametersBuilder.build();
ensuresNonNullIfTrueMethodCalls = ensuresNonNullIfTrueMethodCallsBuilder.build();
nullImpliesNullParameters = nullImpliesNullParametersBuilder.build();
nullableReturns = nullableReturnsBuilder.build();
nonNullReturns = nonNullReturnsBuilder.build();
Expand Down Expand Up @@ -1428,6 +1516,11 @@ public ImmutableSetMultimap<MethodRef, Integer> nullImpliesFalseParameters() {
return nullImpliesFalseParameters;
}

@Override
public ImmutableSetMultimap<MethodRef, MethodRef> ensuresNonNullIfTrueMethodCalls() {
return ensuresNonNullIfTrueMethodCalls;
}

@Override
public ImmutableSetMultimap<MethodRef, Integer> nullImpliesNullParameters() {
return nullImpliesNullParameters;
Expand Down Expand Up @@ -1520,6 +1613,7 @@ boolean nameNotPresent(Symbol.MethodSymbol symbol) {
private final NameIndexedMap<ImmutableSet<Integer>> nonNullParams;
private final NameIndexedMap<ImmutableSet<Integer>> nullImpliesTrueParams;
private final NameIndexedMap<ImmutableSet<Integer>> nullImpliesFalseParams;
private final NameIndexedMap<ImmutableSet<MethodRef>> ensuresNonNullIfTrueMethodCalls;
private final NameIndexedMap<ImmutableSet<Integer>> nullImpliesNullParams;
private final NameIndexedMap<Boolean> nullableRet;
private final NameIndexedMap<Boolean> nonNullRet;
Expand All @@ -1530,19 +1624,20 @@ boolean nameNotPresent(Symbol.MethodSymbol symbol) {

OptimizedLibraryModels(LibraryModels models, Context context) {
Names names = Names.instance(context);
failIfNullParams = makeOptimizedIntSetLookup(names, models.failIfNullParameters());
failIfNullParams = makeOptimizedSetLookup(names, models.failIfNullParameters());
explicitlyNullableParams =
makeOptimizedIntSetLookup(names, models.explicitlyNullableParameters());
nonNullParams = makeOptimizedIntSetLookup(names, models.nonNullParameters());
nullImpliesTrueParams = makeOptimizedIntSetLookup(names, models.nullImpliesTrueParameters());
nullImpliesFalseParams =
makeOptimizedIntSetLookup(names, models.nullImpliesFalseParameters());
nullImpliesNullParams = makeOptimizedIntSetLookup(names, models.nullImpliesNullParameters());
makeOptimizedSetLookup(names, models.explicitlyNullableParameters());
nonNullParams = makeOptimizedSetLookup(names, models.nonNullParameters());
nullImpliesTrueParams = makeOptimizedSetLookup(names, models.nullImpliesTrueParameters());
nullImpliesFalseParams = makeOptimizedSetLookup(names, models.nullImpliesFalseParameters());
ensuresNonNullIfTrueMethodCalls =
makeOptimizedSetLookup(names, models.ensuresNonNullIfTrueMethodCalls());
nullImpliesNullParams = makeOptimizedSetLookup(names, models.nullImpliesNullParameters());
nullableRet = makeOptimizedBoolLookup(names, models.nullableReturns());
nonNullRet = makeOptimizedBoolLookup(names, models.nonNullReturns());
castToNonNullMethods = makeOptimizedIntSetLookup(names, models.castToNonNullMethods());
castToNonNullMethods = makeOptimizedSetLookup(names, models.castToNonNullMethods());
methodTypeVariablesWithNullableUpperBounds =
makeOptimizedIntSetLookup(names, models.methodTypeVariablesWithNullableUpperBounds());
makeOptimizedSetLookup(names, models.methodTypeVariablesWithNullableUpperBounds());
nestedAnnotationsForMethods =
makeOptimizedNestedAnnotationLookup(names, models.nestedAnnotationsForMethods());
}
Expand Down Expand Up @@ -1575,6 +1670,10 @@ ImmutableSet<Integer> nullImpliesFalseParameters(Symbol.MethodSymbol symbol) {
return lookupImmutableSet(symbol, nullImpliesFalseParams);
}

ImmutableSet<MethodRef> ensuresNonNullIfTrueMethodCalls(Symbol.MethodSymbol symbol) {
return lookupImmutableSet(symbol, ensuresNonNullIfTrueMethodCalls);
}

ImmutableSet<Integer> nullImpliesNullParameters(Symbol.MethodSymbol symbol) {
return lookupImmutableSet(symbol, nullImpliesNullParams);
}
Expand All @@ -1594,15 +1693,15 @@ ImmutableSetMultimap<Integer, NestedAnnotationInfo> nestedAnnotationsForMethods(
return (result == null) ? ImmutableSetMultimap.of() : result;
}

private ImmutableSet<Integer> lookupImmutableSet(
Symbol.MethodSymbol symbol, NameIndexedMap<ImmutableSet<Integer>> lookup) {
ImmutableSet<Integer> result = lookup.get(symbol);
private <T> ImmutableSet<T> lookupImmutableSet(
Symbol.MethodSymbol symbol, NameIndexedMap<ImmutableSet<T>> lookup) {
ImmutableSet<T> result = lookup.get(symbol);
return (result == null) ? ImmutableSet.of() : result;
}

private NameIndexedMap<ImmutableSet<Integer>> makeOptimizedIntSetLookup(
Names names, ImmutableSetMultimap<MethodRef, Integer> ref2Ints) {
return makeOptimizedLookup(names, ref2Ints.keySet(), ref2Ints::get);
private <T> NameIndexedMap<ImmutableSet<T>> makeOptimizedSetLookup(
Names names, ImmutableSetMultimap<MethodRef, T> ref2Set) {
return makeOptimizedLookup(names, ref2Set.keySet(), ref2Set::get);
}

private NameIndexedMap<Boolean> makeOptimizedBoolLookup(
Expand Down
43 changes: 43 additions & 0 deletions nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,49 @@ int classIsInstance(@Nullable String s) {
.doTest();
}

@Test
public void defaultLibraryModelsClassIsArray() {
defaultCompilationHelper
.addSourceLines(
"Test.java",
"""
package com.uber;
public class Test {
int classIsArray(Class<?> clazz) {
if (clazz.isArray()) {
return clazz.getComponentType().hashCode();
} else {
// BUG: Diagnostic contains: dereferenced
return clazz.getComponentType().hashCode();
}
}
}
""")
.doTest();
}

@Test
public void interfaceLibraryModelMethodCall() {
defaultCompilationHelper
.addSourceLines(
"Test.java",
"""
package com.uber;
import com.uber.lib.unannotated.CustomInterface;
public class Test {
int interfaceMethodCall(CustomInterface c) {
if (c.hasContent()) {
return c.getContent().hashCode();
} else {
// BUG: Diagnostic contains: dereferenced
return c.getContent().hashCode();
}
}
}
""")
.doTest();
}

@Test
public void checkForNullSupport() {
defaultCompilationHelper
Expand Down
7 changes: 7 additions & 0 deletions test-java-lib/src/main/java/com/uber/lib/unannotated/CustomInterface.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.uber.lib.unannotated;

public interface CustomInterface {
boolean hasContent();

Object getContent();
}
10 changes: 9 additions & 1 deletion test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,21 @@ public ImmutableSetMultimap<MethodRef, Integer> nullImpliesNullParameters() {
return ImmutableSetMultimap.of();
}

@Override
public ImmutableSetMultimap<MethodRef, MethodRef> ensuresNonNullIfTrueMethodCalls() {
return ImmutableSetMultimap.of(
methodRef("com.uber.lib.unannotated.CustomInterface", "hasContent()"),
methodRef("com.uber.lib.unannotated.CustomInterface", "getContent()"));
}

@Override
public ImmutableSet<MethodRef> nullableReturns() {
return ImmutableSet.of(
methodRef("com.uber.AnnotatedWithModels", "returnsNullFromModel()"),
methodRef("com.uber.lib.unannotated.UnannotatedWithModels", "returnsNullUnannotated()"),
methodRef("com.uber.lib.unannotated.UnannotatedWithModels", "returnsNullUnannotated2()"),
methodRef("com.uber.lib.unannotated.Box", "orElse(T)"));
methodRef("com.uber.lib.unannotated.Box", "orElse(T)"),
methodRef("com.uber.lib.unannotated.CustomInterface", "getContent()"));
}

@Override
Expand Down