Bug 325336

Comment 1 Markus Keller 2010-09-15 07:49:00 EDT

Was OK in I20100912-2000. Since Ctrl+T is also affected, I assume it's a problem in JDT/Core (not confirmed yet).

Comment 2 Markus Keller 2010-09-15 08:08:44 EDT

Problem seems to be in UI:
Works fine with I20100912-2000 and jdt.core from HEAD.
Broken with I20100914-0100 and jdt.core v_B11.

Comment 3 Markus Keller 2010-09-15 08:35:57 EDT

Problem is in TypeHierarchyContentProvider. It works again when I revert to last version (and then fix the compile error).

I'll fix this.

Comment 4 Markus Keller 2010-09-15 08:40:27 EDT

Created attachment 178922 [details]
Fix1

Comment 5 Markus Keller 2010-09-15 08:50:24 EDT

This got broken with the patch on bug 21417 comment 18 (worked fine with previous patches).

Raksha, can you please review the fix?

Comment 6 Raksha Vasisht 2010-09-15 09:18:29 EDT

(In reply to comment #5)
> This got broken with the patch on bug 21417 comment 18 (worked fine with
> previous patches).
> 
> Raksha, can you please review the fix?

The fix brings back the issue mentioned in bug 21417 comment 16  :(

* Have 2 simple classes:
package m;
public class A { }
package m;
public class B extends Exception {}

* I don't get the logic when I look at the hierarchy of A and B (compare this
to the hierarchy of package m):
- Why is 'Serializable' shown?

Comment 7 Markus Keller 2010-09-15 09:49:17 EDT

I'm working on an additional fix in TraditionalHierarchyContentProvider#getRootTypes(List). The old assumption was that a hierarchy on a region was in fact a hierarchy on a single package. This is no longer true, so we have to find the right roots ourselves.

My first patch restores operation for the normal case (hierarchy on 1 type or 1 package). This will go into M2, but I hope I can fix the issue for multi-element hierarchies as well.

Comment 8 Raksha Vasisht 2010-09-15 11:12:52 EDT

Created attachment 178945 [details]
Patch for isInHierarchyOfInputElements(..)

(In reply to comment #7)
> I'm working on an additional fix in
> TraditionalHierarchyContentProvider#getRootTypes(List). The old assumption was
> that a hierarchy on a region was in fact a hierarchy on a single package. This
> is no longer true, so we have to find the right roots ourselves.
> 
> My first patch restores operation for the normal case (hierarchy on 1 type or 1
> package). This will go into M2, but I hope I can fix the issue for
> multi-element hierarchies as well.

I have a patch for org.eclipse.jdt.internal.ui.typehierarchy.TypeHierarchyContentProvider.isInHierarchyOfInputElements(IType)which is required to check whether the given type really belongs to the hierarchy of atleast one of the input types. Earlier this method returned true if one of the input elements is a type which is wrong. The check operation might be expensive and works along with the fix in TraditionalHierarchyContentProvider#getRootTypes(List). The TH now shows sub types as well. Markus, could you pls try it out?

Comment 9 Markus Keller 2010-09-15 11:52:11 EDT

Comment on attachment 178945 [details]
Patch for isInHierarchyOfInputElements(..)

This fix is too complicated and too risky for M2. We need a fix with minimal impact on the normal scenario (hierarchy on 1 type or 1 package). It also shows too many types (e.g. shows Serializable in hierarchy on m in comment 6). Could also have problems with logical packages because the name comparison is gone.

Comment 10 Markus Keller 2010-09-15 12:03:23 EDT

Comment on attachment 178922 [details]
Fix1

Released Fix1 for I20100915-1300.

This fixes the normal case but shows too many types with mixed inputs (type + package/etc.).

Comment 11 Markus Keller 2010-09-15 13:44:10 EDT

D'oh! We were trying to fix something we can't reasonably fix.

JDT/Core has support for 2 kinds of type hierarchies:

A.) Type hierarchy on a single type. The result contains all super and subtypes
of the focus type.

B.) Type hierarchy on a region (collection of Java elements). The result
contains all types in the region and necessary supertypes to complete the
hierarchy, but does *not* contain any subtypes outside of the region.

=> We cannot create a "merged" hierarchy from (A)- and (B)-type hierarchies.

I'll attach a patch that does not allow the user to create such hierarchies.

Comment 12 Markus Keller 2010-09-15 15:34:56 EDT

Created attachment 178976 [details]
Fix2 (action enablement)

Raksha, can you please review?

Comment 13 Markus Keller 2010-09-15 15:45:21 EDT

Created attachment 178978 [details]
Fix3 (external archives)

TypeHierarchyLifeCycle contains obsolete code for ITypes in regions, and a bad check that prevents type hierarchies on external archives (e.g. the JUnit library). Patch removes that code.

Comment 14 Markus Keller 2010-09-15 16:41:45 EDT

Created attachment 178980 [details]
Fix4

Combines Fix2 and Fix3, and fixes action enablement (really).

Comment 15 Markus Keller 2010-09-15 16:58:39 EDT

Released Fix4 to HEAD.

Thanks to Raksha for the review and for insisting on the action enablement problem (which was already in the initial multi-hierarchy fix).

Comment 16 Dani Megert 2010-09-16 04:46:07 EDT

Verified in I20100915-2024 that TH and QH work again and that action enablement works.