null analysis should merge null contracts from equivalent super methods

+ safer approach, to ensure the selected method fulfills all criteria
stephan-herrmann · Apr 11, 2024 · f9f5117 · f9f5117
1 parent dcbece8
commit f9f5117
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 4 deletions.
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java
@@ -4849,7 +4849,6 @@ 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++) {
@@ -4911,8 +4910,14 @@ public void acceptPotentiallyCompatibleMethods(MethodBinding[] methods) {/* igno
 							// continue with original 15.12.2.5
 						}
 						if (compilerOptions().isAnnotationBasedNullAnalysisEnabled
-								&& NullAnnotationMatching.hasMoreSpecificNullness(next, mostSpecificNullness)) {
-							mostSpecificNullness = next;
+								&& j > i // don't go backwards
+								&& NullAnnotationMatching.hasMoreSpecificNullness(next, current))
+						{
+							// In this case we want to prefer next among equivalent methods.
+							// (the case where JLS 15.12.2.5 says "...is chosen arbitrarily...")
+							// To try if 'next' matches all criteria, skip outer loop to j (after increment):
+							i = j -1 ;
+							continue nextSpecific;
 						}
 						if (shouldIntersectExceptions && original2.declaringClass.isInterface()) {
 							if (current.thrownExceptions != next.thrownExceptions) {
@@ -4955,7 +4960,7 @@ public void acceptPotentiallyCompatibleMethods(MethodBinding[] methods) {/* igno
 				if (mostSpecificExceptions != null && mostSpecificExceptions != current.thrownExceptions) {
 					return new MostSpecificExceptionMethodBinding(current, mostSpecificExceptions);
 				}
-				return mostSpecificNullness;
+				return current;
 			}
 		}
 

diff --git a/...s.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java b/...s.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
@@ -19351,4 +19351,38 @@ void ba(InterfaceBA ba) {
 	runner.classLibraries = this.LIBS;
 	runner.runConformTest();
 }
+public void testGH2325_a() {
+	// argument nullness variance
+	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 {
+			void perform(@NonNull Object o);
+		}
+		interface InterfaceB {
+			void perform(@Nullable Object o);
+		}
+		interface InterfaceC {
+			void perform(@NonNull Object o);
+		}
+		interface InterfaceAB extends InterfaceA, InterfaceB, InterfaceC {}
+		interface InterfaceBA extends InterfaceB, InterfaceA, InterfaceC {}
+		class Sample {
+			void ab(InterfaceAB ab) {
+				ab.perform(null);
+			}
+			void ba(InterfaceBA ba) {
+				ba.perform(null);
+			}
+		}
+		"""
+	};
+	runner.classLibraries = this.LIBS;
+	runner.runConformTest();
+}
 }