Bug 47509 - Refactor name of package fragment does not handle on demand import
Summary: Refactor name of package fragment does not handle on demand import
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 2.1.2   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 2.1.3   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 29600 (view as bug list)
Depends on:
Blocks:
 
Reported: 2003-11-26 07:33 EST by Philipe Mulet CLA
Modified: 2005-09-06 11:45 EDT (History)
3 users (show)

See Also:


Attachments
Patch for jdt.ui / 2.1.2 (16.72 KB, patch)
2003-12-09 14:45 EST, Markus Keller CLA
no flags Details | Diff
Patch for jdt.ui.tests.refactoring / 2.1.2 (22.02 KB, patch)
2003-12-09 14:45 EST, Markus Keller CLA
no flags Details | Diff
2nd Patch for jdt.ui / 2.1.2 (5.20 KB, patch)
2003-12-11 10:03 EST, Markus Keller CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philipe Mulet CLA 2003-11-26 07:33:01 EST
Build Eclipse 2.1.2

On the following setup, select package fragment 'p' located in project B and 
rename it into 'q'. It will rename the 'import p.*;' statement, but did not 
realize there was still a use for it due to other fragment.
It should have inserted an extra import during the rename action.

Project A
 +- package p
      +- A.java

Project B prereqs A
 +- package p
 |    +- B.java
 +- package c
      +- C.java
           package c;
           import p.*;
           public class C {
           }
Comment 1 Martin Aeschlimann CLA 2003-11-26 10:13:23 EST
Markus, can you have a look? Philippe thinks this might be a 2.1.3 candidate.
Comment 2 Dirk Baeumer CLA 2003-11-26 11:47:02 EST
Some comments:

- fix will not be trivial. Currently rename doesn't do any AST analysis,
  which is verly likely required to solve the problem.

- we should only do some special analysis if two package fragments with the
  same name exists. This esures that we don't break the 98% case in 2.1.2

- the are some tricky corner cases in the case where two classes with the same
  name in both package exist

  Project A
    +- package p
       +- A.java

  Project B
    +- package p
       +- A.java

  depending on the class path renaming one of the p packages might change
  the semantic of the program (a different class A gets visible).
  For 2.1.x we should issue an error if this scenario is detected. We should
  not fix the code.
Comment 3 Philipe Mulet CLA 2003-11-26 15:11:37 EST
Couldn't you go for something somewhat simpler ? Perform the package rename, 
then validate. If an error is detected, then insert the original import back 
and check again. So you would only do extra work on advanced scenario.

Comment 4 Philipe Mulet CLA 2003-11-26 17:48:44 EST
Or what about always inserting a new renamed on demand import, and then enable 
the 'unused import' diagnosis when doing the validation. It would tell you if 
the old one is used any longer...
Comment 5 Dirk Baeumer CLA 2003-11-27 09:07:50 EST
The trick with the "unused import" sounds promising.

Here is what our current plan is:

- first look if a second package with the same name exists.
- then do a reference search for all types in the second package
- check if the set of affected CU's is disjoint. If so, no further action
  is needed.
- if CUs exist that import from both package, some "further action is 
  required. This can either be:
  - inspect the search results and check if all type references are fully 
    qualified. If so, we don't need to do anything. If not, determine if the 
    type is imported via a star import. If so, add another star import.
  - do some AST analysis.
  - do the change in memory and let the compiler decide if the import is 
    needed.
Comment 6 Philipe Mulet CLA 2003-11-27 09:28:02 EST
Once you know a subsequent fragment exists, you could simply notice that the 
set of references to the subsequent fragment intersects on one of the reference 
(can only be the on-demand import). If you have one, then the unused import 
detection should be enough (or whatever you prefer).
Comment 7 Dirk Baeumer CLA 2003-11-27 10:04:57 EST
This is what I meant with "if the set of CU's is disjoint". In this case there 
isn't any CU that imports classes from both package fragments.
Comment 8 Markus Keller CLA 2003-12-09 14:45:12 EST
Created attachment 7093 [details]
Patch for jdt.ui / 2.1.2
Comment 9 Markus Keller CLA 2003-12-09 14:45:52 EST
Created attachment 7094 [details]
Patch for jdt.ui.tests.refactoring / 2.1.2
Comment 10 Markus Keller CLA 2003-12-09 14:53:37 EST
Fix released to R2_1_maintenance branch.
Comment 11 Markus Keller CLA 2003-12-11 10:03:27 EST
Created attachment 7123 [details]
2nd Patch for jdt.ui / 2.1.2

An additional fix for situations with multiple projects.
Relased to R2_1_maintenance branch.
Comment 12 Markus Keller CLA 2004-02-19 15:53:04 EST
Also fixed in 3.0 stream.
Comment 13 Martin Aeschlimann CLA 2004-03-02 05:02:59 EST
verified is in M-M20040225-200402251535 (markus)
Comment 14 Markus Keller CLA 2005-09-06 11:45:35 EDT
*** Bug 29600 has been marked as a duplicate of this bug. ***