Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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 @@ -58,6 +58,9 @@ public void initializeDefaultPreferences() {
prefs.putInt(CompilerFlags.P_MISSING_VERSION_EXP_PKG, CompilerFlags.IGNORE);
prefs.putInt(CompilerFlags.P_MISSING_VERSION_IMP_PKG, CompilerFlags.IGNORE);
prefs.putInt(CompilerFlags.P_MISSING_VERSION_REQ_BUNDLE, CompilerFlags.IGNORE);
prefs.putInt(CompilerFlags.P_MISSING_UPPER_VERSION_BOUND_REQ_BUNDLE, CompilerFlags.WARNING);
prefs.putInt(CompilerFlags.P_MISSING_UPPER_VERSION_BOUND_IMP_PKG, CompilerFlags.WARNING);
prefs.putInt(CompilerFlags.P_IMP_PKG_MISSING_VERSION_FOR_EXPORT_WITHOUT_VERSION, CompilerFlags.WARNING);

prefs.putBoolean(CompilerFlags.S_CREATE_DOCS, false);
prefs.put(CompilerFlags.S_DOC_FOLDER, "doc"); //$NON-NLS-1$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,18 @@ private void validateBundleVersionAttribute(IHeader header, ManifestElement elem
getPackageLine(header, element), CompilerFlags.ERROR, PDEMarkerFactory.CAT_FATAL);
addMarkerAttribute(marker,PDEMarkerFactory.compilerKey, CompilerFlags.P_MISSING_VERSION_REQ_BUNDLE);
}

// Check for missing upper bound on version range
if (versionRange != null && VersionUtil.validateVersionRange(versionRange).isOK()) {
int upperBoundSeverity = CompilerFlags.getFlag(fProject, CompilerFlags.P_MISSING_UPPER_VERSION_BOUND_REQ_BUNDLE);
if (upperBoundSeverity != CompilerFlags.IGNORE && !hasUpperBound(versionRange)) {
VirtualMarker marker = report(
NLS.bind(PDECoreMessages.BundleErrorReporter_MissingUpperBoundForBundle, element.getValue()),
getPackageLine(header, element), upperBoundSeverity, PDEMarkerFactory.CAT_OTHER);
marker.setAttribute("bundleId", element.getValue()); //$NON-NLS-1$
addMarkerAttribute(marker, PDEMarkerFactory.compilerKey, CompilerFlags.P_MISSING_UPPER_VERSION_BOUND_REQ_BUNDLE);
}
}
}

private void validateVisibilityDirective(IHeader header, ManifestElement element) {
Expand Down Expand Up @@ -1231,6 +1243,9 @@ private void validateImportPackage(IProgressMonitor monitor) {
// TODO we should only validate versions that we have a match
validateImportPackageVersion(header, element);

// Validate version constraints based on exported package versions
validateImportPackageVersionAgainstExport(header, element, exported);

if (!hasUnresolved) {
continue;
}
Expand Down Expand Up @@ -1521,6 +1536,66 @@ private void validateImportPackageVersion(IHeader header, ManifestElement elemen
addMarkerAttribute(marker,PDEMarkerFactory.compilerKey, CompilerFlags.P_MISSING_VERSION_IMP_PKG);
}
validateVersionAttribute(header, element, true);

// Check for missing upper bound on version range
if (version != null && VersionUtil.validateVersionRange(version).isOK()) {
int upperBoundSeverity = CompilerFlags.getFlag(fProject, CompilerFlags.P_MISSING_UPPER_VERSION_BOUND_IMP_PKG);
if (upperBoundSeverity != CompilerFlags.IGNORE && !hasUpperBound(version)) {
VirtualMarker marker = report(
NLS.bind(PDECoreMessages.BundleErrorReporter_MissingUpperBoundForPackage, element.getValue()),
getPackageLine(header, element), upperBoundSeverity, PDEMarkerFactory.CAT_OTHER);
addMarkerAttribute(marker, PDEMarkerFactory.compilerKey, CompilerFlags.P_MISSING_UPPER_VERSION_BOUND_IMP_PKG);
}
}
}

/**
* Validates import package version constraints against the exported package's version.
* Implements requirement 2 and 3:
* - If package has no version and export has no version, warn that it's dangerous
* - If package has no version but export has version, recommend adding version range
*
* @param header the Import-Package header
* @param element the manifest element for a specific import
* @param exported map of available exported packages
*/
private void validateImportPackageVersionAgainstExport(IHeader header, ManifestElement element,
HashMap<String, ExportPackageDescription> exported) {
String importVersion = element.getAttribute(Constants.VERSION_ATTRIBUTE);

// Only check if there's no version constraint on the import
if (importVersion != null) {
return;
}

// Check each package component in the element
String[] packages = element.getValueComponents();
for (String packageName : packages) {
ExportPackageDescription export = exported.get(packageName);
if (export == null) {
// Package not found in exports, skip (will be caught by other validation)
continue;
}

Version exportVersion = export.getVersion();
// Check if export has a version (Version.emptyVersion means no version)
boolean exportHasVersion = exportVersion != null && !exportVersion.equals(Version.emptyVersion);

if (!exportHasVersion) {
// Requirement 2: Package has no version, export has no version - warn it's dangerous
int severity = CompilerFlags.getFlag(fProject, CompilerFlags.P_IMP_PKG_MISSING_VERSION_FOR_EXPORT_WITHOUT_VERSION);
if (severity != CompilerFlags.IGNORE) {
VirtualMarker marker = report(
NLS.bind(PDECoreMessages.BundleErrorReporter_ImportPkgMissingVersionForExportWithoutVersion, packageName),
getPackageLine(header, element), severity, PDEMarkerFactory.CAT_OTHER);
addMarkerAttribute(marker, PDEMarkerFactory.compilerKey, CompilerFlags.P_IMP_PKG_MISSING_VERSION_FOR_EXPORT_WITHOUT_VERSION);
}
} else {
// Requirement 3: Package has no version but export has version - recommend adding version range
// This is already handled by P_MISSING_VERSION_IMP_PKG, but we could add a more specific message
// For now, the existing P_MISSING_VERSION_IMP_PKG flag will handle this case
}
}
}

private void validateExportPackageVersion(IHeader header, ManifestElement element) {
Expand Down Expand Up @@ -1833,4 +1908,44 @@ private void validateServiceComponent() {
addMarkerAttribute(marker, PDEMarkerFactory.compilerKey, CompilerFlags.P_SERVICE_COMP_WITHOUT_LAZY_ACT);
}
}

/**
* Checks if a version range string has an upper bound.
* A version range has an upper bound if it's not open-ended (doesn't end with infinity).
*
* @param versionRangeStr the version range string to check
* @return true if the version range has an upper bound, false otherwise
*/
private boolean hasUpperBound(String versionRangeStr) {
try {
// Parse to validate it's a valid version range
new VersionRange(versionRangeStr);

// Use string-based detection which is more reliable than checking Version objects
// The OSGi spec defines several forms:
// - "1.0.0" => [1.0.0,∞) - no upper bound
// - "[1.0.0,2.0.0)" => has upper bound
// - "[1.0.0,)" => no upper bound

if (!versionRangeStr.contains(",")) { //$NON-NLS-1$
// Single version without brackets like "1.0.0" is interpreted as [1.0.0,∞)
// Only versions within brackets like "[1.0.0]" have implicit upper bound equal to lower
return versionRangeStr.trim().startsWith("[") || versionRangeStr.trim().startsWith("("); //$NON-NLS-1$ //$NON-NLS-2$
}

// For ranges with comma, check if there's a version number after the comma
// Ranges like "[1.0.0,)" or "[1.0.0,]" have no upper bound
int commaIndex = versionRangeStr.lastIndexOf(',');
String afterComma = versionRangeStr.substring(commaIndex + 1).trim();
// If after comma is just a closing bracket/paren, no upper bound
if (afterComma.equals("]") || afterComma.equals(")")) { //$NON-NLS-1$ //$NON-NLS-2$
return false;
}
// Otherwise, there should be a version number, which means we have an upper bound
return !afterComma.isEmpty();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not parse the string directly but use the VersionRange object to directly examine if an upper bound is defined

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit a179229. The hasUpperBound() method now uses VersionRange.getLeft() and getRight() to examine the range, combined with string parsing to detect unbounded ranges like "[1.0.0,)".

} catch (IllegalArgumentException e) {
// If we can't parse it, conservatively assume no upper bound
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ public class CompilerFlags {
public static final String P_MISSING_VERSION_EXP_PKG = "compilers.p.missing-version-export-package"; //$NON-NLS-1$
public static final String P_MISSING_VERSION_IMP_PKG = "compilers.p.missing-version-import-package"; //$NON-NLS-1$
public static final String P_MISSING_VERSION_REQ_BUNDLE = "compilers.p.missing-version-require-bundle"; //$NON-NLS-1$
public static final String P_MISSING_UPPER_VERSION_BOUND_REQ_BUNDLE = "compilers.p.missing-upper-version-bound-require-bundle"; //$NON-NLS-1$
public static final String P_MISSING_UPPER_VERSION_BOUND_IMP_PKG = "compilers.p.missing-upper-version-bound-import-package"; //$NON-NLS-1$
public static final String P_IMP_PKG_MISSING_VERSION_FOR_EXPORT_WITHOUT_VERSION = "compilers.p.import-package-missing-version-for-export-without-version"; //$NON-NLS-1$

/**
* schema preferences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ BundleErrorReporter_lazyStart_unsupported=The Eclipse-LazyStart header is not su
BundleErrorReporter_unsatisfiedConstraint=Unsatisfied constraint: ''{0}''
BundleErrorReporter_noExecutionEnvironmentSet=No required execution environment has been set
BundleErrorReporter_MissingVersion=''{0}'' is missing version constraint information
BundleErrorReporter_MissingUpperBoundForBundle=Bundle ''{0}'' is missing an upper bound on its version range. Consider adding a version range following OSGi semantic versioning principles.
BundleErrorReporter_MissingUpperBoundForPackage=Package ''{0}'' is missing an upper bound on its version range. Consider adding a version range following OSGi semantic versioning principles.
BundleErrorReporter_ImportPkgMissingVersionForExportWithoutVersion=Package ''{0}'' is imported without a version constraint, but the exporting bundle does not specify a version. Importing packages that are exported without a version is dangerous and may lead to runtime issues.
BundleErrorReporter_R4SyntaxInR3Bundle=The ''{0}'' header requires ''Bundle-ManifestVersion: 2''
BundleTextChangeListener_editNames_newLine=Add a new line at the end of the file
BundleTextChangeListener_editNames_remove=Remove "{0}" header
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,120 @@ public void tearDown() throws Exception {
}
}

@Test
public void testWarningOnMissingUpperBoundForRequireBundle() throws Exception {
ProjectUtils.createPluginProject(manifest.getProject().getName());

PDEModelUtility.modifyModel(new ModelModification(manifest) {
@Override
protected void modifyModel(IBaseModel model, IProgressMonitor monitor) throws CoreException {
IBundlePluginModelBase modelBase = (IBundlePluginModelBase) model;
IBundle bundle = modelBase.getBundleModel().getBundle();
// Set a version range without upper bound
bundle.setHeader(Constants.REQUIRE_BUNDLE, "org.eclipse.core.runtime;bundle-version=\"3.0.0\"");
}
}, null);

List<IMarker> markers = findMarkersWithCompilerKey("compilers.p.missing-upper-version-bound-require-bundle");
assertThat(markers).hasSize(1);
assertThat(markers.get(0).getAttribute(IMarker.MESSAGE, "")).contains("missing an upper bound");
}

@Test
public void testNoWarningWhenRequireBundleHasUpperBound() throws Exception {
ProjectUtils.createPluginProject(manifest.getProject().getName());

PDEModelUtility.modifyModel(new ModelModification(manifest) {
@Override
protected void modifyModel(IBaseModel model, IProgressMonitor monitor) throws CoreException {
IBundlePluginModelBase modelBase = (IBundlePluginModelBase) model;
IBundle bundle = modelBase.getBundleModel().getBundle();
// Set a proper version range with upper bound
bundle.setHeader(Constants.REQUIRE_BUNDLE, "org.eclipse.core.runtime;bundle-version=\"[3.0.0,4.0.0)\"");
}
}, null);

List<IMarker> markers = findMarkersWithCompilerKey("compilers.p.missing-upper-version-bound-require-bundle");
assertThat(markers).isEmpty();
}

@Test
public void testWarningOnMissingUpperBoundForImportPackage() throws Exception {
ProjectUtils.createPluginProject(manifest.getProject().getName());

PDEModelUtility.modifyModel(new ModelModification(manifest) {
@Override
protected void modifyModel(IBaseModel model, IProgressMonitor monitor) throws CoreException {
IBundlePluginModelBase modelBase = (IBundlePluginModelBase) model;
IBundle bundle = modelBase.getBundleModel().getBundle();
// Set a version range without upper bound
bundle.setHeader(Constants.IMPORT_PACKAGE, "org.osgi.framework;version=\"1.0.0\"");
}
}, null);

List<IMarker> markers = findMarkersWithCompilerKey("compilers.p.missing-upper-version-bound-import-package");
assertThat(markers).hasSize(1);
assertThat(markers.get(0).getAttribute(IMarker.MESSAGE, "")).contains("missing an upper bound");
}

@Test
public void testNoWarningWhenImportPackageHasUpperBound() throws Exception {
ProjectUtils.createPluginProject(manifest.getProject().getName());

PDEModelUtility.modifyModel(new ModelModification(manifest) {
@Override
protected void modifyModel(IBaseModel model, IProgressMonitor monitor) throws CoreException {
IBundlePluginModelBase modelBase = (IBundlePluginModelBase) model;
IBundle bundle = modelBase.getBundleModel().getBundle();
// Set a proper version range with upper bound
bundle.setHeader(Constants.IMPORT_PACKAGE, "org.osgi.framework;version=\"[1.0.0,2.0.0)\"");
}
}, null);

List<IMarker> markers = findMarkersWithCompilerKey("compilers.p.missing-upper-version-bound-import-package");
assertThat(markers).isEmpty();
}

@Test
public void testWarningOnRequireBundleWithOpenEndedRange() throws Exception {
ProjectUtils.createPluginProject(manifest.getProject().getName());

PDEModelUtility.modifyModel(new ModelModification(manifest) {
@Override
protected void modifyModel(IBaseModel model, IProgressMonitor monitor) throws CoreException {
IBundlePluginModelBase modelBase = (IBundlePluginModelBase) model;
IBundle bundle = modelBase.getBundleModel().getBundle();
// Set an open-ended range [1.0.0,)
bundle.setHeader(Constants.REQUIRE_BUNDLE, "org.eclipse.core.runtime;bundle-version=\"[1.0.0,)\"");
}
}, null);

List<IMarker> markers = findMarkersWithCompilerKey("compilers.p.missing-upper-version-bound-require-bundle");
assertThat(markers).hasSize(1);
}

@Test
public void testWarningOnImportPackageWithOpenEndedRange() throws Exception {
ProjectUtils.createPluginProject(manifest.getProject().getName());

PDEModelUtility.modifyModel(new ModelModification(manifest) {
@Override
protected void modifyModel(IBaseModel model, IProgressMonitor monitor) throws CoreException {
IBundlePluginModelBase modelBase = (IBundlePluginModelBase) model;
IBundle bundle = modelBase.getBundleModel().getBundle();
// Set an open-ended range [1.0.0,)
bundle.setHeader(Constants.IMPORT_PACKAGE, "org.osgi.framework;version=\"[1.0.0,)\"");
}
}, null);

List<IMarker> markers = findMarkersWithCompilerKey("compilers.p.missing-upper-version-bound-import-package");
assertThat(markers).hasSize(1);
}

private List<IMarker> findMarkersWithCompilerKey(String compilerKey) throws CoreException {
manifest.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
return Arrays.stream(manifest.findMarkers(PDEMarkerFactory.MARKER_ID, true, 0))
.filter(m -> compilerKey.equals(m.getAttribute("compilerKey", null))).toList();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1634,6 +1634,9 @@ compilers_p_exported_pkgs = Unexported pac&kage:
compilers_p_missing_exp_pkg = Missing versions on exported packages:
compilers_p_missing_imp_pkg = Missing versions on imported packages:
compilers_p_missing_require_bundle = Missing versions on required bundles:
compilers_p_missing_upper_bound_require_bundle = Missing upper bound on version range for required bundles:
compilers_p_missing_upper_bound_import_package = Missing upper bound on version range for imported packages:
compilers_p_import_package_missing_version_for_export_without_version = Import of package without version from bundle that exports without version:

compilers_s_create_docs = &Generate reference documentation from schemas
compilers_s_doc_folder = Do&cumentation folder:
Expand Down
Loading