From dcbece8082487e906d3eae5704060e15c50fc583 Mon Sep 17 00:00:00 2001
From: Stephan Herrmann <stephan.herrmann@berlin.de>
Date: Thu, 11 Apr 2024 21:55:31 +0200
Subject: [PATCH] null analysis should merge null contracts from equivalent
 super methods

fixes #2325

+ if method choice would be arbitrary prefer more specific nullness
---
 .../compiler/ast/NullAnnotationMatching.java  | 20 +++++++++++
 .../jdt/internal/compiler/lookup/Scope.java   |  7 +++-
 .../regression/NullTypeAnnotationTest.java    | 35 +++++++++++++++++++
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java
index bf259468cc6..051cea830b1 100644
--- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java
+++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java
@@ -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$
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
index de06d99f68a..802402a1830 100644
--- 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,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++) {
@@ -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) {
@@ -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;
 			}
 		}
 
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
index 739123e7c50..23e89eec03a 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
@@ -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();
+}
 }