API tooling reports 300 false positive errors on SWT

Before the change ApiBaseline.resolveSystemLibrary() did following:
1) Collected all JVM installs matching all given execution environments
id's
2) Iterated over all found installs **in random order**
3) For every JVM install ApiBaseline tried to initialize itself from
that install
4) The condition used to stop the loop (almost) never worked as it
always compared either null or previously initialized JVM with the
current one, so for all **different** JVM's ApiBaseline  initialized
itself from that JVM - and that in random order.
5) The **last** iterated JVM install defined the maximal "supported"
execution environment. In case of installed Java 1.8, 11, 17, 21 it
could be **any one** if the target platform contained bundles required
different execution environments.

With that, SWT bundle (that requires 17 environment) from saved API
baseline was not resolved with given baseline if any of lower
environments "won the race" in resolveSystemLibrary().

Because SWT bundle was not resolved, none of SWT classes were found in
the baseline and so not considered "API" in
ApiComparator.internalCompare(). Because there were no API classes in
the baseline, ALL public API types from workspace SWT project were
considered as new API and "missing @SInCE tags" errors were created.

With the change ApiBaseline.resolveSystemLibrary() does following:
1) Collects all JVM installs matching all given execution environments
id's
2) Sorts them by their Java version, with highest version first
3) Iterates over all found installs **in descending order**
4) The first (highest Java version) matching JVM install will be used to
initialize ApiBaseline
5) The loop continues only if the API baseline fails to initialize from
given JVM

With that, the **highest supported** JVM install that is required by
given execution environments defines the maximal "supported" execution
environment for the baseline.

Additional changes:

1) ApiBaselineManager.readBaselineComponents() will sort bundles to get
more stable behavior of the API tooling.
2) ApiBaseline.rebindVM() will use same ApiBaselineManagerRule that is
used by ApiBaselineManager to dispose baselines. This will prevent that
API analysis jobs may run in parallel with JVM re-initialization of the
baseline or baseline JVM update will interfere with baseline disposal.

Fixes eclipse-pde#1073

apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/ApiBaselineManager.java

@@ -483,6 +483,8 @@ public IApiComponent[] readBaselineComponents(ApiBaseline baseline, InputStream
 					}
 				}
 				restored = components.toArray(new IApiComponent[components.size()]);
+				// Avoid unstable bundle traversal order to simplify our life 
+				Arrays.sort(restored, (o1, o2) -> o1.getName().compareTo(o2.getName()));
 			}
 		} catch (IOException | SAXException e) {
 			throw new CoreException(Status.error("Error restoring API baseline", e)); //$NON-NLS-1$
@@ -637,10 +639,10 @@ public IApiBaseline getWorkspaceBaseline() {
 	 * the next request.
 	 */
 	public void disposeWorkspaceBaseline() {
-		if (workspacebaseline == null) {
+		final IApiBaseline originalBaseline = workspacebaseline;
+		if (originalBaseline == null) {
 			return;
 		}
-		final IApiBaseline originalBaseline = workspacebaseline;
 		IJobFunction runnable = m -> {
 			IApiBaseline oldBaseline = null;
 			synchronized (ApiBaselineManager.this) {

apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/ApiBaseline.java

@@ -24,6 +24,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -32,13 +33,15 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.FileLocator;
+import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.MultiStatus;
 import org.eclipse.core.runtime.OperationCanceledException;
@@ -47,6 +50,7 @@
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.jdt.core.JavaCore;
 import org.eclipse.jdt.launching.IVMInstall;
+import org.eclipse.jdt.launching.IVMInstall2;
 import org.eclipse.jdt.launching.IVMInstallChangedListener;
 import org.eclipse.jdt.launching.JavaRuntime;
 import org.eclipse.jdt.launching.PropertyChangeEvent;
@@ -63,6 +67,7 @@
 import org.eclipse.osgi.service.resolver.StateObjectFactory;
 import org.eclipse.pde.api.tools.internal.AnyValue;
 import org.eclipse.pde.api.tools.internal.ApiBaselineManager;
+import org.eclipse.pde.api.tools.internal.ApiBaselineManager.ApiBaselineManagerRule;
 import org.eclipse.pde.api.tools.internal.CoreMessages;
 import org.eclipse.pde.api.tools.internal.builder.ApiAnalysisBuilder.ApiAnalysisJob;
 import org.eclipse.pde.api.tools.internal.provisional.ApiPlugin;
@@ -165,7 +170,7 @@ public class ApiBaseline extends ApiElement implements IApiBaseline, IVMInstallC
 	 * The VM install this baseline is bound to for system libraries or
 	 * <code>null</code>. Only used in the IDE when OSGi is running.
 	 */
-	private IVMInstall fVMBinding;
+	private volatile IVMInstall fVMBinding;
 
 	private volatile boolean disposed;
 
@@ -423,7 +428,7 @@ protected void resolveSystemLibrary(HashSet<String> ees) {
 		if (ApiPlugin.isRunningInFramework() && fAutoResolve) {
 			IStatus error = null;
 			IExecutionEnvironmentsManager manager = JavaRuntime.getExecutionEnvironmentsManager();
-			Map<IVMInstall, Set<String>> vmToEEs = new HashMap<>();
+			Map<IVMInstall, Set<String>> vmToEEs = new TreeMap<>(new VmVersionComparator());
 			for (String ee : ees) {
 				IExecutionEnvironment environment = manager.getEnvironment(ee);
 				if (environment != null) {
@@ -433,43 +438,22 @@ protected void resolveSystemLibrary(HashSet<String> ees) {
 					}
 				}
 			}
-			// select VM that is compatible with most required environments
-			// keep list of all VMs
+			// The list is sorted with highest VM version first
 			List<IVMInstall> allVMInstalls = new ArrayList<>(vmToEEs.keySet());
-			String systemEE = null;
-			if (!allVMInstalls.isEmpty()) {
-				for (IVMInstall iVMInstall : allVMInstalls) {
-					// find the EE this VM is strictly compatible with
-					IExecutionEnvironment[] environments = manager.getExecutionEnvironments();
-					for (IExecutionEnvironment environment : environments) {
-						if (environment.isStrictlyCompatible(iVMInstall)) {
-							systemEE = environment.getId();
-							break;
-						}
-					}
-					if (systemEE == null) {
-						// https://bugs.eclipse.org/bugs/show_bug.cgi?id=383261
-						// we don't need to compute anything here, in all cases
-						// if
-						// we fail to find a compatible EE, fall back to highest
-						// known.
-						systemEE = "JavaSE-" + JavaCore.latestSupportedJavaVersion(); //$NON-NLS-1$
-					}
-					// only update if different from current or missing VM
-					// binding
-					if (!systemEE.equals(getExecutionEnvironment()) || fVMBinding == null) {
-						try {
-							File file = Util.createEEFile(iVMInstall, systemEE);
-							JavaRuntime.addVMInstallChangedListener(this);
-							fVMBinding = iVMInstall;
-							ExecutionEnvironmentDescription ee = new ExecutionEnvironmentDescription(file);
-							initialize(ee);
-							file.delete();
-						} catch (CoreException | IOException e) {
-							error = Status.error(CoreMessages.ApiBaseline_2, e);
-						}
-					}
+			for (IVMInstall iVMInstall : allVMInstalls) {
+				try {
+					File tmpEeFile = Util.createEEFile(iVMInstall);
+					initialize(new ExecutionEnvironmentDescription(tmpEeFile));
+					tmpEeFile.delete();
+					fVMBinding = iVMInstall;
+					break;
+				} catch (CoreException | IOException e) {
+					error = Status.error(CoreMessages.ApiBaseline_2, e);
 				}
+			}
+
+			if (fVMBinding != null) {
+				JavaRuntime.addVMInstallChangedListener(this);
 			} else {
 				// no VMs match any required EE
 				error = Status.error(CoreMessages.ApiBaseline_6);
@@ -497,6 +481,70 @@ protected void resolveSystemLibrary(HashSet<String> ees) {
 		}
 	}
 
+	/**
+	 * Sorts highest VM version first
+	 */
+	static final class VmVersionComparator implements Comparator<IVMInstall> {
+
+		private static final String UNKNOWN_VERSION = "UNKNOWN"; //$NON-NLS-1$
+		private static final Integer UNKNOWN_VERSION_ORDINAL = Integer.valueOf(-1);
+		private static final Map<String, Integer> KNOWN_VERSIONS_MAP;
+		static {
+			List<String> allVersions = JavaCore.getAllVersions();
+			KNOWN_VERSIONS_MAP = new HashMap<>(allVersions.size() + 1);
+			for (int i = 0; i < allVersions.size(); i++) {
+				KNOWN_VERSIONS_MAP.put(allVersions.get(i), Integer.valueOf(i));
+			}
+			KNOWN_VERSIONS_MAP.put(UNKNOWN_VERSION, UNKNOWN_VERSION_ORDINAL);
+		}
+
+		@Override
+		public int compare(IVMInstall o1, IVMInstall o2) {
+			String vmVersion1 = getSimpleVmVersion(o1);
+			String vmVersion2 = getSimpleVmVersion(o2);
+			Integer ordinal1 = getVmOrdinal(vmVersion1);
+			Integer ordinal2 = getVmOrdinal(vmVersion2);
+			// reversed order, so highest version is sorted first
+			return ordinal2.compareTo(ordinal1);
+		}
+
+		@SuppressWarnings("nls")
+		private static String getSimpleVmVersion(IVMInstall vm) {
+			if (!(vm instanceof IVMInstall2 vm2)) {
+				return UNKNOWN_VERSION;
+			}
+			String javaVersion = vm2.getJavaVersion();
+			if (javaVersion == null) {
+				return UNKNOWN_VERSION;
+			}
+			javaVersion = javaVersion.strip();
+			if (javaVersion.length() > 2 && javaVersion.startsWith("1.")) {
+				// 1.8.0 -> 1.8
+				javaVersion = javaVersion.substring(0, 3);
+			} else {
+				int firstDot = javaVersion.indexOf(".");
+				if (firstDot > 0) {
+					// 21.0.1 -> 21
+					javaVersion = javaVersion.substring(0, firstDot);
+				}
+			}
+			return javaVersion;
+		}
+
+		private static Integer getVmOrdinal(String vmVersion) {
+			Integer value = KNOWN_VERSIONS_MAP.get(vmVersion);
+			if (value == null) {
+				try {
+					// assume it is > Java 21 and can be parsed as integer
+					return Integer.valueOf(vmVersion);
+				} catch (Exception e) {
+					return UNKNOWN_VERSION_ORDINAL;
+				}
+			}
+			return value;
+		}
+	}
+
 	/**
 	 * Returns true if the {@link IApiBaseline} has its information loaded
 	 * (components) false otherwise. This is a handle only method that will not
@@ -956,22 +1004,47 @@ public void vmChanged(PropertyChangeEvent event) {
 	 * Re-binds the VM this baseline is bound to.
 	 */
 	private void rebindVM() {
-		Job.createSystem("Rebinding JVM", monitor -> { //$NON-NLS-1$
-			try {
-				// Let all the already running job finish first, to avoid errors
-				Job.getJobManager().join(ApiAnalysisJob.class, monitor);
-			} catch (OperationCanceledException | InterruptedException e) {
-				ApiPlugin.log("Interrupted while rebinding JVM", e); //$NON-NLS-1$
-				return;
+		final IVMInstall originalVm = fVMBinding;
+		Job.getJobManager().cancel(ApiAnalysisJob.class);
+		Job job = new Job("Rebinding JVM") { //$NON-NLS-1$
+
+			@Override
+			public IStatus run(IProgressMonitor monitor) {
+				if (monitor.isCanceled() || ApiBaseline.this.isDisposed()) {
+					return Status.CANCEL_STATUS;
+				}
+				try {
+					// Let all the already running job finish first, to avoid errors
+					Job.getJobManager().join(ApiAnalysisJob.class, monitor);
+				} catch (OperationCanceledException | InterruptedException e) {
+					ApiPlugin.log("Interrupted while rebinding JVM", e); //$NON-NLS-1$
+					return Status.CANCEL_STATUS;
+				}
+				if (originalVm != fVMBinding) {
+					return Status.CANCEL_STATUS;
+				}
+				fVMBinding = null;
+				IApiComponent[] components = getApiComponents();
+				HashSet<String> ees = new HashSet<>();
+				for (IApiComponent component2 : components) {
+					try {
+						ees.addAll(Arrays.asList(component2.getExecutionEnvironments()));
+					} catch (CoreException e) {
+						ApiPlugin.log("Error reading execution environment from " + component2, e); //$NON-NLS-1$
+					}
+				}
+				resolveSystemLibrary(ees);
+				return Status.OK_STATUS;
 			}
-			fVMBinding = null;
-			IApiComponent[] components = getApiComponents();
-			HashSet<String> ees = new HashSet<>();
-			for (IApiComponent component2 : components) {
-				ees.addAll(Arrays.asList(component2.getExecutionEnvironments()));
+
+			@Override
+			public boolean belongsTo(Object family) {
+				return super.belongsTo(family) || family == ApiBaseline.class;
 			}
-			resolveSystemLibrary(ees);
-		}).schedule();
+		};
+		job.setRule(new ApiBaselineManagerRule());
+		job.setSystem(true);
+		job.schedule();
 	}
 
 	@Override

apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/provisional/comparator/ApiComparator.java

@@ -645,9 +645,12 @@ public void visit(String packageName, IApiTypeRoot typeRoot) {
 									// anonymous)
 									return;
 								}
-								int visibility = 0;
+								int visibility;
 								if (elementDescription != null) {
 									visibility = elementDescription.getVisibility();
+								} else {
+									// Annotation is missing, not an API?
+									visibility = 0;
 								}
 								IApiTypeRoot typeRoot2 = null;
 								if (isSWT) {

apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/util/Util.java

@@ -117,6 +117,8 @@
 import org.eclipse.jdt.launching.JavaRuntime;
 import org.eclipse.jdt.launching.LibraryLocation;
 import org.eclipse.jdt.launching.environments.ExecutionEnvironmentDescription;
+import org.eclipse.jdt.launching.environments.IExecutionEnvironment;
+import org.eclipse.jdt.launching.environments.IExecutionEnvironmentsManager;
 import org.eclipse.jface.text.IDocument;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.pde.api.tools.internal.FilterStore;
@@ -440,9 +442,10 @@ public static boolean copy(File file, File newFile) {
 	}
 
 	/**
-	 * Creates an EE file for the given JRE and specified EE id
+	 * Creates an EE file for the given JRE
 	 */
-	public static File createEEFile(IVMInstall jre, String eeid) throws IOException {
+	public static File createEEFile(IVMInstall jre) throws IOException {
+		String eeid = getStrictCompatibleEE(jre);
 		String string = Util.generateEEContents(jre, eeid);
 		File eeFile = createTempFile("eed", ".ee"); //$NON-NLS-1$ //$NON-NLS-2$
 		try (FileOutputStream outputStream = new FileOutputStream(eeFile)) {
@@ -451,6 +454,21 @@ public static File createEEFile(IVMInstall jre, String eeid) throws IOException
 		return eeFile;
 	}
 
+	private static String getStrictCompatibleEE(IVMInstall iVMInstall) {
+		IExecutionEnvironmentsManager manager = JavaRuntime.getExecutionEnvironmentsManager();
+		IExecutionEnvironment[] environments = manager.getExecutionEnvironments();
+		for (IExecutionEnvironment environment : environments) {
+			if (environment.isStrictlyCompatible(iVMInstall)) {
+				return environment.getId();
+			}
+		}
+		// https://bugs.eclipse.org/bugs/show_bug.cgi?id=383261
+		// we don't need to compute anything here, in all cases
+		// if we fail to find a compatible EE, fall back to highest
+		// known.
+		return "JavaSE-" + JavaCore.latestSupportedJavaVersion(); //$NON-NLS-1$
+	}
+
 	/**
 	 * Returns whether the objects are equal, accounting for either one being
 	 * <code>null</code>.