Bug 128661 - [content assist][organize imports] Completion on type in javadoc formal references should not add import
Summary: [content assist][organize imports] Completion on type in javadoc formal refer...
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P3 enhancement with 19 votes (vote)
Target Milestone: ---   Edit
Assignee: JDT-Text-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
: 10936 194652 279096 292677 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-02-20 10:12 EST by Frederic Fusier CLA
Modified: 2018-04-05 05:12 EDT (History)
16 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2006-02-20 10:12:55 EST
Considering same test case than bug 118038:
MyTestClass.java
  package test;
  public class MyTestClass {}
X.java
  package pack;
  /**
   * Complete after: MyTest|
   */
  public class X {
  }

Complete after MyTest and select first proposal: {@link MyTestClass}
If 'Add import instead of qualified name' option for Content Assist is checked, then an import is automatically added. Problem is that there's a potential unused import which is added if user decide not to Process javadoc comments...

A more annoying use case is shown in o.e.jdt.core.compiler.CategorizedProblem.
In getExtraMarkerAttributeNames method javadoc comment, there's a reference to IMarker interface constants. If we use content assist to insert @link tags on these constants, then an import is added which breaks the independency of source folder compiler from other eclipse plugins and makes the compilation failing when building the stand-alone compiler...

So, we think that, by default, complete a type in Javadoc should never add any import whatever the option 'Add import instead of qualified name' value is. As this change would not need API changes, it could be done for 3.2.

If one think that import needs to be inserted same way as it is done for Java code, then perhaps an additional sub-option may be added in preferences page under already existing one. Perhaps something like:
'Add import instead of qualified name'
  + 'Apply this rule also for completion in Javadoc'

This API change of course should wait for next release and would be followed-up by another bug...
Comment 1 Tom Hofmann CLA 2006-02-21 05:23:21 EST
Qualification behavior inside formal references: 

The current reasoning was that a javadoc formal reference should be treated like code.

I don't think that the scenario you describe with CategorizedProblem is something we want to support, as it is really a problem with your classpath / project setup: you have a source folder that must be kept clean of certain references, despite them being visible on its class path. The correct solution for this is to create an independent project with proper depenendencies - and then, there won't be any proposals for IMarker showing up. If this is not possible... bad luck. You have the same problem when writing normal code: you always have to be careful not to import any illegal type.

We already handle the problem with javadoc processing: if javadoc processing is disabled, we always insert the FQN inside formal references. Of course this causes problems:
- when you disable javadoc processing later (same for many other options)
- when your build system uses different compiler options than configured
  (same for many other compiler options)

In short, I believe we are doing the right thing here, and the default should be as it currently is. FQNs clutter up the editor, be it in code or javadoc.

Adding an additional option would be possible. Re API: IMO even the additional preference would be possible for 3.2, as we've always added preference keys after the API freeze. Dani?
Comment 2 Dani Megert CLA 2006-02-21 05:34:23 EST
No plans to change this.
Comment 3 Philipe Mulet CLA 2006-02-21 13:10:26 EST
Then it should be WONTFIX rather than WORKSFORME.

I would still consider this a unoptimal though, as javadoc is not part of normal compilation; and adding imports for the sole purpose of resolving javadoc can be viewed as polluting the code.
Comment 4 Dani Megert CLA 2006-02-22 02:49:24 EST
>Then it should be WONTFIX rather than WORKSFORME.
Wrong. WONTFIX means we agree it's a bug but don't fix it. As we see it is the correct behavior: the initial arguments were that it gives  warnings due to unchecked "Process javadoc comments...", which is not true and the other argument i.e. giving an error is clearly a build path problem which also burns you when using code assist to add a type in the code.

>and adding imports for the sole purpose of resolving
>javadoc can be viewed as polluting the code.
OK, that's a different view and argument. We discussed about the preferences but since one of our goal is to reduce preferences I voted against it. Feel free to reopen if you really think this additional preference is added value and a must have for Eclipse.
Comment 5 Joerg Hohwiller CLA 2007-08-01 14:13:10 EDT
Hello there. I am using eclipse for several years now. I have the same problem as   Frédéric with the javadoc completion. I now testet 3.3 and since it still does NOT allow to change this behavior I was so annoyed that I decided to enter a bug report. First I scanned the issues and so I finnaly got here.
I am even more surprised to see that this feature was already requested for 3.2. and was set to RESOLVED with WORKSFORME.

To describe the problem from my point of view:
The current behavior is bad because it adds import statements for reasons of documentation that have no functional need. But an import statement can badly influence the functionality of your code. I can tell you that we had ClassNotFoundExceptions when testing our applications because of this problem. If you add an import statement to another class and the imported class later gets not included into your classpath for some reason, the classloader will refuse to load the documented class with the import statement generated by eclipse.

Please consider about your meaning of this issue!
Comment 6 Joerg Hohwiller CLA 2007-08-01 14:22:52 EDT
Please also consider that checkstyle complains about imports that are only used in javadoc references if the rule "UnusedImports" is active.

For years I am completing references in javadoc, cuting the qualified name from the generated import statement, removing the import statement and then pasting the qualified name into the javadoc. This is really NOT nice and I know many others that use eclipse this way and ask me how to change this.
Comment 7 Dani Megert CLA 2007-08-02 04:31:08 EDT
Sorry, I can't follow your "class not found scenario". Exactly the same can happen with classes inside your code if
" the imported class later gets not included into your classpath for some reason" 

If Javadoc is not important to you then simply uncheck processing of Javadoc and code assist won't add an import.

Having said that, if we get a good quality patch that adds a preference to control this behavior we will add it.
Comment 8 Frederic Fusier CLA 2007-08-02 05:49:55 EDT
(In reply to comment #7)
> 
> If Javadoc is not important to you then simply uncheck processing of Javadoc
> and code assist won't add an import.
> 
Uncheck processing of Javadoc will result to have no completion at all...
So, I do not think this is an option in this case.
Comment 9 Joerg Hohwiller CLA 2007-08-02 06:06:27 EDT
> Sorry, I can't follow your "class not found scenario". Exactly the same can
> happen with classes inside your code if
> " the imported class later gets not included into your classpath for some
> reason" 
Yes, the same can happen for functional imports. But all I am saying is that the import statement has an important functional impact on the class. And javadoc should NOT influence the functional behaviour of that class. 

Please consider dummy classes as example. When you are using maven, you have a separation of main code and test code. However eclipse can not make this seperation in a single project. Therefore I can make a reference from a main class to a test class in the javadoc. This is kind of odd but might make sense in specific situations. Anyhow the test code does NOT get included into the jar file that is build by maven. Therefore no import statement should be generated on the test class.

> If Javadoc is not important to you then simply uncheck processing of 
> Javadoc and code assist won't add an import.
Javadoc is very important to me. That is why I care about this issue!
I need the assist because else I could edit my sources with vi.

> Having said that, if we get a good quality patch that adds a preference to
> control this behavior we will add it.
Okay. At least this is an option. I am involved in many other Open-Source Projects and have little time. But I will have a look. Could you give a little hint to get startet (e.g. SVN link on the related class). Thanks.
Comment 10 Frederic Fusier CLA 2007-08-02 06:13:36 EDT
(In reply to comment #8)
> (In reply to comment #7)
> > 
> > If Javadoc is not important to you then simply uncheck processing of Javadoc
> > and code assist won't add an import.
> > 
> Uncheck processing of Javadoc will result to have no completion at all...
> So, I do not think this is an option in this case.
> 
Ooops, forget my previous comment. Completion still works even if javadoc comments are not process by compiler as the completion parser overrides this setting...
I need to refresh my memories on this part of code... :-(
Sorry for the confusion.
Comment 11 Dani Megert CLA 2007-08-02 06:22:45 EDT
The code is in CVS:
  repo path: /cvsroot/eclipse
  module:    org.eclipse.jdt.ui

The new preference would probably be added to CodeAssistConfigurationBlock. See how other prefs are done in there. Various ICompletionProposal(s) in JDT UI need to be adjusted: keep in mind that not only type but also method and field proposals are affected.
Comment 12 Joerg Hohwiller CLA 2007-08-07 13:44:53 EDT
Thanks for the hint.

FYI: This seems to be somewhat related to bug #90572
Comment 13 Joerg Hohwiller CLA 2007-08-07 17:09:19 EDT
okay, I did
cvs -d :pserver:anonymous@dev.eclipse.org:/cvsroot/eclipse co org.eclipse.jdt.ui

further I added this as eclipse Plugin-Project. However there are missing dependencies and I especially have no idea how to really test or debug whatever I modify. I was using cetha and had to start a runtime-workbench what was some sort of eclipse started from my initial eclipse. Anyhow I am kind of lost. Do you have a link to some developer manual?
Comment 14 Dani Megert CLA 2007-08-08 02:53:51 EDT
Start a fresh workspace (don't import your known pref). It seems that somehow you're not using PDE containers. Also, if you take HEAD you have to use one of the latest 3.4 I-builds.
Comment 15 Dani Megert CLA 2007-09-03 03:17:31 EDT
Reopning as several people seem to want this option.
Comment 16 Dani Megert CLA 2007-09-03 03:17:51 EDT
*** Bug 194652 has been marked as a duplicate of this bug. ***
Comment 17 Dani Megert CLA 2007-09-17 12:40:08 EDT
*** Bug 10936 has been marked as a duplicate of this bug. ***
Comment 18 Dani Megert CLA 2009-06-04 09:34:31 EDT
*** Bug 279096 has been marked as a duplicate of this bug. ***
Comment 19 Joerg Hohwiller CLA 2009-06-25 16:37:14 EDT
There are also situations in which this feature is causing an illegal import statement.
Add a class "X" with a protected inner class "Foo". Then add a protected method "bar" that uses this class as argument.
Now add a class with the same name "X" in some other package and add a javadoc comment "@see bar(Foo)". This will cause organize imports to add an import to the inner class "X.Foo" but this class is not visible. Compilation will fail.
Since "organize imports" is often used in save-actions this can cause real headaches. 
Please have mercy, see all the duplicated bug reports, votes, etc.
Thanks :)
Comment 20 Joerg Hohwiller CLA 2009-06-26 03:00:53 EDT
I meant 
"@see #bar(Foo)"
Comment 21 Dani Megert CLA 2009-06-26 03:33:41 EDT
re comment 19: this looks like a separate bug. Please file it so we can take a look.
Comment 22 Dani Megert CLA 2009-10-19 11:56:37 EDT
*** Bug 292677 has been marked as a duplicate of this bug. ***
Comment 23 Stefan Cordes CLA 2013-09-03 03:16:20 EDT
Maybe extending type assist to insert qualified names on @link and @see always?

Currently:
typing "create a File" <Ctrl-Space><Enter>
=> "create a {@link File}" and import java.io.File;

Proposed behaviour:
typing "create a File" <Ctrl-Space><Enter> 
=> "create a {@link java.io.File}" and no import.

So both are happy: 
those who want to get rid of the import and
those who want to have javadoc working.
Comment 24 Dani Megert CLA 2013-09-03 03:22:50 EDT
(In reply to Stefan Cordes from comment #23)
> Maybe extending type assist to insert qualified names on @link and @see
> always?
> 
> Currently:
> typing "create a File" <Ctrl-Space><Enter>
> => "create a {@link File}" and import java.io.File;
> 
> Proposed behaviour:
> typing "create a File" <Ctrl-Space><Enter> 
> => "create a {@link java.io.File}" and no import.
> 
> So both are happy: 
> those who want to get rid of the import and
> those who want to have javadoc working.

We wouldn't change this now, but adding an option for this would be OK.