false positive contradictory null annotations since 2024-03 (#2226)

fixes #2178

includes updates to enable using jclMin21.jar + src.zip

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java

@@ -589,7 +589,7 @@ void faultInImports() {
 	this.typeOrPackageCache = new HashtableOfObject(length);
 	for (int i = 0; i < length; i++) {
 		ImportBinding binding = this.imports[i];
-		if (!binding.onDemand && binding.resolvedImport instanceof ReferenceBinding || binding instanceof ImportConflictBinding)
+		if (!binding.onDemand && binding.getResolvedBindingKind() == Binding.TYPE || binding instanceof ImportConflictBinding)
 			this.typeOrPackageCache.put(binding.getSimpleName(), binding);
 	}
 	this.skipCachingImports = this.environment.suppressImportErrors && unresolvedFound;
@@ -768,24 +768,34 @@ ImportBinding[] getDefaultImports() {
 		BinaryTypeBinding missingObject = this.environment.createMissingType(null, TypeConstants.JAVA_LANG_OBJECT);
 		importBinding = missingObject.fPackage;
 	}
-	ReferenceBinding templateSTR;
 	ImportBinding[] allImports = null;
 	if (this.environment.globalOptions.complianceLevel >= ClassFileConstants.JDK21) {
-		boolean old = this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled;
-		this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled = false;
-		templateSTR = (ReferenceBinding) ((PackageBinding) importBinding).getTypeOrPackage(TypeConstants.JAVA_LANG_STRING_TEMPLATE_STR[2], module(), false);
-		if (templateSTR != null) {
-			FieldBinding str = templateSTR.getField("STR".toCharArray(), true); //$NON-NLS-1$
-			ImportBinding ibinding = new ImportBinding(TypeConstants.JAVA_LANG_STRING_TEMPLATE_STR, false, str, null) {
-				@Override
-				public boolean isStatic() {
-					return true;
+		ImportBinding ibinding = new ImportBinding(TypeConstants.JAVA_LANG_STRING_TEMPLATE_STR, false, importBinding, null) {
+			@Override
+			public boolean isStatic() {
+				return true;
+			}
+			@Override
+			public int getResolvedBindingKind() {
+				return Binding.FIELD; // avoid resolving during faultInImports()
+			}
+			@Override
+			public Binding getResolvedImport() {
+				// resolve lazily:
+				Binding resolvedImport = super.getResolvedImport();
+				if (resolvedImport instanceof PackageBinding) { // package was past into the constructor, need to dig deeper now:
+					ReferenceBinding templateSTR = (ReferenceBinding) ((PackageBinding) resolvedImport).getTypeOrPackage(TypeConstants.JAVA_LANG_STRING_TEMPLATE_STR[2], module(), false);
+					if (templateSTR != null) {
+						FieldBinding fieldBinding = templateSTR.getField("STR".toCharArray(), true); //$NON-NLS-1$
+						if (fieldBinding != null)
+							return setResolvedImport(fieldBinding);
+					}
 				}
-			};
-			allImports = new ImportBinding[] {
-					new ImportBinding(TypeConstants.JAVA_LANG, true, importBinding, null), ibinding};
-		}
-		this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled = old;
+				return resolvedImport;
+			}
+		};
+		allImports = new ImportBinding[] {
+				new ImportBinding(TypeConstants.JAVA_LANG, true, importBinding, null), ibinding};
 	}
 	if (allImports == null){
 		allImports = new ImportBinding[] {new ImportBinding(TypeConstants.JAVA_LANG, true, importBinding, null)};
@@ -936,11 +946,14 @@ void recordTypeReferences(TypeBinding[] types) {
 	}
 }
 Binding resolveSingleImport(ImportBinding importBinding, int mask) {
-	if (importBinding.resolvedImport == null) {
-		importBinding.resolvedImport = findSingleImport(importBinding.compoundName, mask, importBinding.isStatic());
-		if (!importBinding.resolvedImport.isValidBinding() || importBinding.resolvedImport instanceof PackageBinding) {
-			if (importBinding.resolvedImport.problemId() == ProblemReasons.Ambiguous)
-				return importBinding.resolvedImport;
+	Binding resolvedBinding = importBinding.getResolvedImport();
+	if (resolvedBinding != null) {
+		return resolvedBinding;
+	} else {
+		resolvedBinding = importBinding.setResolvedImport(findSingleImport(importBinding.compoundName, mask, importBinding.isStatic()));
+		if (!resolvedBinding.isValidBinding() || resolvedBinding instanceof PackageBinding) {
+			if (resolvedBinding.problemId() == ProblemReasons.Ambiguous)
+				return resolvedBinding;
 			if (this.imports != null) {
 				ImportBinding[] newImports = new ImportBinding[this.imports.length - 1];
 				for (int i = 0, n = 0, max = this.imports.length; i < max; i++)
@@ -950,8 +963,8 @@ Binding resolveSingleImport(ImportBinding importBinding, int mask) {
 			}
 			return null;
 		}
+		return resolvedBinding;
 	}
-	return importBinding.resolvedImport;
 }
 public void storeDependencyInfo() {
 	// add the type hierarchy of each referenced supertype
@@ -1109,7 +1122,7 @@ private int checkAndRecordImportBinding(
 								recordImportBinding(new ImportBinding(compoundName, false, importBinding, importReference));
 							}
 						}
-					} else if (resolved.resolvedImport == referenceBinding) {
+					} else if (resolved.getResolvedImport() == referenceBinding) {
 						if (importReference.isStatic() != resolved.isStatic()) {
 							recordImportBinding(new ImportBinding(compoundName, false, importBinding, importReference));
 						}
@@ -1128,10 +1141,12 @@ private int checkAndRecordImportBinding(
 				// 7.5.3 says nothing about collision of single static imports and JDK8 tolerates them, though use is flagged.
 				for (int j = 0; j < this.importPtr; j++) {
 					ImportBinding resolved = this.tempImports[j];
-					if (resolved.isStatic() && resolved.resolvedImport instanceof ReferenceBinding && importBinding != resolved.resolvedImport) {
+					Binding resolvedImport = resolved.getResolvedImport();
+					if (resolved.isStatic()
+							&& resolvedImport instanceof ReferenceBinding type
+							&& importBinding != resolvedImport) {
 						if (CharOperation.equals(compoundName[compoundName.length - 1], resolved.compoundName[resolved.compoundName.length - 1])) {
-							ReferenceBinding type = (ReferenceBinding) resolved.resolvedImport;
-							resolved.resolvedImport = new ProblemReferenceBinding(new char[][] { name }, type, ProblemReasons.Ambiguous);
+							resolved.setResolvedImport(new ProblemReferenceBinding(new char[][] { name }, type, ProblemReasons.Ambiguous));
 							return -1;
 						}
 					}
@@ -1145,12 +1160,14 @@ private int checkAndRecordImportBinding(
 		for (int j = 0; j < this.importPtr; j++) {
 			ImportBinding resolved = this.tempImports[j];
 			// find other static fields with the same name
-			if (resolved.isStatic() && resolved.resolvedImport instanceof FieldBinding && importBinding != resolved.resolvedImport) {
+			Binding resolvedImport = resolved.getResolvedImport();
+			if (resolved.isStatic()
+					&& resolvedImport instanceof FieldBinding field
+					&& importBinding != resolvedImport) {
 				if (CharOperation.equals(name, resolved.compoundName[resolved.compoundName.length - 1])) {
 					if (compilerOptions().sourceLevel >= ClassFileConstants.JDK1_8) {
 						// 7.5.3 says nothing about collision of single static imports and JDK8 tolerates them, though use is flagged.
-						FieldBinding field = (FieldBinding) resolved.resolvedImport;
-						resolved.resolvedImport = new ProblemFieldBinding(field, field.declaringClass, name, ProblemReasons.Ambiguous);
+						resolved.setResolvedImport(new ProblemFieldBinding(field, field.declaringClass, name, ProblemReasons.Ambiguous));
 						return -1;
 					} else {
 						problemReporter().duplicateImport(importReference);

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ImportBinding.java

@@ -21,7 +21,7 @@ public class ImportBinding extends Binding {
 	public boolean onDemand;
 	public ImportReference reference;
 
-	public Binding resolvedImport; // must ensure the import is resolved
+	private Binding resolvedImport;
 
 public ImportBinding(char[][] compoundName, boolean isOnDemand, Binding binding, ImportReference reference) {
 	this.compoundName = compoundName;
@@ -54,6 +54,20 @@ public char[] readableName() {
 	else
 		return CharOperation.concatWith(this.compoundName, '.');
 }
+public int getResolvedBindingKind() {
+	if (this.resolvedImport == null)
+		return 0;
+	return this.resolvedImport.kind() & (Binding.TYPE | Binding.FIELD | Binding.METHOD);
+}
+public Binding getResolvedImport() {
+	return this.resolvedImport;
+}
+
+public Binding setResolvedImport(Binding resolvedImport) {
+	this.resolvedImport = resolvedImport;
+	return resolvedImport;
+}
+
 @Override
 public String toString() {
 	return "import : " + new String(readableName()); //$NON-NLS-1$

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java

@@ -2233,8 +2233,10 @@ public Binding getBinding(char[] name, int mask, InvocationSite invocationSite,
 						for (ImportBinding importBinding : imports) {
 							if (importBinding.isStatic() && !importBinding.onDemand) {
 								if (CharOperation.equals(importBinding.getSimpleName(), name)) {
-									if (unitScope.resolveSingleImport(importBinding, Binding.TYPE | Binding.FIELD | Binding.METHOD) != null && importBinding.resolvedImport instanceof FieldBinding) {
-										foundField = (FieldBinding) importBinding.resolvedImport;
+									if (unitScope.resolveSingleImport(importBinding, Binding.TYPE | Binding.FIELD | Binding.METHOD) != null
+											&& importBinding.getResolvedImport() instanceof FieldBinding resolvedField)
+									{
+										foundField = resolvedField;
 										ImportReference importReference = importBinding.reference;
 										if (importReference != null && needResolve) {
 											importReference.bits |= ASTNode.Used;
@@ -2259,7 +2261,7 @@ public Binding getBinding(char[] name, int mask, InvocationSite invocationSite,
 						ReferenceBinding sourceCodeReceiver = null;
 						for (ImportBinding importBinding : imports) {
 							if (importBinding.isStatic() && importBinding.onDemand) {
-								Binding resolvedImport = importBinding.resolvedImport;
+								Binding resolvedImport = importBinding.getResolvedImport();
 								if (resolvedImport instanceof ReferenceBinding) {
 									ReferenceBinding importedReferenceBinding = (ReferenceBinding) resolvedImport;
 									FieldBinding temp = findField(importedReferenceBinding, name, invocationSite, needResolve);
@@ -2745,7 +2747,7 @@ public MethodBinding getImplicitMethod(char[] selector, TypeBinding[] argumentTy
 				boolean skipOnDemand = false; // set to true when matched static import of method name so stop looking for on demand methods
 				for (ImportBinding importBinding : imports) {
 					if (importBinding.isStatic()) {
-						Binding resolvedImport = importBinding.resolvedImport;
+						Binding resolvedImport = importBinding.getResolvedImport();
 						MethodBinding possible = null;
 						if (importBinding.onDemand) {
 							if (!skipOnDemand && resolvedImport instanceof ReferenceBinding) {
@@ -3519,13 +3521,14 @@ final Binding getTypeOrPackage(char[] name, int mask, boolean needResolve) {
 				if (cachedBinding instanceof ImportBinding) { // single type import cached in faultInImports(), replace it in the cache with the type
 					ImportBinding importBinding = (ImportBinding) cachedBinding;
 					ImportReference importReference = importBinding.reference;
-					if (importReference != null && !isUnnecessarySamePackageImport(importBinding.resolvedImport, unitScope)) {
+					Binding resolvedImport = importBinding.getResolvedImport();
+					if (importReference != null && !isUnnecessarySamePackageImport(resolvedImport, unitScope)) {
 						importReference.bits |= ASTNode.Used;
 					}
 					if (cachedBinding instanceof ImportConflictBinding)
 						typeOrPackageCache.put(name, cachedBinding = ((ImportConflictBinding) cachedBinding).conflictingTypeBinding); // already know its visible
 					else
-						typeOrPackageCache.put(name, cachedBinding = importBinding.resolvedImport); // already know its visible
+						typeOrPackageCache.put(name, cachedBinding = resolvedImport); // already know its visible
 				}
 				if ((mask & Binding.TYPE) != 0) {
 					if (foundType != null && foundType.problemId() != ProblemReasons.NotVisible && cachedBinding.problemId() != ProblemReasons.Ambiguous)
@@ -3549,7 +3552,7 @@ final Binding getTypeOrPackage(char[] name, int mask, boolean needResolve) {
 							if (resolvedImport == null) continue nextImport;
 							if (resolvedImport instanceof TypeBinding) {
 								ImportReference importReference = importBinding.reference;
-								if (importReference != null && !isUnnecessarySamePackageImport(importBinding.resolvedImport, unitScope))
+								if (importReference != null && !isUnnecessarySamePackageImport(importBinding.getResolvedImport(), unitScope))
 									importReference.bits |= ASTNode.Used;
 								return resolvedImport; // already know its visible
 							}
@@ -3581,7 +3584,7 @@ final Binding getTypeOrPackage(char[] name, int mask, boolean needResolve) {
 				ReferenceBinding type = null;
 				for (ImportBinding someImport : imports) {
 					if (someImport.onDemand) {
-						Binding resolvedImport = someImport.resolvedImport;
+						Binding resolvedImport = someImport.getResolvedImport();
 						ReferenceBinding temp = null;
 						if (resolvedImport instanceof PackageBinding) {
 							temp = findType(name, (PackageBinding) resolvedImport, currentPackage);

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTests21.java

@@ -19,6 +19,7 @@
 
 import org.eclipse.jdt.core.JavaCore;
 import org.eclipse.jdt.core.tests.util.Util;
+import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 
 import junit.framework.Test;
@@ -999,6 +1000,8 @@ public class X {
 	}
 
 	public void testGH1964_since_22() {
+		if (this.complianceLevel < ClassFileConstants.JDK22)
+			return;
 		Runner runner = new Runner();
 		runner.customOptions = getCompilerOptions();
 		runner.customOptions.put(CompilerOptions.OPTION_EnablePreviews, CompilerOptions.ENABLED);

org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/AbstractJavaModelTests.java

@@ -3713,18 +3713,18 @@ public void setUpJCLClasspathVariables(String compliance, boolean useFullJCL) th
 			}
 		} else if ("21".equals(compliance)) {
 			if (JavaCore.getClasspathVariable("JCL_21_LIB") == null) {
-				setupExternalJCL("jclMin17");
+				setupExternalJCL("jclMin21");
 				JavaCore.setClasspathVariables(
-					new String[] {"JCL_17_LIB", "JCL_17_SRC", "JCL_SRCROOT"},
-					new IPath[] {getExternalJCLPath("17"), getExternalJCLSourcePath("17"), getExternalJCLRootSourcePath()},
+					new String[] {"JCL_21_LIB", "JCL_21_SRC", "JCL_SRCROOT"},
+					new IPath[] {getExternalJCLPath("21"), getExternalJCLSourcePath("21"), getExternalJCLRootSourcePath()},
 					null);
 			}
 		} else if ("22".equals(compliance)) {
 			if (JavaCore.getClasspathVariable("JCL_22_LIB") == null) {
-				setupExternalJCL("jclMin17");
+				setupExternalJCL("jclMin21");
 				JavaCore.setClasspathVariables(
-					new String[] {"JCL_17_LIB", "JCL_17_SRC", "JCL_SRCROOT"},
-					new IPath[] {getExternalJCLPath("17"), getExternalJCLSourcePath("17"), getExternalJCLRootSourcePath()},
+					new String[] {"JCL_21_LIB", "JCL_21_SRC", "JCL_SRCROOT"},
+					new IPath[] {getExternalJCLPath("21"), getExternalJCLSourcePath("21"), getExternalJCLRootSourcePath()},
 					null);
 			}
 		} else {

org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations18Test.java

@@ -269,7 +269,10 @@ void setupJavaProject(String name, boolean useFullJCL, boolean addAnnotationLib)
 	}
 
 	void myCreateJavaProject(String name) throws CoreException {
-		this.project = createJavaProject(name, new String[]{"src"}, new String[]{this.jclLib}, null, null, "bin", null, null, null, this.compliance);
+		myCreateJavaProject(name, this.compliance, this.jclLib);
+	}
+	void myCreateJavaProject(String name, String projectCompliance, String projectJclLib) throws CoreException {
+		this.project = createJavaProject(name, new String[]{"src"}, new String[]{projectJclLib}, null, null, "bin", null, null, null, projectCompliance);
 		addLibraryEntry(this.project, this.ANNOTATION_LIB, false);
 		Map options = this.project.getOptions(true);
 		options.put(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.ENABLED);
@@ -3353,4 +3356,76 @@ Integer test() {
 		IProblem[] problems = reconciled.getProblems();
 		assertNoProblems(problems);
 	}
+	public void testGH2178() throws CoreException, IOException {
+		myCreateJavaProject("GH2178", "21", "JCL_21_LIB");
+		Map options = this.project.getOptions(true);
+		options.put(JavaCore.COMPILER_PB_NULL_SPECIFICATION_VIOLATION, JavaCore.ERROR);
+		options.put(JavaCore.COMPILER_PB_NULL_ANNOTATION_INFERENCE_CONFLICT, JavaCore.ERROR);
+		this.project.setOptions(options);
+
+		addLibraryWithExternalAnnotations(this.project, "21", "jreext.jar", "annots", new String[] {
+				"/UnannotatedLib/lib/Objects.java",
+				"""
+				package lib;
+				public class Objects {
+					public static <T> T requireNonNull(T t) { return t; }
+				}
+				"""
+			}, null);
+		createFileInProject("annots/java/lang", "String.eea",
+				"""
+				class java/lang/String
+				valueOf
+				 (Z)Ljava/lang/String;
+				 (Z)L1java/lang/String;
+				""");
+		createFileInProject("annots/lib", "Objects.eea",
+				"""
+				class lib/Objects
+				requireNonNull
+				 <T:Ljava/lang/Object;>(TT;)TT;
+				 <T:Ljava/lang/Object;>(T0T;)T1T;
+				""");
+		addEeaToVariableEntry("JCL_21_LIB", "/GH2178/annots");
+
+
+		IPackageFragment fragment = this.project.getPackageFragmentRoots()[0].createPackageFragment("repro", true, null);
+		ICompilationUnit unit = fragment.createCompilationUnit("ExternalNullAnnotationsConfusion.java",
+				"""
+				package repro;
+
+				import lib.Objects;
+
+				import org.eclipse.jdt.annotation.NonNull;
+
+				public class ExternalNullAnnotationsConfusion {
+
+					public String conflictingNonNullAndNullable() {
+
+						// String.valueOf() is annotated to return @NonNull
+						@NonNull
+						String valueOfAnnotatedNonNull = String.valueOf(false);
+
+						// Objects.requireNonNull is annotated to take a @Nullable parameter
+						@NonNull
+						String result = Objects.requireNonNull(valueOfAnnotatedNonNull);
+
+						// error marker at the last argument in the previous line:
+						// Contradictory null annotations:
+						// method was inferred as '@NonNull String requireNonNull(@Nullable @NonNull String)',
+						// but only one of '@NonNull' and '@Nullable' can be effective at any location
+						return result;
+					}
+				}
+				""",
+				true, new NullProgressMonitor()).getWorkingCopy(new NullProgressMonitor());
+		CompilationUnit reconciled = unit.reconcile(getJLS8(), true, null, new NullProgressMonitor());
+		IProblem[] problems = reconciled.getProblems();
+		assertNoProblems(problems);
+
+		this.project.getProject().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, null);
+		IMarker[] markers = this.project.getProject().findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE);
+		assertNoMarkers(markers);
+	}
+
 }