From f9f5117bb0b91757bd45c329767e0919f50bf4db Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Thu, 11 Apr 2024 22:29:35 +0200 Subject: [PATCH] null analysis should merge null contracts from equivalent super methods + safer approach, to ensure the selected method fulfills all criteria --- .../jdt/internal/compiler/lookup/Scope.java | 13 ++++--- .../regression/NullTypeAnnotationTest.java | 34 +++++++++++++++++++ 2 files changed, 43 insertions(+), 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 index 802402a1830..8793c9d3de4 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,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/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 23e89eec03a..ce21018db94 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 @@ -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(); +} }