Community
Participate
Working Groups
ImportRewrite is too smart for me. In order to fix 71707, I need to add an explicit import p.A to a CU with an existing import p.* (to avoid hiding of references to "A" in an existing CU "pack.User" when a new type "A" is added to package "pack"). setFindAmbiguosImports() and setFilterImplicitImports() don't allow me to configure this behavior - the added import p.A is always swallowed when p.* already exists. setFindAmbiguosImports(true) doesn't work in my use case, since the ambiguity is only to be introduced by the refactoring.
We should look at this during 3.8. Paste is also not working as expected and therefore the additional search to do the conflict resolution - which costs time (see e.g. bug 343837) - is useless.
setFindAmbiguosImports(boolean) also needs to get better Javadoc.
If you plan to include a fix for this in 3.8 M7, please adjust the target suitably, so it becomes easier to track.
Actually this seems to work fine in JDT/Core. ImportRewrite doesn't expose the API setFindAmbiguosImports(), but always does 'find ambiguous imports'. It does seem to work fine now.
Created attachment 213326 [details] Test case patch Test patch to confirm that problem mentioned in comment 0 works fine now.
The problem is that pack1.A doesn't exist in our use cases. At the time we call 'imports.addImport("pack2.A");', the conflict with pack1.A is not yet there. But after the refactoring is executed, there will be a pack1.A, so the explicit 'import pack2.A;' will be necessary. To reproduce in your test case, comment out the line 'pack1.createCompilationUnit("A.java", ...'. To solve the problem, we somehow need to inject knowledge about the soon-to-exist "pack1.A" into ImportRewriteAnalyzer#evaluateStarImportConflicts(..). At the end of this method, we have to iterate over the foundTypes to check whether one of the simpleNames is going to be available in any of the other packages. I first thought we could use an ImportRewriteContext to let the client decide this, but then I realized that the context is used to configure behavior on addImport(..), but it's not used when evaluating star imports. I think we need new API in ImportRewrite to solve this, e.g. a callback parameter for rewriteImports(..) that tells whether a type is found or not.
> I think we need new API in ImportRewrite to solve this, e.g. a callback > parameter for rewriteImports(..) that tells whether a type is found or not. Or we could store this information when adding imports, e.g. by having the ImportRewriteContext return a new RES_NAME_UNKNOWN_NEEDS_EXPLICIT_IMPORT which prevents the given type from being star-imported.
(In reply to comment #7) > > I think we need new API in ImportRewrite to solve this, e.g. a callback > > parameter for rewriteImports(..) that tells whether a type is found or not. > > Or we could store this information when adding imports, e.g. by having the > ImportRewriteContext return a new RES_NAME_UNKNOWN_NEEDS_EXPLICIT_IMPORT which > prevents the given type from being star-imported. Yes, this should be good approach. Considering that we are past the API freeze, do you want this in 3.8?
Let's defer this. Stability of import rewrite is paramount, and the blocking bugs didn't get much attention and were both filed by (former) JDT committers.
Markus, I am thinking of moving this out of 4.3. Do you have any objections?
Not a top priority, moving out of 4.3.
I'm adding this functionality to ImportRewriteAnalyzer as part of my work on it, and adding the new constant ImportRewriteContext.RES_NAME_UNKNOWN_NEEDS_EXPLICIT_IMPORT that Markus suggested.
(In reply to John Glassmyer from comment #12) > I'm adding this functionality to ImportRewriteAnalyzer as part of my work on > it, and adding the new constant > ImportRewriteContext.RES_NAME_UNKNOWN_NEEDS_EXPLICIT_IMPORT that Markus > suggested. So, can we assign this bug to you John?
(In reply to Dani Megert from comment #13) > So, can we assign this bug to you John? Yes.
Fixed via bug 430303.
Verified for 4.5 M6 with build I20150316-2000.
