Bug 303776

Summary:

Member types imports are removed too aggressively

Product:

[Eclipse Project] JDT

Reporter:

Olivier Thomann <Olivier_Thomann>

Component:

Core

Assignee:

Olivier Thomann <Olivier_Thomann>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

aniefer, daniel_megert, markus.kell.r, pascal, thomas

Version:

3.6

Target Milestone:

3.6 M6

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Proposed fix + regression test	none
Proposed fix + regression test	none

Description Olivier Thomann

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

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

2010-02-24 12:45:51 EST

Created attachment 160089 [details]
Proposed fix + regression test

Comment 3 Markus Keller

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

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

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

2010-02-24 14:50:41 EST

Created attachment 160106 [details]
Proposed fix + regression test

Comment 7 Olivier Thomann

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

2010-02-26 20:54:49 EST

*** Bug 304111 has been marked as a duplicate of this bug. ***

Comment 9 Frederic Fusier

2010-03-09 10:00:30 EST

Verified for 3.6M6 using I20100307-2000 build.