Handle deletion scenario for cleaning up empty exported packages by nburnwal09 · Pull Request #2292 · eclipse-pde/eclipse.pde

nburnwal09 · 2026-04-17T04:54:41Z

Enhancement added for handling class/type deletion scenario.
When all classes in a package are deleted via delete operation, the package(having no java files or any other resources) should be automatically removed from the Export-Package header in MANIFEST.MF if it was previously exported.

Addressing #2290

type.delete.refactor.mov

preview.part.mov

nburnwal09 · 2026-04-17T04:59:07Z

This is an enhancement to the previous task mentioned here

github-actions · 2026-04-17T05:36:41Z

Test Results

126 files + 42 126 suites +42 38m 28s ⏱️ + 16m 16s
3 502 tests ± 0 3 448 ✅ ± 0 54 💤 ± 0 0 ❌ ±0
9 321 runs +3 107 9 191 ✅ +3 064 130 💤 +43 0 ❌ ±0

Results for commit 82b820a. ± Comparison against base commit 41cc62c.

♻️ This comment has been updated with latest results.

nburnwal09 · 2026-04-23T06:51:07Z

@laeubi
Kindly review this PR.
Addressed the earlier mentioned scenario here

noopur2507 · 2026-04-30T10:15:05Z

+					}
+				}
+			} catch (CoreException e) {
+				e.printStackTrace();


Printing the stack trace instead of proper logging or handling the issue can cause the action to fail silently.

noopur2507 · 2026-04-30T10:27:12Z

+				if (member instanceof IFile memberFile) {
+					String fileName = memberFile.getName();
+					String extension = memberFile.getFileExtension();
+					if ("class".equals(extension)) { //$NON-NLS-1$


Please check whether any constants exist in the PDE code for the file extensions that can be reused here, or create them at the class level instead of hardcoding them here.

noopur2507 · 2026-04-30T10:31:53Z

+	}
+
+	@Override
+	public void addElement(Object element, RefactoringArguments arguments) {


Can the "element" be of a type other than IType and IPackageFragment? That should be handled in the 'else' case (probably via logging).

noopur2507 · 2026-04-30T10:36:15Z

 ManifestEditorContributor_externStringsActionName=Externalize Strings...

 ManifestTypeRenameParticipant_composite=Rename classes referenced in plug-in manifest files
+ManifestTypeDeleteParticipant_composite=Delete classes referenced in plug-in manifest files


Check the usage of this message and update it, as it's talking about deleting class references from the manifest, which isn't the case in this PR.

noopur2507 · 2026-04-30T10:48:38Z

+	@Override
+	public void addElement(Object element, RefactoringArguments arguments) {
+		if (element instanceof IType type) {
+			fElements.put(element, type.getFullyQualifiedName());


It is better to initialize "fElements" at declaration time to prevent any potential NPE here. Also, initializing it to empty in ManifestTypeDeleteParticipant#initialize will break the sharing behavior of ISharableParticipant in my view.

noopur2507 · 2026-04-30T10:53:03Z

+	@Override
+	protected boolean initialize(Object element) {
+		if (element instanceof IType type) {
+			IJavaProject javaProject = (IJavaProject) type.getAncestor(IJavaElement.JAVA_PROJECT);


"getAncestor" can return null.

noopur2507 · 2026-04-30T10:54:20Z

+			IJavaProject javaProject = (IJavaProject) type.getAncestor(IJavaElement.JAVA_PROJECT);
+			IProject project = javaProject.getProject();
+			if (WorkspaceModelManager.isPluginProject(project)) {
+				fProject = javaProject.getProject();


"javaProject.getProject()" is called twice. Here and at line 45.

noopur2507 · 2026-04-30T10:59:32Z

+			IProject project = javaProject.getProject();
+			if (WorkspaceModelManager.isPluginProject(project)) {
+				fProject = javaProject.getProject();
+				fElements = new HashMap<>();


See PDEDeleteParticipant, line 46 comment.

noopur2507 · 2026-04-30T11:12:39Z

+	 */
+	private boolean willPackageBeEmpty(IPackageFragment pkg, List<IType> deletedTypes) throws CoreException {
+		IJavaElement[] javaChildren = pkg.getChildren();
+		if (javaChildren.length > deletedTypes.size()) {


I think the entire logic here assumes 1:1 mapping between IType and compilation units, which is not the case in JDT. For inner classes, multiple types exist in the same compilation unit. Deleting one inner class shouldn't mark the entire .java file as deleted. The logic should be updated to handle this.

nburnwal09 · 2026-05-02T05:26:15Z

@noopur2507
I have addressed all of your comments. Kindly review.

HannesWell

Thank you @nburnwal09 for this.
In general it looks good and works well.

But I have multiple comments below to stream-line the code.
Furthermore please make sure to properly format all new files and edited lines.

Additionally, do you think it makes sense to include the extra change from #2312 here too?
Then the changes could be reviewed together and more holistically.

nburnwal09 · 2026-05-05T04:30:24Z

Additionally, do you think it makes sense to include the extra change from #2312 here too? Then the changes could be reviewed together and more holistically.

I realised about package deletion part later(after raising this PR). Also as that is for package deletion (although the goal of removing the package from manifest is the same), I thought of taking that as a different commit.
But if you suggest, I will merge the change here so that it can be reviewed together.

HannesWell · 2026-05-05T04:47:39Z

But if you suggest, I will merge the change here so that it can be reviewed together.

In this case I think it's similar and related enough to be together in one change.
Of course if you strongly prefer to keep it separated, that's also fine for me. It's just a suggestion from my side. :)

nburnwal09 · 2026-05-05T05:59:19Z

But if you suggest, I will merge the change here so that it can be reviewed together.

In this case I think it's similar and related enough to be together in one change. Of course if you strongly prefer to keep it separated, that's also fine for me. It's just a suggestion from my side. :)

Sure @HannesWell I will merge the changes in this PR.
Then I will make suggested updates. Thank you.

nburnwal09 · 2026-05-06T10:58:20Z

@HannesWell
I have merged the package deletion part and have addressed all your suggestions.
Kindly review them.

HannesWell

Thank you @nburnwal09 for the updates, they look good.
I made a second pass through this considering the now included ManifestPackageDeleteParticipant and also looked again at the previous changes.
Please see the comments below.
With them resolved, I expect this to be ready.

Please squash all changes into one commit named, e.g.:
Handle class and package deletion for removing empty exported packages.

Furthermore, just a nitpick, please add new lines at the end of the new files. Github/git shows annoying markers at files without newline at the end.

nburnwal09 · 2026-05-15T04:42:56Z

@HannesWell
Sorry for the delay. I was occupied with a hackathon at my company and couldn’t focus on this earlier.
I have now addressed all the comments. Kindly review the changes at your convenience.

HannesWell · 2026-05-15T06:23:01Z

Sorry for the delay. I was occupied with a hackathon at my company and couldn’t focus on this earlier.
I have now addressed all the comments. Kindly review the changes at your convenience.

No problem and thanks for the update. I plan to review it later today.

@MohananRahul could you please hold-off M3 promotion planed for today? I'd like to have this (and maybe another change) included. I can run the promotion myself later today or tomorrow morning (if an extra I-build is not reasonable).

HannesWell

Thank you for all the updates. This is a great enhancement and I'm looking forward to have it.
It looks very good now and is ready for submission (just in time for M3).

nburnwal09 force-pushed the del_pkgEmpty branch from de3f8af to 83fbd43 Compare April 17, 2026 06:16

gireeshpunathil approved these changes Apr 20, 2026

View reviewed changes

nburnwal09 force-pushed the del_pkgEmpty branch from 83fbd43 to 5cbdda6 Compare April 24, 2026 07:44

nburnwal09 mentioned this pull request Apr 24, 2026

Handle package deletion scenario for cleaning up exported packages #2312

Closed

noopur2507 requested changes Apr 30, 2026

View reviewed changes

nburnwal09 force-pushed the del_pkgEmpty branch from 43fa845 to e964c59 Compare May 2, 2026 05:15

nburnwal09 force-pushed the del_pkgEmpty branch from e964c59 to a308c4e Compare May 2, 2026 05:49

HannesWell reviewed May 3, 2026

View reviewed changes

noopur2507 approved these changes May 5, 2026

View reviewed changes

nburnwal09 changed the title ~~Handle class deletion scenario for cleaning up empty exported packages~~ Handle deletion scenario for cleaning up empty exported packages May 6, 2026

HannesWell reviewed May 9, 2026

View reviewed changes

nburnwal09 force-pushed the del_pkgEmpty branch 2 times, most recently from e69bc62 to 83389c3 Compare May 15, 2026 04:39

HannesWell force-pushed the del_pkgEmpty branch from 83389c3 to 7cb5117 Compare May 15, 2026 15:27

Handle class and package deletion for removing empty exported packages

82b820a

HannesWell force-pushed the del_pkgEmpty branch from 7cb5117 to 82b820a Compare May 15, 2026 21:30

HannesWell approved these changes May 15, 2026

View reviewed changes

HannesWell merged commit 4074bd4 into eclipse-pde:master May 15, 2026
16 of 19 checks passed

Conversation

nburnwal09 commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nburnwal09 commented Apr 17, 2026

Uh oh!

github-actions Bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

nburnwal09 commented Apr 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nburnwal09 commented May 2, 2026

Uh oh!

HannesWell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nburnwal09 commented May 5, 2026

Uh oh!

HannesWell commented May 5, 2026

Uh oh!

nburnwal09 commented May 5, 2026

Uh oh!

nburnwal09 commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HannesWell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nburnwal09 commented May 15, 2026

Uh oh!

HannesWell commented May 15, 2026

Uh oh!

HannesWell left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

nburnwal09 commented Apr 17, 2026 •

edited

Loading

github-actions Bot commented Apr 17, 2026 •

edited

Loading

nburnwal09 commented May 6, 2026 •

edited

Loading

HannesWell left a comment •

edited

Loading