Skip to content

Commit ccc10e6

Browse files
HannesWellHannesWell
committed
Propose all available dependency versions in compiler error quick-fixes
When creating a quick-fix for a compiler error due to missing requirements, add all available versions of an importable package or requirable bundle, not only the latest one. Also add the version in the display string to allow the user to distinguish between versions.
1 parent c5f5c9e commit ccc10e6

File tree

4 files changed

+49
-38
lines changed

4 files changed

+49
-38
lines changed

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/java/FindClassResolutionsOperation.java‎

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import java.util.Arrays;
1717
import java.util.Collections;
18+
import java.util.Comparator;
1819
import java.util.HashMap;
1920
import java.util.HashSet;
2021
import java.util.LinkedHashSet;
@@ -44,6 +45,7 @@
4445
import org.eclipse.jdt.core.search.SearchRequestor;
4546
import org.eclipse.jdt.ui.text.java.IJavaCompletionProposal;
4647
import org.eclipse.jface.operation.IRunnableWithProgress;
48+
import org.eclipse.osgi.service.resolver.BaseDescription;
4749
import org.eclipse.osgi.service.resolver.BundleDescription;
4850
import org.eclipse.osgi.service.resolver.BundleSpecification;
4951
import org.eclipse.osgi.service.resolver.ExportPackageDescription;
@@ -188,6 +190,9 @@ public FindClassResolutionsOperation(IProject project, CompilationUnit cu, Strin
188190
fCollector = collector;
189191
}
190192

193+
private static final Comparator<BaseDescription> BY_DESCENDING_VERSION = Comparator
194+
.comparing(BaseDescription::getVersion).reversed();
195+
191196
@Override
192197
public void run(IProgressMonitor monitor) {
193198
int idx = fClassName.lastIndexOf('.');
@@ -198,7 +203,7 @@ public void run(IProgressMonitor monitor) {
198203
}
199204

200205
Set<IPackageFragment> packagesToExport = new HashSet<>();
201-
Map<String, ExportPackageDescription> validPackages = getValidPackages(typeName, fClassName, packageName,
206+
Map<String, List<ExportPackageDescription>> validPackages = getValidPackages(typeName, fClassName, packageName,
202207
packagesToExport, monitor);
203208
if (validPackages != null) {
204209

@@ -215,32 +220,35 @@ public void run(IProgressMonitor monitor) {
215220
&& (allowMultipleFixes || !fCollector.isDone());) {
216221
var entry = validPackagesIter.next();
217222
String qualifiedType = entry.getKey();
218-
ExportPackageDescription currentPackage = entry.getValue();
223+
List<ExportPackageDescription> currentPackages = entry.getValue();
219224
// if package is already visible, skip over
220-
if (visiblePackages.contains(currentPackage)) {
221-
continue;
222-
}
223-
// if currentPackage will resolve class and is valid, pass it to collector
224-
fCollector.addResolutionModification(fProject, currentPackage, fCompilationUnit, qualifiedType);
225+
currentPackages.stream().sorted(BY_DESCENDING_VERSION).filter(p -> !visiblePackages.contains(p))
226+
.forEach(p -> {
227+
// If package will resolve class and is valid, pass it
228+
fCollector.addResolutionModification(fProject, p, fCompilationUnit, qualifiedType);
229+
});
225230
}
226231
// additionally add require bundle proposals
227232
Set<String> bundleNames = getCurrentBundleNames();
228-
validPackages.forEach((key, currentPackage) -> {
229-
BundleDescription desc = currentPackage.getExporter();
230-
// Ignore already required bundles and duplicate proposals (currently we do not consider version constraints)
231-
if (desc != null && !bundleNames.contains(desc.getName())) {
232-
fCollector.addRequireBundleModification(fProject, currentPackage, 3, fCompilationUnit, key);
233-
bundleNames.add(desc.getName());
234-
}
233+
validPackages.forEach((key, currentPackages) -> {
234+
currentPackages.stream().sorted(BY_DESCENDING_VERSION).forEach(currentPackage -> {
235+
BundleDescription desc = currentPackage.getExporter();
236+
// Ignore already required bundles and duplicate proposals
237+
// (currently we do not consider version constraints)
238+
if (desc != null && !bundleNames.contains(desc.getName())) {
239+
fCollector.addRequireBundleModification(fProject, currentPackage, 3, fCompilationUnit, key);
240+
bundleNames.add(desc.getName());
241+
}
242+
});
235243
});
236244
}
237245
}
238246

239-
private Map<String, ExportPackageDescription> getValidPackages(String typeName, String qualifiedTypeToImport,
247+
private Map<String, List<ExportPackageDescription>> getValidPackages(String typeName, String qualifiedTypeToImport,
240248
String packageName, Set<IPackageFragment> packagesToExport, IProgressMonitor monitor) {
241249
SubMonitor subMonitor = SubMonitor.convert(monitor, 3);
242250

243-
Map<String, ExportPackageDescription> validPackages = null;
251+
Map<String, List<ExportPackageDescription>> validPackages = null;
244252
ImportPackageSpecification[] importPkgs = null;
245253
IPluginModelBase model = PluginRegistry.findModel(fProject);
246254
if (model != null && model.getBundleDescription() != null) {
@@ -252,7 +260,7 @@ private Map<String, ExportPackageDescription> getValidPackages(String typeName,
252260
if (packageName != null) {
253261
if (!isImportedPackage(packageName, importPkgs)) {
254262
List<ExportPackageDescription> packages = getValidPackages(packageName);
255-
validPackages = !packages.isEmpty() ? Map.of(qualifiedTypeToImport, packages.getLast()) : Map.of();
263+
validPackages = !packages.isEmpty() ? Map.of(qualifiedTypeToImport, packages) : Map.of();
256264
}
257265
subMonitor.split(1);
258266
} else {
@@ -276,7 +284,7 @@ private Map<String, ExportPackageDescription> getValidPackages(String typeName,
276284
* if no valid package to import was found
277285
* @return the set of packages to import
278286
*/
279-
private Map<String, ExportPackageDescription> findValidPackagesContainingSimpleType(String aTypeName,
287+
private Map<String, List<ExportPackageDescription>> findValidPackagesContainingSimpleType(String aTypeName,
280288
ImportPackageSpecification[] importPkgs, Set<IPackageFragment> packagesToExport, IProgressMonitor monitor) {
281289
SubMonitor subMonitor = SubMonitor.convert(monitor);
282290

@@ -334,13 +342,10 @@ public void acceptSearchMatch(SearchMatch aMatch) throws CoreException {
334342
}
335343

336344
// finally create the list of ExportPackageDescriptions
337-
Map<String, ExportPackageDescription> exportDescriptions = new HashMap<>(packages.size());
338-
ExportPackageDescription[] knownPackages = state.getExportedPackages();
339-
for (ExportPackageDescription knownPackage : knownPackages) {
340-
if (packages.containsKey(knownPackage.getName())) {
341-
exportDescriptions.put(qualifiedTypeNames.get(knownPackage.getName()), knownPackage);
342-
}
343-
}
345+
Map<String, List<ExportPackageDescription>> exportDescriptions = Arrays
346+
.stream(state.getExportedPackages()).filter(p -> packages.containsKey(p.getName()))
347+
.collect(Collectors.groupingBy(p -> qualifiedTypeNames.get(p.getName())));
348+
344349
if (exportDescriptions.isEmpty()) {
345350
// no packages to import found, maybe there are packages to export
346351
packagesToExport.addAll(packages.values());

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/java/JavaResolutionFactory.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import org.eclipse.swt.graphics.Point;
6363
import org.eclipse.text.edits.TextEdit;
6464
import org.osgi.framework.Constants;
65+
import org.osgi.framework.Version;
6566
import org.osgi.framework.VersionRange;
6667

6768
/**
@@ -248,8 +249,10 @@ public String getDescription() {
248249
@Override
249250
public String getName() {
250251
BundleDescription requiredBundle = getChangeObject();
251-
return MessageFormat.format(!isUndo() ? PDEUIMessages.UnresolvedImportFixProcessor_0
252-
: PDEUIMessages.UnresolvedImportFixProcessor_1, requiredBundle.getName());
252+
return MessageFormat.format(
253+
!isUndo() ? PDEUIMessages.UnresolvedImportFixProcessor_0
254+
: PDEUIMessages.UnresolvedImportFixProcessor_1,
255+
requiredBundle.getName(), getRequirementVersion(requiredBundle.getVersion()));
253256
}
254257

255258
@Override
@@ -330,8 +333,10 @@ public Image getImage() {
330333
@Override
331334
public String getName() {
332335
ExportPackageDescription importedPackage = getChangeObject();
333-
return MessageFormat.format(!isUndo() ? PDEUIMessages.UnresolvedImportFixProcessor_3
334-
: PDEUIMessages.UnresolvedImportFixProcessor_4, importedPackage.getName());
336+
return MessageFormat.format(
337+
!isUndo() ? PDEUIMessages.UnresolvedImportFixProcessor_3
338+
: PDEUIMessages.UnresolvedImportFixProcessor_4,
339+
importedPackage.getName(), getRequirementVersion(importedPackage.getVersion()));
335340
}
336341

337342
@Override
@@ -342,7 +347,10 @@ public Object getModifiedElement() {
342347
}
343348
return super.getModifiedElement();
344349
}
350+
}
345351

352+
private static Version getRequirementVersion(Version bundleVersion) {
353+
return new Version(bundleVersion.getMajor(), bundleVersion.getMinor(), 0);
346354
}
347355

348356
private static class ExportPackageChange extends AbstractManifestChange<IPackageFragment> {

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/correction/java/QuickFixProcessor.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,13 +270,11 @@ public void addResolutionModification(IProject project, ExportPackageDescription
270270
// really useful as import package is computed automatically
271271
return;
272272
}
273+
var change = JavaResolutionFactory.createImportPackageChange(project, desc, cu, typeToImport);
274+
result.add(JavaResolutionFactory.createJavaCompletionProposal(change, 20));
273275
// guard against multiple import package resolutions for the
274276
// same package
275-
if (addedImportPackageResolutions.add(desc.getName())) {
276-
var change = JavaResolutionFactory.createImportPackageChange(project, desc, cu, typeToImport);
277-
result.add(JavaResolutionFactory.createJavaCompletionProposal(change, 20));
278-
isDone = true;
279-
}
277+
isDone |= addedImportPackageResolutions.add(desc.getName());
280278
}
281279

282280
@Override

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,12 +1290,12 @@ Menus_new_label = &New
12901290

12911291
Actions_Feature_OpenProjectWizardAction = &New Feature Project...
12921292
Actions_Site_OpenProjectWizardAction = &New Update Site...
1293-
UnresolvedImportFixProcessor_1=Remove ''{0}'' from required bundles
1293+
UnresolvedImportFixProcessor_1=Remove ''{0} - {1}'' from required bundles
12941294
UnresolvedImportFixProcessor_2=Adds a bundle that exports the needed package to the list of required bundles for this project and adds the related import statement to this Java class.
1295-
UnresolvedImportFixProcessor_3=Add ''{0}'' to imported packages
1296-
UnresolvedImportFixProcessor_4=Remove ''{0}'' from imported packages
1295+
UnresolvedImportFixProcessor_3=Add ''{0} - {1}'' to imported packages
1296+
UnresolvedImportFixProcessor_4=Remove ''{0} - {1}'' from imported packages
12971297
UnresolvedImportFixProcessor_5=Adds an import package dependency for this project and adds the related import statement to this Java class.
1298-
UnresolvedImportFixProcessor_0=Add ''{0}'' to required bundles
1298+
UnresolvedImportFixProcessor_0=Add ''{0} - {1}'' to required bundles
12991299
UpdateClasspathJob_error_title = Update Classpaths
13001300
UpdateClasspathJob_error_message = Updating failed. See log for details.
13011301
UpdateClasspathResolution_label=Update the classpath and compliance settings

0 commit comments

Comments
 (0)