Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
@@ -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 {

}
Original file line number Diff line number Diff line change
@@ -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 {

}
Original file line number Diff line number Diff line change
@@ -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() {}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -2333,13 +2334,12 @@ private void checkApiComponentPackageVersions(BundleComponent referenceBundle, B
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);
}
Map<String, RequiredPackageVersionChange> 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;
Expand All @@ -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<String, ExportPackageDescription> referencePackages,
Map<String, ExportPackageDescription> componentPackages,
Map<String, RequiredPackageVersionChange> requiredChanges,
BundleComponent componentBundle) throws CoreException {

// Find all types in the component that extend the base type
Set<String> 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<String> visited = new HashSet<>();
return implementsInterfaceHelper(type, interfaceName, visited);
}

private boolean implementsInterfaceHelper(IApiType type, String interfaceName, Set<String> 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<String, RequiredPackageVersionChange> 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) {
Expand Down