Bug 113944 - [plan] Support for refactoring of JAR files
Summary: [plan] Support for refactoring of JAR files
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.2 M4   Edit
Assignee: Jerome Lanneluc CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-27 05:29 EDT by Tobias Widmer CLA
Modified: 2005-12-13 11:17 EST (History)
4 users (show)

See Also:


Attachments
API to set a raw classpath without modifying the .classpath file (4.40 KB, patch)
2005-11-23 07:54 EST, Jerome Lanneluc CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Widmer CLA 2005-10-27 05:29:52 EDT
In 3.2 we would like to support the following refactoring scenario:
- A project of a Java library v1.0 needs to be refactored
- Refactorings are recorded between v1.0 and v.1.1
- Developer ships JAR file v1.1 with recorded refactorings
- Client upgrades to v1.1
   - Recorded refactorings are executed on client workspace as well
   - JAR file is replaced and workspace is rebuilt

In order to support this, refactorings need a means to temporarily treat
a JAR file as writable. Due to the nature of JAR files, the only needed¨
operations for refactorings are:

- Add/Delete/Modify Type
- Add/Delete Package

The operations Add/Delete/Modify Type could be realized by the API
introduced with bug 110160, but requires major rewriting of existing 
refactorings.

The remaining operations on packages could be solved using different 
approaches:
1. Providing support for adding and deleting/hiding package fragments in JARs
    This approach probably requires substantial rewriting of existing 
refactorings
2. Creating the structural equivalent of (parts of) the JAR, hide the
   appropriate packages and put structural equivalent on class path.
    Class path change is problematic, but this approach requires only minor
    adaptions in the refactorings. We already have code for the structural
    equivalent.

Approach 1 is a special case of a class path change which could be leveraged:
- New source folder which receives the structural equivalent initially is empty
  when put on the class path
- Structure of the Java model of the structural equivalent below the package
  fragment root is the same as in the JAR, except that the structure is 
writable
  and IClassFile is replaced by ICompilationUnit
- All modifications are temporary, meaning that they are overridden by the 
replaced
  JAR file.
- Indexes must be updated for search engine, building is not desired
Comment 1 Tobias Widmer CLA 2005-11-07 06:23:05 EST
Jerome, What is the status of this item? In order to better estimate the 
effort on our side, I'd like to now into which directiony we have to go.
Comment 2 Philipe Mulet CLA 2005-11-16 11:48:00 EST
We should get interactions going soon, for UI adopting this before M4.
Comment 3 Jerome Lanneluc CLA 2005-11-17 10:20:06 EST
Sorry for the late reply Tobias. And now I'm trying to understand what you need... 

- Add/Delete/Modify Type
Bug 110160 gave a solution but it forces you to rewrite refactoring. Can you
explain what is bad about creating a class file working copy ? I assumed that
you already did that for regular compilation unit.

- Add/Delete Package
1. Same question as above: why do you have to rewrite refactoring if we allowed
to temporarily create/delete a package in a jar ?
2. What do you call a structural equivalent ? Do you still want to temporarily
change the classpath ?
Comment 4 Tobias Widmer CLA 2005-11-17 11:42:01 EST
Refactorings currently do not use working copy owners for change generation.
There is no need to do so. Only a few refactorings such as Extract Interface and
Rename Method internally use working copies to perform some analysis regarding
semantic shifts.
As soon as we want to use class file working copies, all refactorings had to be
rewritten to be working copy aware, which would cause major changes. Considering
the time frame we have, this is rather a no-go.
Assuming that the creation and deletion of packages in JARs is also based on
working copies, the same argument is valid here too.
My definition of a structural equivalent are basically type stubs which contain
stubbed members only (methods with default return statement, field declarations
(where necessary initialized with the default value), and stubbed types). JDT UI
already has the code to convert a set of package fragments to their structural
equivalent in a source folder. The JDT Core Disassembler produces output which
is quite similar to our generated type stubs.
Given that rewriting all our existing refactorings is clearly not an easy option
and the code to generate the structural equivalent already exists, temporarily
removing the JAR from the class path and putting the source folder with its
structural equivalent on the class path seems the best solution from our side.
What are your biggest concerns regarding a temporary class path change? Could we
leverage some of the special properties of this change described in comment 0?
If so, this class path change may be not so intrusive and far reaching as exspected.
Comment 5 Jerome Lanneluc CLA 2005-11-18 09:24:42 EST
So you need is be able to temporarily set the classpath, and refactoring will
generate the structural equivalent of the jar, right ?

Then what kind of operations do you need to perform with this temporary classpath ? 
1. Do you need to navigate the Java model (getChildren() or equivalent) ? 
2. Do you need to search in this classpath (using SearchEngine) ?
3. Do you need to compute type hierarchies ?
4. Do you need to create DOM ASTs ?
Comment 6 Tobias Widmer CLA 2005-11-18 09:39:47 EST
Yes, exactly. The JAR import operation first creates a source folder with the
structural equivalent of the old version of the JAR, puts the folder onto the
classpath, executes a series of refactorings, overwrites the old JAR with the
new version, and changes the classpath back to the state from before.

Even if we restrict ourselves to the most important (possibly API-breaking)
refactorings such as Move, Rename and Change Signature, we have to be able to
perform all four operations you listed in comment 5.
Comment 7 Jerome Lanneluc CLA 2005-11-18 10:56:28 EST
I'm investigating, but this is much more than what we anticipated during the JDT
summit. What worries me a lot is:
- how to hide this temporary classpath change from other clients (running in
other threads)
- managing indexes for this temporary classpath
- preventing the Java builder from seeing this changes (may not be an issue if
we take the workspace lock...)
Comment 8 Tobias Widmer CLA 2005-11-18 11:11:02 EST
I see your points. The whole refactoring process is exspected to run using
workspace lock (e.g an IWorkspaceRunnable or inside a JavaModelOperation)
Hopefully we can leverage some of the special properties in comment 0.
Comment 9 Jerome Lanneluc CLA 2005-11-21 05:53:43 EST
(In reply to comment #8)
> I see your points. The whole refactoring process is exspected to run using
> workspace lock (e.g an IWorkspaceRunnable or inside a JavaModelOperation)
> Hopefully we can leverage some of the special properties in comment 0.
I'm not sure what special properties we can leverage.
For now, it is a no-go from JDT Core on this change. Temporary changing the classpath would involve rewriting too much code in JDT Core.

All we can offer is a mechanism to hide/create packages in a JAR file.
Comment 10 Tobias Widmer CLA 2005-11-22 04:26:30 EST
On second thoughts, our main problem is the outgoing change caused by the writing of the .classpath file on classpath changes, and not the fact that the classpath change has to be hidden completely from clients. For us, it is perfectly ok if the change is visible to clients (the refactoring process is run in a wizard using the workspace lock). However, we do not want to cause outgoing changes.

I suppose this can be implemented more easily. We just need a way to declare to the Java Model that during the scope of this refactoring operation no classpath information is persisted in the workspace (which makes sense, as the classpath is to be reverted at the end of the operation anyway).

If you could provide us with a mechanism to hide packages in a JAR file, we could use a more incremental approach: As soon as a refactoring tries to modify content in a package, we just hide the package in the JAR file and create its structural equivalent in the source folder initially put onto the classpath. That way, the creation of the structual equivalent becomes more scalable.

Do you think this is feasible?
Comment 11 Jerome Lanneluc CLA 2005-11-23 07:54:32 EST
Created attachment 30480 [details]
API to set a raw classpath without modifying the .classpath file

The API IJavaProject#setRawClasspath(IClasspathEntry[],boolean,IProgressMonitor) allows you to set the classpath without touching the .classpath file. No optimization has been done at this stage, so expect re-indexing, etc.
Comment 12 Tobias Widmer CLA 2005-11-28 07:06:06 EST
This API works for us. But I think it would be reasonable to introduce its counterpart as well: Something like IJavaProject#resetClasspath(IProgressMonitor) which would reset the classpath to the state on disk, or do nothing if classpath and .classpath file are in sync.
Comment 13 Jerome Lanneluc CLA 2005-11-28 10:35:50 EST
Released proposed API and added tests ClasspathTests#testNoResourceChange01/04.

As for adding a resetClasspath method, this would be a helper for setRawClasspath(readRawClasspath(), false, monitor). This is so simple that it is not worth adding an extra API.
Comment 14 Jerome Lanneluc CLA 2005-11-28 10:36:31 EST
Please let me know if you still need to hide/show packages in jars.
Comment 15 Tobias Widmer CLA 2005-11-28 10:55:38 EST
Sure, its a simple helper, I missed that one. Hiding JARs is still needed, as we do not want to create the structural equivalent of a whole jar (which is not feasible for big ones), but rather convert only those packages needed by the refactorings to source.
Comment 16 Tobias Widmer CLA 2005-11-30 07:17:20 EST
Jerome, I think it would be more consistent to offer the following API:
IJavaProject#setRawClasspath(IClasspathEntry[],IPath,boolean,IProgressMonitor)
with IPath being the output location
Comment 17 Jerome Lanneluc CLA 2005-11-30 07:21:20 EST
Do you need to change the output location ?
Comment 18 Tobias Widmer CLA 2005-11-30 08:37:58 EST
Yes, I do (for projects with source folder = project folder)
Comment 19 Jerome Lanneluc CLA 2005-11-30 10:49:13 EST
Added IJavaProject#setRawClasspath(IClasspathEntry[], IPath, boolean, IProgressMonitor) to change the output location as well as the classpath without touching resources.
Added test ClasspathTests#testNoResourceChange05().
Comment 20 Jerome Lanneluc CLA 2005-12-01 13:09:48 EST
Would something like the following be ok for you ?
- IPackageFragmentRoot#hideBinaryPackageFragment(String packageName)
  That would act as if the package had been deleted, thus it would not appear in
  the model, in search, etc.
- IPackageFragmentRoot#restoreHiddenPackageFragments()
  You would call it at the end of refactoring to restore all package fragments
  that were hidden during your operation
Comment 21 Tobias Widmer CLA 2005-12-02 04:15:56 EST
Yes, IPackageFragmentRoot#hideBinaryPackageFragment(String packageName) is exactly what we need.
However, a method to restore the hidden fragments is not strictly required, since the JAR is replaced anyway before reverting back the classpath.
Comment 22 Jerome Lanneluc CLA 2005-12-02 04:38:15 EST
Actually, if you don't restore hidden package fragments, even if the jar is replaced at the end, the package fragments will still be hidden in the new jar.
Comment 23 Martin Aeschlimann CLA 2005-12-02 05:37:44 EST
I'm a bit scared with this API suggestion. It feels to me that this can introduce hidden state of packages set once disabled but some (contributed) plugin forgot to set back.
It feels that this is related to the existing excludion/inclusion patterns for source folders, or, in a way to the access restrictions.
Would a solution be to extend the current access restriction with a new state 'unvisible', resulting the compiler to to not find a type in a JAR anymore?

The advantage would be that everything is managed in the .classpath and we can do everything with the 'unsaved' classpath API.
Comment 24 Tobias Widmer CLA 2005-12-02 05:42:40 EST
I also agree that an API such as this can be dangerous (especially regarding comment 22).

Access rules or exclusion filters which would allow to exclude elements in a JAR down to package level or even deeper would work as well.
Comment 25 Jerome Lanneluc CLA 2005-12-02 06:13:18 EST
Hidding/restoring a package fragment is like becomeWorkingCopy/discardWorkingCopy. If a plugin forgets to discard a primary working copy, everyone will see the working copy for ever.
Moreover, if you set an exclusion filter in the .classpath and forget to remove it, you will be in the same unwanted state.
I'm not saying that using an exclusion filter is not a good idea, this is just to say that the "this API is dangerous" argument is not valid.

So if we added something on the library classpath entries, this could only be an exclusion filter (and not an access rule), as the Java model needs to hide the package fragments as well, and access rules don't hide packages, they just tag them as non-accessible when being resolved.

Of course this is a much harder problem, and would require much more work than hidding/restoring package fragments.
Comment 26 Tobias Widmer CLA 2005-12-02 06:17:29 EST
I am totally fine with the proposed API. Since I am running inside a wizard, it is guaranteed that the packages are unhidden at the end.
Comment 27 Philipe Mulet CLA 2005-12-02 06:39:05 EST
I agree that a long term solution aligning with exclusion rules and/or access rules would be preferrable. But it is unrealistic with 3.2 timeframe. 

Either we shoot for hidden packages, or we forget them, and you explode the JAR as a workaround until the better support comes (3.3). I would prefer latter option, as it wouldn't introduce transient APIs soon to be deprecated.
Is that really so bad ? I mean, how often do you need to hide packages ? Is that a common refactoring scenario ?
Comment 28 Tobias Widmer CLA 2005-12-02 06:54:56 EST
The scenario is not an every-day refactoring and performs quite well performance-wise. The current implementation greedily creates stubs for the entire JAR while importing. This works, but does not scale for bigger JARs (it takes about 30s to create stubs for the JDT UI JAR). Hiding packages selectively would improve performance since refactorings usually only affect a few packages of an entire JAR and a refactoring can estimate the extent of its changes.
The request for an API to set the classpath without persisting it is an enablement for the JAR Import Wizard to be introduced in M4. This has been released, so I suggest to defer the remaining items to 3.3 and strive for a more consistent solution using access rules or exclusion filters.
Comment 29 Jerome Lanneluc CLA 2005-12-06 10:45:13 EST
Thanks Tobias. I entered bug 119419 for the exclusion filter support on jar files. I'm closing this bug since you said that the "API to set the classpath without persisting it is an enablement for the JAR Import Wizard".
Comment 30 David Audel CLA 2005-12-13 11:17:56 EST
Verified for 3.2 M4 using build I20051213-0010