Bug 303776 - Member types imports are removed too aggressively
Summary: Member types imports are removed too aggressively
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.6 M6   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 304111 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-02-24 10:49 EST by Olivier Thomann CLA
Modified: 2010-03-09 10:00 EST (History)
5 users (show)

See Also:

Attachments
Proposed fix + regression test (6.61 KB, patch)
2010-02-24 12:45 EST, Olivier Thomann CLA 		no flags Details | Diff
Proposed fix + regression test (7.19 KB, patch)
2010-02-24 14:50 EST, Olivier Thomann CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Thomann CLA 2010-02-24 10:49:02 EST 
Member types imports are removed too aggressively.
When using:
import java.util.*;
import java.util.Map.Entry;

The latter one should not be removed as Entry could then not be resolved anymore in the compilation unit.

This bug is now visible because java.util.* and java.util.Map.Entry are now within the same import group.
Comment 1 Dani Megert CLA 2010-02-24 11:40:01 EST 
To reproduce, the 'Organize Imports' preference for number of needed imports must be set to something low, e.g. 0.
Comment 2 Olivier Thomann CLA 2010-02-24 12:45:51 EST 
Created attachment 160089 [details]
Proposed fix + regression test
Comment 3 Markus Keller CLA 2010-02-24 13:26:33 EST 
I quickly glanced over the patch (didn't try it out). If I understood that right, ImportRewriteAnalyzer.ImportDeclEntry#getSimpleName() is now actually returning the typeQualifiedName, so it should be renamed.

But I see a problem in ImportRewriteAnalyzer#evaluateStarImportConflicts(..) then, because there, the "simple" names are stored in simpleTypeNames, which is passed to a SearchEngine#searchAllTypeNames(..) method that only expects simple names.
Comment 4 Olivier Thomann CLA 2010-02-24 13:39:18 EST 
(In reply to comment #3)
> I quickly glanced over the patch (didn't try it out). If I understood that
> right, ImportRewriteAnalyzer.ImportDeclEntry#getSimpleName() is now actually
> returning the typeQualifiedName, so it should be renamed.
I'll check that.
 
> But I see a problem in ImportRewriteAnalyzer#evaluateStarImportConflicts(..)
> then, because there, the "simple" names are stored in simpleTypeNames, which is
> passed to a SearchEngine#searchAllTypeNames(..) method that only expects simple
> names.
Ok, I'll take a look. Thanks.
Comment 5 Olivier Thomann CLA 2010-02-24 14:49:55 EST 
(In reply to comment #3)
> I quickly glanced over the patch (didn't try it out). If I understood that
> right, ImportRewriteAnalyzer.ImportDeclEntry#getSimpleName() is now actually
> returning the typeQualifiedName, so it should be renamed.
I would say it actually returns the type name inside the package.
I create a new method called:
getTypeQualifiedName();

> But I see a problem in ImportRewriteAnalyzer#evaluateStarImportConflicts(..)
> then, because there, the "simple" names are stored in simpleTypeNames, which is
> passed to a SearchEngine#searchAllTypeNames(..) method that only expects simple
> names.
I am using the new method only when computing the remaining imports for a group of imports and the threshold has been reached.
Comment 6 Olivier Thomann CLA 2010-02-24 14:50:41 EST 
Created attachment 160106 [details]
Proposed fix + regression test
Comment 7 Olivier Thomann CLA 2010-02-24 14:57:26 EST 
Released for 3.6M6.
Regression test added in:
org.eclipse.jdt.core.tests.rewrite.describing.ImportRewriteTest#testAddImports5
Comment 8 Olivier Thomann CLA 2010-02-26 20:54:49 EST 
*** Bug 304111 has been marked as a duplicate of this bug. ***
Comment 9 Frederic Fusier CLA 2010-03-09 10:00:30 EST 
Verified for 3.6M6 using I20100307-2000 build.