diff --git a/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/VersionTest.java b/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/VersionTest.java index 660772bff16..de051740d5f 100644 --- a/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/VersionTest.java +++ b/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/VersionTest.java @@ -359,4 +359,35 @@ private void xRegardlessMajorInc(boolean incremental) throws Exception { ApiProblem[] problems = getEnv().getProblemsFor(filePath, null); assertProblems(problems); } + + /** + * Tests that when a method is added to a base class, the package version + * must be incremented for packages containing subclasses that inherit the + * new method. See https://github.com/eclipse-pde/eclipse.pde/issues/2064 + */ + private void xInheritedMethodVersionIncrement(boolean incremental) throws Exception { + IPath filePath = WORKSPACE_CLASSES_PACKAGE_A.append("InheritedMethodBase.java"); //$NON-NLS-1$ + // Expected: two version problems + // 1. Package a.version needs minor version increment (1.0.0 -> 1.1.0) + // 2. Package a.version.sub needs minor version increment (1.0.0 -> 1.1.0) due to inherited method + int[] ids = new int[] { + ApiProblemFactory.createProblemId(IApiProblem.CATEGORY_VERSION, IElementDescriptor.RESOURCE, + IApiProblem.MINOR_VERSION_CHANGE_PACKAGE, IApiProblem.NO_FLAGS), + ApiProblemFactory.createProblemId(IApiProblem.CATEGORY_VERSION, IElementDescriptor.RESOURCE, + IApiProblem.MINOR_VERSION_CHANGE_PACKAGE, IApiProblem.NO_FLAGS) }; + setExpectedProblemIds(ids); + String[][] args = new String[2][]; + args[0] = new String[] { "a.version", "1.1.0", "1.0.0" }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + args[1] = new String[] { "a.version.sub", "1.1.0", "1.0.0" }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + setExpectedMessageArgs(args); + performVersionTest(filePath, incremental); + } + + public void testInheritedMethodVersionIncrementI() throws Exception { + xInheritedMethodVersionIncrement(true); + } + + public void testInheritedMethodVersionIncrementF() throws Exception { + xInheritedMethodVersionIncrement(false); + } } diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/META-INF/MANIFEST.MF b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/META-INF/MANIFEST.MF index 97bb4ef7698..677396d85ec 100644 --- a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/META-INF/MANIFEST.MF +++ b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/META-INF/MANIFEST.MF @@ -23,4 +23,5 @@ Export-Package: a.annotations, a.methods.modifiers, a.methods.typeparameters, a.since, - a.version + a.version;version="1.0.0", + a.version.sub;version="1.0.0" diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/src/a/version/InheritedMethodBase.java b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/src/a/version/InheritedMethodBase.java new file mode 100644 index 00000000000..e1271611c3f --- /dev/null +++ b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/src/a/version/InheritedMethodBase.java @@ -0,0 +1,21 @@ +/******************************************************************************* + * Copyright (c) 2025 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 a.version; + +/** + * Base class for testing inherited method version increment + */ +public class InheritedMethodBase { + +} diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/src/a/version/sub/InheritedMethodSub.java b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/src/a/version/sub/InheritedMethodSub.java new file mode 100644 index 00000000000..dd21f4c0745 --- /dev/null +++ b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.a/src/a/version/sub/InheritedMethodSub.java @@ -0,0 +1,23 @@ +/******************************************************************************* + * Copyright (c) 2025 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 a.version.sub; + +import a.version.InheritedMethodBase; + +/** + * Subclass that inherits methods from base class in different package + */ +public class InheritedMethodSub extends InheritedMethodBase { + +} diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/compat/version/InheritedMethodBase.java b/apitools/org.eclipse.pde.api.tools.tests/test-builder/compat/version/InheritedMethodBase.java new file mode 100644 index 00000000000..737bee9ba3e --- /dev/null +++ b/apitools/org.eclipse.pde.api.tools.tests/test-builder/compat/version/InheritedMethodBase.java @@ -0,0 +1,23 @@ +/******************************************************************************* + * Copyright (c) 2025 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 a.version; + +/** + * Base class for testing inherited method version increment + */ +public class InheritedMethodBase { + + public void newMethod() {} + +} diff --git a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/BaseApiAnalyzer.java b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/BaseApiAnalyzer.java index 6ccc8868348..c76a59aaa58 100644 --- a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/BaseApiAnalyzer.java +++ b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/BaseApiAnalyzer.java @@ -110,6 +110,7 @@ import org.eclipse.pde.api.tools.internal.provisional.model.IApiType; import org.eclipse.pde.api.tools.internal.provisional.model.IApiTypeContainer; import org.eclipse.pde.api.tools.internal.provisional.model.IApiTypeRoot; +import org.eclipse.pde.api.tools.internal.provisional.model.ApiTypeContainerVisitor; import org.eclipse.pde.api.tools.internal.provisional.problems.IApiProblem; import org.eclipse.pde.api.tools.internal.provisional.problems.IApiProblemFilter; import org.eclipse.pde.api.tools.internal.provisional.problems.IApiProblemTypes; @@ -2312,12 +2313,12 @@ private void checkApiComponentPackageVersions(BundleComponent referenceBundle, B 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); + requiredChanges, componentBundle); } for (IDelta delta : breakingChanges) { // a breaking change must result in a major package change analyzePackageDelta(delta, IApiProblem.MAJOR_VERSION_CHANGE_PACKAGE, referencePackages, componentPackages, - requiredChanges); + requiredChanges, componentBundle); } for (String pkg : referencePackages.keySet()) { if (!componentPackages.containsKey(pkg)) { @@ -2333,13 +2334,12 @@ private void checkApiComponentPackageVersions(BundleComponent referenceBundle, B private void analyzePackageDelta(IDelta delta, int category, Map referencePackages, Map componentPackages, - Map requiredChanges) { - String packageName = delta.getTypeName(); - if (packageName != null) { - int idx = packageName.lastIndexOf('.'); - if (idx > 0) { - packageName = packageName.substring(0, idx); - } + Map requiredChanges, BundleComponent componentBundle) { + String typeName = delta.getTypeName(); + if (typeName != null) { + int idx = typeName.lastIndexOf('.'); + String packageName = idx > 0 ? typeName.substring(0, idx) : typeName; + ExportPackageDescription pkgRef = referencePackages.get(packageName); if (pkgRef == null) { return; @@ -2352,28 +2352,191 @@ private void analyzePackageDelta(IDelta delta, int category, 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); + + // Mark the package containing the changed type + markPackageVersionChange(packageName, category, baselineVersion, baselinePackage.getVersion(), requiredChanges); + + // Check if we need to mark packages containing subclasses + // This is needed when a method is added to a base class - subclasses in other packages inherit it + if (shouldCheckInheritance(delta)) { + try { + markSubclassPackages(typeName, packageName, category, referencePackages, componentPackages, + requiredChanges, componentBundle); + } catch (CoreException e) { + // Log but continue processing + if (ApiPlugin.DEBUG_API_ANALYZER) { + System.err.println("Error checking subclasses for " + typeName + ": " + e.getMessage()); //$NON-NLS-1$ //$NON-NLS-2$ + } + } } - Version compVersion = baselinePackage.getVersion(); - if (compVersion == null || compVersion.compareTo(baselineVersion) < 0) { - requiredChanges.put(packageName, - new RequiredPackageVersionChange(category, baselineVersion, compVersion, suggested)); + } + } + + /** + * Determines if we should check for inherited members when processing this delta. + * We need to check inheritance when a method or field is added to a class or interface. + */ + private boolean shouldCheckInheritance(IDelta delta) { + int elementType = delta.getElementType(); + int kind = delta.getKind(); + int flags = delta.getFlags(); + + // Check for method addition to a class + if (elementType == IDelta.CLASS_ELEMENT_TYPE && kind == IDelta.ADDED && flags == IDelta.METHOD) { + return true; + } + // Check for field addition to a class (fields are also inherited) + if (elementType == IDelta.CLASS_ELEMENT_TYPE && kind == IDelta.ADDED && flags == IDelta.FIELD) { + return true; + } + // Check for method addition to an interface (default methods are inherited) + if (elementType == IDelta.INTERFACE_ELEMENT_TYPE && kind == IDelta.ADDED && flags == IDelta.METHOD) { + return true; + } + return false; + } + + /** + * Marks packages containing subclasses of the given type as needing version increments. + */ + private void markSubclassPackages(String baseTypeName, String basePackageName, int category, + Map referencePackages, + Map componentPackages, + Map requiredChanges, + BundleComponent componentBundle) throws CoreException { + + // Find all types in the component that extend the base type + Set subclassPackages = new HashSet<>(); + + componentBundle.accept(visitor -> { + try { + IApiTypeRoot typeRoot = visitor.getTypeRoot(); + if (typeRoot != null) { + IApiType type = typeRoot.getStructure(); + if (type != null && !type.getName().equals(baseTypeName)) { + // Check if this type extends the base type + if (extendsType(type, baseTypeName)) { + String subTypeName = type.getName(); + int idx = subTypeName.lastIndexOf('.'); + if (idx > 0) { + String subPackageName = subTypeName.substring(0, idx); + // Only mark if it's a different package and is exported + if (!subPackageName.equals(basePackageName) && componentPackages.containsKey(subPackageName)) { + subclassPackages.add(subPackageName); + } + } + } + } + } + } catch (CoreException e) { + // Continue processing other types } - if (compVersion.getMajor() > baselineVersion.getMajor()) { - return; + return true; + }); + + // Mark all packages containing subclasses + for (String subPackageName : subclassPackages) { + ExportPackageDescription refPkg = referencePackages.get(subPackageName); + ExportPackageDescription compPkg = componentPackages.get(subPackageName); + if (refPkg != null && compPkg != null) { + Version baselineVersion = refPkg.getVersion(); + if (baselineVersion != null && !Version.emptyVersion.equals(baselineVersion)) { + markPackageVersionChange(subPackageName, category, baselineVersion, compPkg.getVersion(), requiredChanges); + } } - if (IApiProblem.MINOR_VERSION_CHANGE_PACKAGE == category) { - if (compVersion.getMinor() > baselineVersion.getMinor()) { - return; + } + } + + /** + * Checks if the given type extends or implements (directly or indirectly) the specified base type. + */ + private boolean extendsType(IApiType type, String baseTypeName) throws CoreException { + // Check superclass hierarchy + IApiType current = type; + while (current != null) { + IApiType superclass = current.getSuperclass(); + if (superclass == null) { + break; + } + if (superclass.getName().equals(baseTypeName)) { + return true; + } + current = superclass; + } + + // Check interface hierarchy + return implementsInterface(type, baseTypeName); + } + + /** + * Checks if the given type implements (directly or indirectly) the specified interface. + */ + private boolean implementsInterface(IApiType type, String interfaceName) throws CoreException { + Set visited = new HashSet<>(); + return implementsInterfaceHelper(type, interfaceName, visited); + } + + private boolean implementsInterfaceHelper(IApiType type, String interfaceName, Set visited) throws CoreException { + if (type == null || !visited.add(type.getName())) { + return false; + } + + // Check direct interfaces + String[] interfaceNames = type.getSuperInterfaceNames(); + if (interfaceNames != null) { + for (String iface : interfaceNames) { + // The interface name might be in binary format (with $ for inner types) + if (iface.replace('$', '.').equals(interfaceName)) { + return true; + } + // Recursively check super-interfaces + try { + IApiType ifaceType = type.getApiComponent().findType(iface); + if (ifaceType != null && implementsInterfaceHelper(ifaceType, interfaceName, visited)) { + return true; + } + } catch (CoreException e) { + // Continue checking other interfaces } } + } + + // Check interfaces implemented by superclass + IApiType superclass = type.getSuperclass(); + if (superclass != null) { + return implementsInterfaceHelper(superclass, interfaceName, visited); + } + + return false; + } + + /** + * Marks a package as requiring a version change. + */ + private void markPackageVersionChange(String packageName, int category, Version baselineVersion, + Version compVersion, Map requiredChanges) { + 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); + } + + if (compVersion == null || compVersion.compareTo(baselineVersion) < 0) { requiredChanges.put(packageName, new RequiredPackageVersionChange(category, baselineVersion, compVersion, suggested)); + return; + } + 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) {