Skip to content

Commit

Permalink
null analysis should merge null contracts from equivalent super methods
Browse files Browse the repository at this point in the history
fixes eclipse-jdt#2325

+ if method choice would be arbitrary prefer more specific nullness
  • Loading branch information
stephan-herrmann committed Apr 11, 2024
1 parent 8fd8594 commit dcbece8
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,26 @@ private static TypeBinding mergeTypeAnnotations(TypeBinding type, TypeBinding ot
return mainType;
}

/**
* Help Scope.mostSpecificMethodBinding(MethodBinding[], int, TypeBinding[], InvocationSite, ReferenceBinding):
* If choice between equivalent methods would otherwise be arbitrary, determine if m1 should be preferred due
* to a more specific null contract.
*/
public static boolean hasMoreSpecificNullness(MethodBinding m1, MethodBinding m2) {
long nullness1 = m1.returnType.tagBits & TagBits.AnnotationNullMASK;
long nullness2 = m2.returnType.tagBits & TagBits.AnnotationNullMASK;
if (nullness1 == TagBits.AnnotationNonNull && nullness2 != TagBits.AnnotationNonNull)
return true;
int len = Math.max(m1.parameters.length, m2.parameters.length);
for (int i=0; i<len; i++) {
nullness1 = m1.parameters[i].tagBits & TagBits.AnnotationNullMASK;
nullness2 = m2.parameters[i].tagBits & TagBits.AnnotationNullMASK;
if (nullness1 == TagBits.AnnotationNullable && nullness2 != TagBits.AnnotationNullable)
return true;
}
return false;
}

@Override
public String toString() {
if (this == NULL_ANNOTATIONS_OK) return "OK"; //$NON-NLS-1$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4849,6 +4849,7 @@ public void acceptPotentiallyCompatibleMethods(MethodBinding[] methods) {/* igno
MethodBinding current = moreSpecific[i];
if (current != null) {
ReferenceBinding[] mostSpecificExceptions = null;
MethodBinding mostSpecificNullness = current;
MethodBinding original = current.original();
boolean shouldIntersectExceptions = original.declaringClass.isAbstract() && original.thrownExceptions != Binding.NO_EXCEPTIONS; // only needed when selecting from interface methods
for (int j = 0; j < visibleSize; j++) {
Expand Down Expand Up @@ -4909,6 +4910,10 @@ public void acceptPotentiallyCompatibleMethods(MethodBinding[] methods) {/* igno
}
// continue with original 15.12.2.5
}
if (compilerOptions().isAnnotationBasedNullAnalysisEnabled
&& NullAnnotationMatching.hasMoreSpecificNullness(next, mostSpecificNullness)) {
mostSpecificNullness = next;
}
if (shouldIntersectExceptions && original2.declaringClass.isInterface()) {
if (current.thrownExceptions != next.thrownExceptions) {
if (next.thrownExceptions == Binding.NO_EXCEPTIONS) {
Expand Down Expand Up @@ -4950,7 +4955,7 @@ public void acceptPotentiallyCompatibleMethods(MethodBinding[] methods) {/* igno
if (mostSpecificExceptions != null && mostSpecificExceptions != current.thrownExceptions) {
return new MostSpecificExceptionMethodBinding(current, mostSpecificExceptions);
}
return current;
return mostSpecificNullness;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19316,4 +19316,39 @@ public class DMessageHandlerRegistryImpl<@NonNull C extends DConnection>
TypeDeclaration.TESTING_GH_2158 = false;
}
}
public void testGH2325() {
Runner runner = new Runner();
runner.customOptions = getCompilerOptions();
runner.customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.IGNORE);
runner.testFiles = new String[] {
"Sample.java",
"""
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
interface InterfaceA {
@Nullable Object get();
}
interface InterfaceB {
@NonNull Object get();
}
interface InterfaceAB extends InterfaceA, InterfaceB {}
interface InterfaceBA extends InterfaceB, InterfaceA {}
class Sample {
void ab(InterfaceAB ab) {
@NonNull Object obj = ab.get();
// ^^^^^^^^
// ⚠ Null type mismatch (type annotations): required '@NonNull Object' but this expression has type '@Nullable Object'
// Expected: no "Null type mismatch" problem,
// because the union of the two null constraints has to be @Nullable, the most restrictive null constraint
// (@Nullable violates the null constraint given by InterfaceB; @NonNull fulfills both null constraints from InterfaceA and InterfaceB)
}
void ba(InterfaceBA ba) {
@NonNull Object obj = ba.get(); // (no "Null type mismatch" as expected)
}
}
"""
};
runner.classLibraries = this.LIBS;
runner.runConformTest();
}
}

0 comments on commit dcbece8

Please sign in to comment.