Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 361079 - [rename] package rename corrupts files
Summary: [rename] package rename corrupts files
Status: RESOLVED NOT_ECLIPSE
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7.1   Edit
Hardware: PC Linux
: P3 major (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-16 05:09 EDT by Ferry Huberts CLA
Modified: 2011-10-20 14:01 EDT (History)
2 users (show)

See Also:


Attachments
the java code generating the changes (5.65 KB, text/plain)
2011-10-16 05:10 EDT, Ferry Huberts CLA
no flags Details
package rename, step 1 (136.15 KB, image/png)
2011-10-16 05:11 EDT, Ferry Huberts CLA
no flags Details
package rename, step 2 (167.10 KB, image/png)
2011-10-16 05:11 EDT, Ferry Huberts CLA
no flags Details
package rename, step 3 (178.32 KB, image/png)
2011-10-16 05:12 EDT, Ferry Huberts CLA
no flags Details
package rename, step 4 (177.81 KB, image/png)
2011-10-16 05:12 EDT, Ferry Huberts CLA
no flags Details
package rename, step 5 (172.10 KB, image/png)
2011-10-16 05:13 EDT, Ferry Huberts CLA
no flags Details
package rename, step 6 (177.69 KB, image/png)
2011-10-16 05:13 EDT, Ferry Huberts CLA
no flags Details
bnd.bnd file before rename (695 bytes, text/plain)
2011-10-16 05:13 EDT, Ferry Huberts CLA
no flags Details
bnd.bnd file after rename (corrupted) (715 bytes, text/plain)
2011-10-16 05:14 EDT, Ferry Huberts CLA
no flags Details
the new (working) rename code (5.43 KB, text/plain)
2011-10-20 11:35 EDT, Ferry Huberts CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ferry Huberts CLA 2011-10-16 05:09:33 EDT
Build Identifier: 20110916-0149

I'm a bndtools developer.
I'm using bndtools https://github.com/njbartlett/bndtools, b49ff4f611f40e39e71f4f5becf3193b115e885d.

I've replaced bndtools.core/src/bndtools/refactor/PkgRenameParticipant.java with the attached version of the file.

I run this setup of a chackout of https://github.com/fhuberts/jnotify, 62dc918cef67b703a8f58e2cbf16e7888abd3c84.

I now rename a package, and ask for a preview.
The preview shows the correct changes but the endresult is not correct.

I've attached screenshots of the rename procedure, the bnd.bnd file (before the rename) and the bnd.bnd.new file (after the rename)

clearly the bnd.bnd.new file does not correspond to the preview.
Since my code is not involved after the preview I've concluded that this must be an eclipse bug.

Reproducible: Always

Steps to Reproduce:
1. see details
2.
3.
Comment 1 Ferry Huberts CLA 2011-10-16 05:10:18 EDT
Created attachment 205272 [details]
the java code generating the changes
Comment 2 Ferry Huberts CLA 2011-10-16 05:11:07 EDT
Created attachment 205273 [details]
package rename, step 1
Comment 3 Ferry Huberts CLA 2011-10-16 05:11:32 EDT
Created attachment 205274 [details]
package rename, step 2
Comment 4 Ferry Huberts CLA 2011-10-16 05:12:00 EDT
Created attachment 205275 [details]
package rename, step 3
Comment 5 Ferry Huberts CLA 2011-10-16 05:12:26 EDT
Created attachment 205276 [details]
package rename, step 4
Comment 6 Ferry Huberts CLA 2011-10-16 05:13:01 EDT
Created attachment 205277 [details]
package rename, step 5
Comment 7 Ferry Huberts CLA 2011-10-16 05:13:30 EDT
Created attachment 205278 [details]
package rename, step 6
Comment 8 Ferry Huberts CLA 2011-10-16 05:13:55 EDT
Created attachment 205279 [details]
bnd.bnd file before rename
Comment 9 Ferry Huberts CLA 2011-10-16 05:14:20 EDT
Created attachment 205280 [details]
bnd.bnd file after rename (corrupted)
Comment 10 Dani Megert CLA 2011-10-17 03:08:43 EDT
> Since my code is not involved after the preview I've concluded that this must
> be an eclipse bug.
Not sure, as it obviously only happens with your participant.

Markus, please take a look.
Comment 11 Ferry Huberts CLA 2011-10-18 13:34:17 EDT
My system:
Fedora 15 x86_64
openjdk 1.6.0.0-59.1.10.3.fc15

Neil Bartlett just indicated that he could not reproduce this bug on Mac OS X Lion.

we're currently testing some other platform combinations
Comment 12 Markus Keller CLA 2011-10-18 13:54:47 EDT
Neil probably has the "[x] Rename subpackages" option disabled.

I didn't debug it, but I guess the problem is that your RenameParticipant is called multiple times (for each subpackage). Each time, you create a new TextFileChange that targets the same .bnd files. You create your ReplaceEdits based on the original version, which is why it looks correct in the preview. But after the first change has been executed, the content of the file has changed and the other changes are not valid any more.

One possible fix is to use RefactoringParticipant#getTextChange(Object) to get an existing TextChange. Then, you can add your edits to that Change.
Comment 13 Ferry Huberts CLA 2011-10-18 16:10:48 EDT
(In reply to comment #12)
> Neil probably has the "[x] Rename subpackages" option disabled.
> 
> I didn't debug it, but I guess the problem is that your RenameParticipant is
> called multiple times (for each subpackage). Each time, you create a new
> TextFileChange that targets the same .bnd files. You create your ReplaceEdits
> based on the original version, which is why it looks correct in the preview.
> But after the first change has been executed, the content of the file has
> changed and the other changes are not valid any more.
> 
> One possible fix is to use RefactoringParticipant#getTextChange(Object) to get
> an existing TextChange. Then, you can add your edits to that Change.

Win XP with latest jdk6 (b27) shows the same problem.

Will try your hint, but how to go about that? My participant is not reused but a new instance is used every time and I _really_ hate statics
Comment 14 Markus Keller CLA 2011-10-18 16:28:03 EDT
> Will try your hint, but how to go about that? My participant is not reused but
> a new instance is used every time and I _really_ hate statics

That's the beauty of RefactoringParticipant#getTextChange(Object). The implementation talks to the refactoring processor, and that one knows all changes. See also the Javadoc of RefactoringParticipant#createChange(..).

If you wanted to have a shared participant, you can also implement ISharableParticipant.

So, you can keep your statics aversion for something else ;-)
Comment 15 Ferry Huberts CLA 2011-10-20 11:35:52 EDT
Created attachment 205644 [details]
the new (working) rename code

(In reply to comment #14)
> > Will try your hint, but how to go about that? My participant is not reused but
> > a new instance is used every time and I _really_ hate statics
> 
> That's the beauty of RefactoringParticipant#getTextChange(Object). The
> implementation talks to the refactoring processor, and that one knows all
> changes. See also the Javadoc of RefactoringParticipant#createChange(..).

Thanks for your help, it works!

I have one issue left though:
The rename only runs in the project in which the package rename is done, while the package is referenced in bnd files in other projects.
How should we make Eclipse run the rename on these projects too?


(attached the new source file)
Comment 16 Markus Keller CLA 2011-10-20 11:50:11 EDT
(In reply to comment #15)
> The rename only runs in the project in which the package rename is done, while
> the package is referenced in bnd files in other projects.
> How should we make Eclipse run the rename on these projects too?

The Java rename processor finds references in dependent projects as well. Maybe your code should do the same? You are choosing your scope here:

    pkgFragment.getResource().getProject().accept(visitor, IContainer.NONE);
Comment 17 Ferry Huberts CLA 2011-10-20 14:01:58 EDT
(In reply to comment #16)
> (In reply to comment #15)
> > The rename only runs in the project in which the package rename is done, while
> > the package is referenced in bnd files in other projects.
> > How should we make Eclipse run the rename on these projects too?
> 
> The Java rename processor finds references in dependent projects as well. Maybe
> your code should do the same? You are choosing your scope here:
> 
>     pkgFragment.getResource().getProject().accept(visitor, IContainer.NONE);

cool.
many many thanks for the tips!

I did:

-        pkgFragment.getResource().getProject().accept(visitor, IContainer.NONE);
+
+        Set<IProject> visitedProjects = new HashSet<IProject>();
+
+        IProject containingProject = pkgFragment.getResource().getProject();
+        containingProject.accept(visitor, IContainer.NONE);
+        visitedProjects.add(containingProject);
+
+        for (IProject project : pkgFragment.getResource().getProject().getReferencingProjects()) {
+            project.accept(visitor, IContainer.NONE);
+            visitedProjects.add(project);
+        }
+
+        for (IProject project : pkgFragment.getResource().getProject().getReferencedProjects()) {
+            if (!visitedProjects.contains(project)) {
+                project.accept(visitor, IContainer.NONE);
+                visitedProjects.add(project);
+            }
+        }