Bug 498048 - Imports not resolved correctly with generics and inner classes (v2)
Summary: Imports not resolved correctly with generics and inner classes (v2)
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.6   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL: https://bugs.openjdk.java.net/browse/...
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-18 06:38 EDT by Juraj Wagner CLA
Modified: 2022-07-09 01:56 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Juraj Wagner CLA 2016-07-18 06:38:14 EDT
Observed on:
Version: Neon Release (4.6.0)
Build id: 20160613-1800
Oracle JDK 1.8.0_92

Observed behavior:
After refactoring of classes we run into an issue that is very similar to that reported in #294057 with one alternation.

Take following example:

IModel.java

package model;
import model.IModel.IModelListener;
public interface IModel<L extends IModelListener> 
{
  public interface IModelListener {
  }
}

IListModel.java

package model;
import model.IListModel.IListModelListener;
public interface IListModel<T> extends IModel<IListModelListener>
{
  public interface IListModelListener extends IModelListener {
  }
}

The class IListModel is compiled by Eclipse but not by javac8:
  model/IListModel.java:[5,53] cannot find symbol
    symbol:   class IModelListener
    location: interface model.IListModel<T>

Adding the missing model.IModel.IModelListener import fixes the javac compilation issue with Eclipse viewing the import as unnecessary.

One additional quirk we have been observing for some time is that while in case of IListModel being not parametrized by <T> Eclipse is able to resolve the imports it does so every 2nd time, i.e. if odd invocations of Organize Imports add model.IModel.IModelListener import, even invocations remove it. Should this be report as a separate bug?
Comment 1 Stephan Herrmann CLA 2016-07-19 05:40:17 EDT
Do you have any hints on why we should reject that program?

Note that the following *is* accepted by javac, too:

//---
package model;
import model.IListModel.IListModelListener;
public interface IListModel<T> extends IModel<IListModelListener>
{
  public interface IListModelListener extends IModel.IModelListener {
  }
  IModelListener getListener();
}
//---

Two things shown:
(1) an import is not really needed, qualifying the inner with its outer (from same package) makes javac happy.
(2) in other positions like method return type also javac believes that IModelListener is visible in this scope.

With this in mind javac's error message in comment 0 seems to be the odd one out, if you ask me.


Yet another hint: alternatively, removing the type argument from "extends IModel" makes javac happy, too. For references to IModelListener the type argument of IModel should have no influence, since IModelListener as in interface is implicitly static and thus has no connection to IModel's type parameter.
Comment 2 Juraj Wagner CLA 2016-07-21 10:03:30 EDT
(In reply to Stephan Herrmann from comment #1)
> Do you have any hints on why we should reject that program?

The code sample in this report is in essence identical to that from bug #294057. Removing the type parameter <T> from the declaration of IListModel will result in Eclipse treating the missing import as an error. Why does the presence of the type parameter <T> make any difference?

Not knowing any better, I assume that the presence of the type parameter <T> must not make any difference. Thus unless #294057 is undone, Eclipse should reject the code.

> Note that the following *is* accepted by javac, too:
> 
> //---
> package model;
> import model.IListModel.IListModelListener;
> public interface IListModel<T> extends IModel<IListModelListener>
> {
>   public interface IListModelListener extends IModel.IModelListener {
>   }
>   IModelListener getListener();
> }
> //---
> 
> Two things shown:
> (1) an import is not really needed, qualifying the inner with its outer
> (from same package) makes javac happy.
> (2) in other positions like method return type also javac believes that
> IModelListener is visible in this scope.
> 
> With this in mind javac's error message in comment 0 seems to be the odd one
> out, if you ask me.
> 
> 
> Yet another hint: alternatively, removing the type argument from "extends
> IModel" makes javac happy, too. For references to IModelListener the type
> argument of IModel should have no influence, since IModelListener as in
> interface is implicitly static and thus has no connection to IModel's type
> parameter.

I took the time and read parts of the JLS. After reading ยง6.3 I do not understand why IModel.IModelListener in the interface body makes a difference, after all IModelListener is in scope of declaration of IListModel. Not seeing any other argument I agree with your assessment.

One more thing I would be happy to have explained is why in this case:

import model.IModel.IModelListener;
public interface IListModel extends IModel<IListModel.IListModelListener>
{
	public interface IListModelListener extends IModelListener {
	}
}

Eclipse requires the import of IModelListener. IModelListener is in scope of declaration of IListModel so a single-type-import declaration seems to be unnecessary.
Comment 3 Stephan Herrmann CLA 2016-07-25 16:13:38 EDT
(In reply to Juraj Wagner from comment #2)
> One more thing I would be happy to have explained is why in this case:
> 
> import model.IModel.IModelListener;
> public interface IListModel extends IModel<IListModel.IListModelListener>
> {
> 	public interface IListModelListener extends IModelListener {
> 	}
> }
> 
> Eclipse requires the import of IModelListener. IModelListener is in scope of
> declaration of IListModel so a single-type-import declaration seems to be
> unnecessary.

Debugging that case it seems the reason for rejecting that variant is purely technical.

Details:
--------
ClassScope.findSuperType() is invoked for "IListModel extends IModel<IListModelListener>"

To resolve "IModel<IListModelListener>" we first need to resolve the type argument, i.e., invoke TypeReference.resolveTypeArgument() for "IListModelListener".

After finding the binding for "IListModelListener" we need to test for a hierarchy cycle, i.e., invoke ClassScope.detectHierarchyCycle() for "IListModelListener"

In this course we need to resolve the super types of "IListModelListener", i.e., "IModelListener".

During attempt to resolve "IModelListener" we navigate to the enclosing type "IListModel" and search its member types.

At this point, however, "IListModel" doesn't yet have any super types connected (this has just started in the first frame mentioned above). => Inherited member "IModelListener" is not found.

=> Boom. 


Two things to check:
- Does JLS define a relationship between scoping and cycle detection, as our implementation has?

- If accepting is OK by JLS, then we may want to pull out the actual cycleChecking from resolving and defer it to the end of ClassScope.connectTypeHierarchy() or even later. See ClassScope#deferredBoundChecks for a related precedent.
Comment 4 Juraj Wagner CLA 2016-08-11 13:45:12 EDT
(In reply to Stephan Herrmann from comment #3)

> 
> Two things to check:
> - Does JLS define a relationship between scoping and cycle detection, as our
> implementation has?

Sections 8.1.4., 9.1.3. define when a class depends on itself and that this is a compile-time error. I did not find a section that would define when a compiler is expected to detect such compile-time errors, i.e. what cycle detection must occur.

It would seem that this is left up to the implementer.

> 
> - If accepting is OK by JLS, then we may want to pull out the actual
> cycleChecking from resolving and defer it to the end of
> ClassScope.connectTypeHierarchy() or even later. See
> ClassScope#deferredBoundChecks for a related precedent.

If what I wrote above is correct then the import is not required by the specification.

Worth noting is that javac in JDK 9 build 130 does no longer require import of IModelListener. This might be due to JEP 216: Process Import Statements Correctly.

One more comment (not sure whether this is known). While looking at JEP 216 I had a peek into JDK-8041994 and to my surprise Eclipse compiles to example, which seems to me to be in violation with JLS 8.1.4.
Comment 5 Juraj Wagner CLA 2016-08-11 14:06:15 EDT
(In reply to Stephan Herrmann from comment #3)

Looking just on Eclipse's behavior I find it in following case inconsistent:

package model;
import model.IListModel.IListModelListener;
public interface IListModel<T> extends IModel<IListModelListener>
{
  public interface IListModelListener extends IModelListener {
  }
}

Why is the import of IModelListener not needed anymore? What has the parameter type T have to do with the resolution of IListModelListener?

When I compare this to:

import model.IModel.IModelListener;
public interface IListModel<T> extends IModel<IListModel.IListModelListener>
{
	public interface IListModelListener extends IModelListener {
	}
}

where the import is always required, my head explodes :)
Comment 6 Manoj N Palat CLA 2018-05-17 03:25:53 EDT
bulk move out of 4.8
Comment 7 Eclipse Genie CLA 2022-07-09 01:56:38 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.