diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index a8de8ef9f2..2f4642a9f9 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -87,6 +87,17 @@ public interface LibraryModels { */ ImmutableSetMultimap nullImpliesFalseParameters(); + /** + * Get (method, target method) pairs where returning true 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 ensuresNonNullIfTrueMethodCalls() { + return ImmutableSetMultimap.of(); + } + /** * Get (method, parameter) pairs that cause the method to return null when passed * null on that parameter. diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 8858efb369..09e1c8f2cf 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -292,8 +292,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 nullImpliesNullIndexes = optLibraryModels.nullImpliesNullParameters(callee); @@ -349,10 +348,11 @@ private boolean isNullableFieldInLibraryModels(@Nullable Symbol symbol) { private void setConditionalArgumentNullness( AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, - List arguments, + MethodInvocationNode node, Symbol.MethodSymbol callee, VisitorState state, AccessPath.AccessPathContext apContext) { + List arguments = node.getArguments(); OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context); ImmutableSet nullImpliesTrueParameters = optLibraryModels.nullImpliesTrueParameters(callee); @@ -366,6 +366,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 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; } private static List accessPathsAtIndexes( @@ -973,6 +1035,14 @@ private static class DefaultLibraryModels implements LibraryModels { 0) .build(); + private static final ImmutableSetMultimap + ENSURES_NONNULL_IF_TRUE_METHOD_CALLS = + new ImmutableSetMultimap.Builder() + .put( + methodRef("java.lang.Class", "isArray()"), + methodRef("java.lang.Class", "getComponentType()")) + .build(); + private static final ImmutableSetMultimap NULL_IMPLIES_NULL_PARAMETERS = new ImmutableSetMultimap.Builder() .put(methodRef("java.lang.Class", "cast(java.lang.Object)"), 0) @@ -1179,6 +1249,11 @@ public ImmutableSetMultimap nullImpliesFalseParameters() { return NULL_IMPLIES_FALSE_PARAMETERS; } + @Override + public ImmutableSetMultimap ensuresNonNullIfTrueMethodCalls() { + return ENSURES_NONNULL_IF_TRUE_METHOD_CALLS; + } + @Override public ImmutableSetMultimap nullImpliesNullParameters() { return NULL_IMPLIES_NULL_PARAMETERS; @@ -1241,6 +1316,8 @@ private static class CombinedLibraryModels implements LibraryModels { private final ImmutableSetMultimap nullImpliesFalseParameters; + private final ImmutableSetMultimap ensuresNonNullIfTrueMethodCalls; + private final ImmutableSetMultimap nullImpliesNullParameters; private final ImmutableSet nullableReturns; @@ -1280,6 +1357,8 @@ private static class CombinedLibraryModels implements LibraryModels { new ImmutableSetMultimap.Builder<>(); ImmutableSetMultimap.Builder nullImpliesFalseParametersBuilder = new ImmutableSetMultimap.Builder<>(); + ImmutableSetMultimap.Builder ensuresNonNullIfTrueMethodCallsBuilder = + new ImmutableSetMultimap.Builder<>(); ImmutableSetMultimap.Builder nullImpliesNullParametersBuilder = new ImmutableSetMultimap.Builder<>(); ImmutableSet.Builder nullableReturnsBuilder = new ImmutableSet.Builder<>(); @@ -1325,6 +1404,13 @@ private static class CombinedLibraryModels implements LibraryModels { } nullImpliesFalseParametersBuilder.put(entry); } + for (Map.Entry entry : + libraryModels.ensuresNonNullIfTrueMethodCalls().entries()) { + if (shouldSkipModel(entry.getKey())) { + continue; + } + ensuresNonNullIfTrueMethodCallsBuilder.put(entry); + } for (Map.Entry entry : libraryModels.nullImpliesNullParameters().entries()) { if (shouldSkipModel(entry.getKey())) { @@ -1379,6 +1465,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(); @@ -1427,6 +1514,11 @@ public ImmutableSetMultimap nullImpliesFalseParameters() { return nullImpliesFalseParameters; } + @Override + public ImmutableSetMultimap ensuresNonNullIfTrueMethodCalls() { + return ensuresNonNullIfTrueMethodCalls; + } + @Override public ImmutableSetMultimap nullImpliesNullParameters() { return nullImpliesNullParameters; @@ -1513,6 +1605,7 @@ boolean nameNotPresent(Symbol.MethodSymbol symbol) { private final NameIndexedMap> nonNullParams; private final NameIndexedMap> nullImpliesTrueParams; private final NameIndexedMap> nullImpliesFalseParams; + private final NameIndexedMap> ensuresNonNullIfTrueMethodCalls; private final NameIndexedMap> nullImpliesNullParams; private final NameIndexedMap nullableRet; private final NameIndexedMap nonNullRet; @@ -1523,19 +1616,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()); } @@ -1568,6 +1662,10 @@ ImmutableSet nullImpliesFalseParameters(Symbol.MethodSymbol symbol) { return lookupImmutableSet(symbol, nullImpliesFalseParams); } + ImmutableSet ensuresNonNullIfTrueMethodCalls(Symbol.MethodSymbol symbol) { + return lookupImmutableSet(symbol, ensuresNonNullIfTrueMethodCalls); + } + ImmutableSet nullImpliesNullParameters(Symbol.MethodSymbol symbol) { return lookupImmutableSet(symbol, nullImpliesNullParams); } @@ -1587,15 +1685,15 @@ ImmutableSetMultimap nestedAnnotationsForMethods( return (result == null) ? ImmutableSetMultimap.of() : result; } - private ImmutableSet lookupImmutableSet( - Symbol.MethodSymbol symbol, NameIndexedMap> lookup) { - ImmutableSet result = lookup.get(symbol); + private ImmutableSet lookupImmutableSet( + Symbol.MethodSymbol symbol, NameIndexedMap> lookup) { + ImmutableSet result = lookup.get(symbol); return (result == null) ? ImmutableSet.of() : result; } - private NameIndexedMap> makeOptimizedIntSetLookup( - Names names, ImmutableSetMultimap ref2Ints) { - return makeOptimizedLookup(names, ref2Ints.keySet(), ref2Ints::get); + private NameIndexedMap> makeOptimizedSetLookup( + Names names, ImmutableSetMultimap ref2Set) { + return makeOptimizedLookup(names, ref2Set.keySet(), ref2Set::get); } private NameIndexedMap makeOptimizedBoolLookup( diff --git a/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java b/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java index 21ca4b00f4..b08ca8275e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java @@ -278,6 +278,28 @@ int classIsInstance(@Nullable String s) { .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 diff --git a/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java b/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java index f822244e3e..523a540c79 100644 --- a/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java @@ -96,6 +96,31 @@ void testNonNullContents(List list) { .doTest(); } + @Test + public void defaultLibraryModelsClassIsArray() { + makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + List.of("-XepOpt:NullAway:AnnotatedPackages=foo"))) + .addSourceLines( + "Test.java", + """ + package foo; + import org.jspecify.annotations.NullMarked; + @NullMarked + 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 biConsumerNullableUpperBound() { makeHelper() diff --git a/test-java-lib/src/main/java/com/uber/lib/unannotated/CustomInterface.java b/test-java-lib/src/main/java/com/uber/lib/unannotated/CustomInterface.java new file mode 100644 index 0000000000..0c9a9772d3 --- /dev/null +++ b/test-java-lib/src/main/java/com/uber/lib/unannotated/CustomInterface.java @@ -0,0 +1,7 @@ +package com.uber.lib.unannotated; + +public interface CustomInterface { + boolean hasContent(); + + Object getContent(); +} diff --git a/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java b/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java index 7a470d4fbf..2f2fa311ac 100644 --- a/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java +++ b/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java @@ -96,13 +96,21 @@ public ImmutableSetMultimap nullImpliesNullParameters() { return ImmutableSetMultimap.of(); } + @Override + public ImmutableSetMultimap ensuresNonNullIfTrueMethodCalls() { + return ImmutableSetMultimap.of( + methodRef("com.uber.lib.unannotated.CustomInterface", "hasContent()"), + methodRef("com.uber.lib.unannotated.CustomInterface", "getContent()")); + } + @Override public ImmutableSet 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