Add support for checking package exports for changed API

Currently API tools only check for API changes on the bundle level, but even more important are checks on the exported packages. If the bundle version is not properly incremented this can lead to method not found or similar errors, also consumers of the package can not depend on the package version reliable to get new API. This now adds some basic checks to check for API changes on the package level, compare it with the baseline and suggest a new version based on the resulting delta being a breaking or non breaking change.
eclipse-pde · Jan 13, 2025 · 4ca541b · 4ca541b
1 parent 5702213
commit 4ca541b
Show file tree

Hide file tree

Showing 5 changed files with 165 additions and 13 deletions.
diff --git a/...eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/BaseApiAnalyzer.java b/...eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/BaseApiAnalyzer.java
@@ -29,9 +29,12 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.jar.JarFile;
+import java.util.stream.Collectors;
 
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IMarker;
@@ -68,13 +71,15 @@
 import org.eclipse.jdt.internal.core.BinaryType;
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.IDocument;
+import org.eclipse.osgi.service.resolver.ExportPackageDescription;
 import org.eclipse.osgi.service.resolver.ResolverError;
 import org.eclipse.osgi.service.resolver.VersionConstraint;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.pde.api.tools.internal.ApiBaselineManager;
 import org.eclipse.pde.api.tools.internal.ApiFilterStore;
 import org.eclipse.pde.api.tools.internal.IApiCoreConstants;
 import org.eclipse.pde.api.tools.internal.comparator.Delta;
+import org.eclipse.pde.api.tools.internal.model.BundleComponent;
 import org.eclipse.pde.api.tools.internal.model.ProjectComponent;
 import org.eclipse.pde.api.tools.internal.model.WorkspaceBaseline;
 import org.eclipse.pde.api.tools.internal.problems.ApiProblemFactory;
@@ -2043,6 +2048,11 @@ private void checkApiComponentVersion(final IApiComponent reference, final IApiC
 		}
 		IDelta[] breakingChanges = fBuildState.getBreakingChanges();
 		IDelta[] compatibleChanges = fBuildState.getCompatibleChanges();
+		if (reference instanceof BundleComponent referenceBundle) {
+			if (component instanceof BundleComponent componentBundle) {
+				checkApiComponentPackageVersions(referenceBundle, componentBundle, breakingChanges, compatibleChanges);
+			}
+		}
 		if (breakingChanges.length != 0) {
 			// make sure that the major version has been incremented
 			if (compversion.getMajor() <= refversion.getMajor()) {
@@ -2287,6 +2297,85 @@ && checkIfMajorVersionIncreased(compversion, refversion)) {
 		}
 	}
 
+	private void checkApiComponentPackageVersions(BundleComponent referenceBundle, BundleComponent componentBundle,
+			IDelta[] breakingChanges, IDelta[] compatibleChanges) throws CoreException {
+		Map<String, ExportPackageDescription> referencePackages = Arrays
+				.stream(referenceBundle.getBundleDescription().getExportPackages())
+				.collect(Collectors.toMap(ExportPackageDescription::getName, Function.identity()));
+		Map<String, ExportPackageDescription> componentPackages = Arrays
+				.stream(componentBundle.getBundleDescription().getExportPackages())
+				.collect(Collectors.toMap(ExportPackageDescription::getName, Function.identity()));
+		// a mapping between a package name and a required change
+		Map<String, RequiredPackageVersionChange> requiredChanges = new HashMap<>();
+		// we must compare compatible changes first, so these where overwritten later by
+		// breaking changes probably
+		for (IDelta delta : compatibleChanges) {
+			// a compatible change must result in a minor package version increment
+			analyzePackageDelta(delta, IApiProblem.MINOR_VERSION_CHANGE_PACKAGE, referencePackages, componentPackages,
+					requiredChanges);
+		}
+		for (IDelta delta : breakingChanges) {
+			// a breaking change must result in a major package change
+			analyzePackageDelta(delta, IApiProblem.MAJOR_VERSION_CHANGE_PACKAGE, referencePackages, componentPackages,
+					requiredChanges);
+		}
+		for (String pkg : referencePackages.keySet()) {
+			if (!componentPackages.containsKey(pkg)) {
+				// TODO a package export was removed! This should be require a major version
+				// change in bundle!
+			}
+		}
+		for (Entry<String, RequiredPackageVersionChange> entry : requiredChanges.entrySet()) {
+			addProblem(createPackageVersionProblem(entry.getKey(), entry.getValue()));
+		}
+	}
+
+	private void analyzePackageDelta(IDelta delta, int category,
+			Map<String, ExportPackageDescription> referencePackages,
+			Map<String, ExportPackageDescription> componentPackages,
+			Map<String, RequiredPackageVersionChange> requiredChanges) {
+		String packageName = delta.getTypeName();
+		if (packageName != null) {
+			int idx = packageName.lastIndexOf('.');
+			if (idx > 0) {
+				packageName = packageName.substring(0, idx);
+			}
+			ExportPackageDescription pkgRef = referencePackages.get(packageName);
+			if (pkgRef == null) {
+				return;
+			}
+			Version baselineVersion = pkgRef.getVersion();
+			if (baselineVersion == null || Version.emptyVersion.equals(baselineVersion)) {
+				return;
+			}
+			ExportPackageDescription baselinePackage = componentPackages.get(packageName);
+			if (baselinePackage == null) {
+				return;
+			}
+			Version suggested;
+			if (IApiProblem.MINOR_VERSION_CHANGE_PACKAGE == category) {
+				suggested = new Version(baselineVersion.getMajor(), baselineVersion.getMinor() + 1, 0);
+			} else {
+				suggested = new Version(baselineVersion.getMajor() + 1, baselineVersion.getMinor(), 0);
+			}
+			Version compVersion = baselinePackage.getVersion();
+			if (compVersion == null || compVersion.compareTo(baselineVersion) < 0) {
+				requiredChanges.put(packageName,
+						new RequiredPackageVersionChange(category, baselineVersion, compVersion, suggested));
+			}
+			if (compVersion.getMajor() > baselineVersion.getMajor()) {
+				return;
+			}
+			if (IApiProblem.MINOR_VERSION_CHANGE_PACKAGE == category) {
+				if (compVersion.getMinor() > baselineVersion.getMinor()) {
+					return;
+				}
+			}
+			requiredChanges.put(packageName,
+					new RequiredPackageVersionChange(category, baselineVersion, compVersion, suggested));
+		}
+	}
+
 	private boolean reportMultipleIncreaseMinorVersion(Version compversion, Version refversion) {
 		if (compversion.getMajor() == refversion.getMajor()) {
 			if (((compversion.getMinor() - refversion.getMinor()) > 1)) {
@@ -2364,6 +2453,12 @@ private String collectDetails(final IDelta[] deltas) {
 		return String.valueOf(writer.getBuffer());
 	}
 
+	private IApiProblem createPackageVersionProblem(String packageName, RequiredPackageVersionChange versionChange) {
+		return createVersionProblem(versionChange.category(),
+				new String[] { packageName, versionChange.suggested().toString(), versionChange.baseline().toString() },
+				versionChange.suggested().toString(), null, Constants.EXPORT_PACKAGE, packageName);
+	}
+
 	/**
 	 * Creates a marker on a manifest file for a version numbering problem and
 	 * returns it or <code>null</code>
@@ -2372,6 +2467,11 @@ private String collectDetails(final IDelta[] deltas) {
 	 * @return a new {@link IApiProblem} or <code>null</code>
 	 */
 	private IApiProblem createVersionProblem(int kind, final String[] messageargs, String version, String description) {
+		return createVersionProblem(kind, messageargs, version, description, Constants.BUNDLE_VERSION, null);
+	}
+
+	private IApiProblem createVersionProblem(int kind, final String[] messageargs, String version, String description,
+			String header, String value) {
 		IResource manifestFile = null;
 		String path = JarFile.MANIFEST_NAME;
 		if (fJavaProject != null) {
@@ -2394,7 +2494,7 @@ private IApiProblem createVersionProblem(int kind, final String[] messageargs, S
 					String line = null;
 					loop: while ((line = reader.readLine()) != null) {
 						lineCounter++;
-						if (line.startsWith(Constants.BUNDLE_VERSION)) {
+						if (line.startsWith(header)) {
 							lineNumber = lineCounter;
 							break loop;
 						}
@@ -2406,24 +2506,31 @@ private IApiProblem createVersionProblem(int kind, final String[] messageargs, S
 		}
 		if (lineNumber != -1 && contents != null) {
 			// initialize char start, char end
-			int index = CharOperation.indexOf(Constants.BUNDLE_VERSION.toCharArray(), contents, true);
-			loop: for (int i = index + Constants.BUNDLE_VERSION.length() + 1, max = contents.length; i < max; i++) {
+			int index = CharOperation.indexOf(header.toCharArray(), contents, true);
+			int headerOffset = index + header.length() + 1;
+			loop: for (int i = headerOffset, max = contents.length; i < max; i++) {
 				char currentCharacter = contents[i];
 				if (CharOperation.isWhitespace(currentCharacter)) {
 					continue;
 				}
 				charStart = i;
 				break loop;
 			}
-			loop: for (int i = charStart + 1, max = contents.length; i < max; i++) {
-				switch (contents[i]) {
-					case '\r':
-					case '\n':
-						charEnd = i;
-						break loop;
-					default:
-						continue;
+			if (value == null) {
+				loop: for (int i = charStart + 1, max = contents.length; i < max; i++) {
+					switch (contents[i])
+						{
+						case '\r':
+						case '\n':
+							charEnd = i;
+							break loop;
+						default:
+							continue;
+						}
 				}
+			} else {
+				// TODO find the matching value in the header
+				charEnd = charStart;
 			}
 		} else {
 			lineNumber = 1;

diff --git a/...pi.tools/src/org/eclipse/pde/api/tools/internal/builder/RequiredPackageVersionChange.java b/...pi.tools/src/org/eclipse/pde/api/tools/internal/builder/RequiredPackageVersionChange.java
@@ -0,0 +1,20 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Christoph Läubrich and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Christoph Läubrich - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.pde.api.tools.internal.builder;
+
+import org.osgi.framework.Version;
+
+record RequiredPackageVersionChange(int category, Version baseline, Version current, Version suggested) {
+
+}
diff --git a/...ipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/problems/ApiProblemFactory.java b/...ipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/problems/ApiProblemFactory.java
@@ -590,7 +590,10 @@ public static int getProblemMessageId(int category, int element, int kind, int f
 						return 58;
 					case IApiProblem.MINOR_VERSION_CHANGE_UNNECESSARILY:
 						return 59;
-
+					case IApiProblem.MAJOR_VERSION_CHANGE_PACKAGE:
+						return 65;
+					case IApiProblem.MINOR_VERSION_CHANGE_PACKAGE:
+						return 63;
 					default:
 						break;
 				}

diff --git a/....pde.api.tools/src/org/eclipse/pde/api/tools/internal/problems/problemmessages.properties b/....pde.api.tools/src/org/eclipse/pde/api/tools/internal/problems/problemmessages.properties
@@ -11,7 +11,7 @@
 # Contributors:
 #     IBM Corporation - initial API and implementation
 ###############################################################################
-# available message ids  63, 65, 68, 70, 71, 75, 80,
+# available message ids  68, 70, 71, 75, 80,
 # 82, 83, 88, 90, 93
 
 #API baseline 
@@ -37,6 +37,8 @@
 19 = The major version should be incremented in version {0}, because the modification of the version range for the re-exported bundle {1} requires a major version change
 20 = The minor version should be incremented in version {0}, because the modification of the version range for the re-exported bundle {1} requires a minor version change
 62 = The major version should be incremented in version {0}, because the bundle {1} is no longer re-exported
+63 = The minor version for the package ''{0}'' should be incremented to version {1}, since new APIs have been added since version {2}
+65 = The major version for the package ''{0}'' should be incremented to version {1}, since API breakage occurred since version {2}
 
 #API usage problems
 #{0} = referenced type name

diff --git a/...de.api.tools/src/org/eclipse/pde/api/tools/internal/provisional/problems/IApiProblem.java b/...de.api.tools/src/org/eclipse/pde/api/tools/internal/provisional/problems/IApiProblem.java
@@ -242,6 +242,26 @@ public interface IApiProblem {
 	 * @see #CATEGORY_USAGE
 	 */
 
+	/**
+	 * Constant representing the value of the major version change
+	 * {@link IApiProblem} kind for a package. <br>
+	 * Value is: <code>11</code>
+	 *
+	 * @see #getKind()
+	 * @see #CATEGORY_VERSION
+	 */
+	public static final int MAJOR_VERSION_CHANGE_PACKAGE = 11;
+
+	/**
+	 * Constant representing the value of the minor version change
+	 * {@link IApiProblem} kind for a package. <br>
+	 * Value is: <code>12</code>
+	 *
+	 * @see #getKind()
+	 * @see #CATEGORY_VERSION
+	 */
+	public static final int MINOR_VERSION_CHANGE_PACKAGE = 12;
+
 	public static final int ILLEGAL_EXTEND = 1;
 
 	/**