Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 319860 - [organize imports] Unused import added for reference to nested type from Javadoc
Summary: [organize imports] Unused import added for reference to nested type from Javadoc
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 major with 1 vote (vote)
Target Milestone: 3.6.1   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 329921 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-07-14 10:29 EDT by Dani Megert CLA
Modified: 2010-11-10 11:54 EST (History)
6 users (show)

See Also:
markus.kell.r: review+
daniel_megert: review+


Attachments
Proposed fix (1.08 KB, patch)
2010-07-15 12:09 EDT, Olivier Thomann CLA
no flags Details | Diff
Fix with test and Javadoc update (4.76 KB, patch)
2010-07-19 13:36 EDT, 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 Dani Megert CLA 2010-07-14 10:29:17 EDT
R3.6 - works in R3.5.x.

1. start with new workspace
2. enable Javadoc processing
3. paste this:
--------- %< ---------
package p;

/**
 * {@link I}.
 * @see C
 */
public class Main {
	public interface I {
	}
	public class C {}
}
--------- %< ---------
4. Organize Imports (Ctrl+Shift+O)
==> the following unnecessary imports are added and flagged ad unused:
import p.Main.C;
import p.Main.I;

For me this is major as we report unused imports as errors.
Comment 1 TULC CLA 2010-07-14 20:33:36 EDT
Unqualified references in javadoc to any static inner type (interfaces or static classes) will result in an import being created for that reference.

For example, the following code will create imports for <package>.Type.InnerStaticClass and <package>.Type.InnerInterface, but not <package>.Type.InnerClass:

/**
 * {@link InnerClass} {@link InnerStaticClass} {@link InnerInterface}
 */
public class Type {
    public class InnerClass {}
    public static class InnerStaticClass {}
    public interface InnerInterface {}
}

I'm not familiar enough with the spec to know if this is a bug or actually a bug fix since Galileo. In the meantime, as a workaround you can stop it creating the unused import by simply qualifying the references.

For example, to fix the above code use:

/**
 * {@link InnerClass} {@link Type.InnerStaticClass} {@link Type.InnerInterface}
 */
public class Type {
    public class InnerClass {}
    public static class InnerStaticClass {}
    public interface InnerInterface {}
}
Comment 2 TULC CLA 2010-07-14 20:41:22 EDT
Just re-read the original bug report and realised you were getting an import for the non-static inner class as well. I can't reproduce that.
Comment 3 Olivier Thomann CLA 2010-07-15 11:51:34 EDT
The problem comes from the lack of context.
If I change ImportRewrite to consider as implicit import the import to member types of the current compilation unit (this is what the context gives), then I fail again the case:

package p;

import p.Bug.Proto;

import java.io.File;

class Bug {
public void addFile(File file) {}
	interface Proto{};
}
class Foo implements Proto{}

The context should be relative to a type and not a compilation unit, because the import is required to resolve Proto for the implements clause of the Foo type, but it would not be required within the Bug type.

So right now I don't have enough information inside ImportRewrite to make a good choice.

Imports are a notion for a compilation unit, but to known if they are implicit or, it requires more information.
Comment 4 Olivier Thomann CLA 2010-07-15 12:04:59 EDT
Ok, the fix has to be done inside the org.eclipse.jdt.internal.corext.codemanipulation.OrganizeImportsOperation itself.
I believe I have a patch for it.
Once the import is added to the import rewrite, it is too late to decide if this is an implicit import.
Comment 5 Olivier Thomann CLA 2010-07-15 12:05:17 EDT
Patch is under test.
Comment 6 Olivier Thomann CLA 2010-07-15 12:09:17 EDT
Markus, the fix would actually be inside the Bindings class.
As this is widely used, I would like you to have a look at this patch.
If required, we can make the fix very local to the OrganizeImportsOperation, but I believe this is actually a missing check in order to retrieve the proper binding when calling getBindingOfParentTypeContext(..).
Comment 7 Olivier Thomann CLA 2010-07-15 12:09:37 EDT
Created attachment 174417 [details]
Proposed fix
Comment 8 Olivier Thomann CLA 2010-07-15 12:10:53 EDT
This is fixing the issue without breaking the case mentioned in comment 3.
Comment 9 Olivier Thomann CLA 2010-07-19 09:48:09 EDT
Moving to JDT/UI as this is where the patch belongs.
Comment 10 Markus Keller CLA 2010-07-19 13:36:28 EDT
Created attachment 174653 [details]
Fix with test and Javadoc update

Thanks for the investigation and the patch. My patch just adds a Javadoc update and a regression test.
Comment 11 Markus Keller CLA 2010-07-19 13:38:15 EDT
Fixed in HEAD.
Comment 12 Dani Megert CLA 2010-07-20 04:05:46 EDT
Looks good for 3.6.1.
Javadoc: null ==> <code>null</code>
Comment 13 palgemeen CLA 2010-08-09 10:59:58 EDT
I am not sure whether this is related or not, but it seems so related that I did not want to create a separate issue for this. This is my first eclipse issue I report, so please correct me when I miss to follow some guideline or procedure.

Having said all that, when I did a code clean up on the below code I got some explicit imports that are actually implicit imports:

/**
 * @see {@link Inner}
 */
interface Temp {
    interface Inner {
        // nothing
    }
}

Later I found that the same happens when triggering Organize Imports. This is quite annoying because for me this unneeded import generate warnings (which is good), and I use this clean up feature quite frequent, so I find myself having to manually clean up after doing a automatic clean up...

Can any of you guys help me with this in the context of this issue?
Comment 14 Dani Megert CLA 2010-08-10 04:48:34 EDT
(In reply to comment #13)
Looks like the same bug. Please try 3.7 M1 to verify.
Comment 15 palgemeen CLA 2010-08-10 05:14:58 EDT
Hi Dani, thanks for your response. I am so sorry to bother you with this, but I am not sure whether this is correct:

I could not find anything called M1. At least not on the 'Downloads' page, nor on the 'Nightly build' page.
After some googling I found this page: http://wiki.eclipse.org/Eclipse_Project_Update_Sites
that contains the update site for: Integration builds toward 3.7

When I do an "install new software" from eclipse with this update site, then I see the following available:
* Eclipse Platform
  - Eclipse Platform 3.7.0.I20100805-1700
  - Eclipse Platform 3.7.0.I20100805-1200
  - Eclipse Platform 3.7.0.I20100804-1800
* Eclipse Platform SDK
  - Eclipse Platform SDK 3.7.0.I20100805-1700
  - Eclipse Platform SDK 3.7.0.I20100805-1200
  - Eclipse Platform SDK 3.7.0.I20100804-1800
* Eclipse SDK
  - Eclipse SDK 3.7.0.I20100805-1700
  - Eclipse SDK 3.7.0.I20100805-1200
  - Eclipse SDK 3.7.0.I20100804-1800
* Releng Tools
  - Eclipse Releng Tools 3.3.0.v20100427-45-7w3121173A


Which of these should I select?

Thank you for your time!
Regards,
Paul
Comment 16 Dani Megert CLA 2010-08-10 06:16:56 EDT
>Which of these should I select?
* Eclipse SDK
  - Eclipse SDK 3.7.0.I20100805-1700

Or download a fresh clean build:
http://download.eclipse.org/eclipse/downloads/drops/S-3.7M1-201008051700/index.php
Comment 17 palgemeen CLA 2010-08-11 04:58:05 EDT
Hi Dani,

thanks again for your advice. The 3.7M1 does solve my issue, with both the simple code that I posted, as well as the original code with which I ran into the issue.

Is there any chance that this fix will become available as a Helios update (I mean that I can update my current Helios with Help->Find Updates)? I tried M1 for a while and it does not seem very suit for the production work I do (problems with auto-completion and some of my plugins experience problems.

So, is there any way to get this fix in my Helios installation in a stable way?

Regards,
Paul
Comment 18 Dani Megert CLA 2010-08-11 05:01:40 EDT
It will be in the next service release (September) from which you will be able to update but there are already stable builds towards that release:
http://download.eclipse.org/eclipse/downloads/drops/M20100806-0800/index.php. I don't know whether there's a p2 repository that allows you to update from maintenance builds.
Comment 19 palgemeen CLA 2010-08-11 12:07:00 EDT
thanks again!
Comment 20 Markus Keller CLA 2010-08-17 10:14:53 EDT
Released the code patch to R3_6_maintenance.
Comment 21 Raksha Vasisht CLA 2010-08-26 03:08:13 EDT
Starting to verify...
Comment 22 Raksha Vasisht CLA 2010-08-26 03:16:12 EDT
Verified for 3.6.1 RC2 with M20100825-0800.
Comment 23 Olivier Thomann CLA 2010-11-10 11:54:18 EST
*** Bug 329921 has been marked as a duplicate of this bug. ***