Community
Participate
Working Groups
I20090428-0100. The Call Hierarchy should remember for which nodes to expand constructors. Another option would be to allow to configure this.
The difficult question is what element exactly to remember. 'Expand With Constructors' is mainly useful on methods that serve as function pointers, e.g. run() in an anonymous subtype of Runnable. I think it would be annoying if I had to toggle this for every single implementation of the run() method. After a while, it would also become hard to predict whether a given run() method will be expanded with constructors or not. It would be much more convenient if there was a list of methods whose *implementations* (or extensions) should be expanded with constructors. The prime defaults for this list are Runnable#run() and Callable#call(). However, this is not easy to implement efficiently, because we can neither create a supertype hierarchy for every method that shows up in a call hierarchy, nor can we create (and maintain) a subtype hierarchy of all types in the list. I guess the hierarchy would have to be created on demand (using the supertype hierarchy cache from the override indicators), whenever the user expands a node or opens the context menu.
Maybe we could trick a but and use the method name plus some code parsing i.e. check whether it's inside an anonymous class.
> Maybe we could trick a but and use the method name I was also thinking about that, and I'm starting to like this as a quick fix for 3.5. > plus some code parsing i.e. check whether it's inside an anonymous class. I fear that would already introduce too much "black magic", since it would fail as soon as the anonymous class is converted into its own class (which is reasonable for delayed tasks or listener implementations). But we could generalize it and just check the simple names of the direct super types. That info is relatively cheaply available from the IMethod: - if the declaring type is anonymous, just check the type name - otherwise, check the declaring type's #getSuperInterfaceNames() and #getSuperclassName() This approach has the advantage that the "special" methods can already now be stored fully qualified, and thus the stored data could still be used if we decide later to implement the full-fledged solution outlined in comment 2. It can lead to false positives, but that's acceptable IMO.
re comment 3: good idea.
Created attachment 134405 [details] Patch with the fix.
*** Bug 275128 has been marked as a duplicate of this bug. ***
Comment on attachment 134405 [details] Patch with the fix. * CallHierarchyContentProvider#isInTheDefaultExpandWithConstructorList(IType): - Always watch out for NPEs. 'superClass= type.getSuperclassName()' -> superClass can be null - Extract local variable for repeated expressions. defaultTypes[i] - IType#getFullyQualifiedName() is most of the time not what we need. Should be getFullyQualifiedName('.'). - 'String[] superInterface' should be 'superInterfaces' (plural) * CallerMethodWrapper#fIsDefaultExpandWithConstructors: I don't see how this property should work. I would expect that the state of the "Expand with Constructors" menu item always correctly reflects the state of the selected node, but currently, it is also unchecked for methods that are automatically expanded with constructors. I think the easiest solution for this would be to change CallerMethodWrapper#fExpandWithConstructors to be type Boolean and then interpret 'null' from getExpandWithConstructors() as "has not been explicitly set, so take the default". And if it is null, then ExpandWithConstructorsAction#canActionBeAdded() can set the default state.
Created attachment 135332 [details] Improved patch
> interpret 'null' from getExpandWithConstructors() as "has not been explicitly > set, so take the default". And if it is null, then > ExpandWithConstructorsAction#canActionBeAdded() can set the default state. The last patch is still missing this, so e.g. the context menu on a run() method still does not have "Expand with Constructors" checked. You really need to do the check before adding the action and set the flag there as well (and you need to keep it in the content provider as well, in case the user expands the node before opening the menu).
Created attachment 135525 [details] Added check in EWC.
Dani, can you please review patch 3? Cosmetics I'd apply to CallHierarchyContentProvider#setDefaultExpandWithConstructors(..): - move 'IType type= ..' to inside the if block - change Javadoc to * Sets the default "expand with constructors" mode for the method wrapper. * Does nothing if the mode has already been set. - rename method to "ensureExpandWithConstructorsSet"
Works as expected. Besides the changes suggested by Markus I suggest to encapsulate the knowledge whether the expand mode has been set or not inside the wrapper so that we don't need to change the data type of fExpandWithConstructors and set/getExpandWithConstructors(). Committed to HEAD with above changes plus commenting the empty catch block.
Created attachment 135802 [details] Fixes NPE The last patch caused NPEs when a member was not a method (but e.g. a type that doesn't have a constructor). The fix ensures that CallHierarchyContentProvider#ensureDefaultExpandWithConstructors(CallerMethodWrapper) only sets the expandWithConstructors flag if the given member can be expanded with constructors at all. Released to HEAD.
Created attachment 135852 [details] Configure automatic Expand with Constructors Adds a view menu action that opens a dialog to configure for which types 'Expand with Constructors' should be the default.
Released for I20090514-2000. Dani, could you please sanity check HEAD?
Fix is good. Found one minor issue (see bug 276429).
Filed bug <a href="show_bug.cgi?id=276466" title="[call hierarchy] Improve UI for View Menu Preferences">bug 276466</a>) for UI (Call Hierarchy View Menu -> Preferences) improvement.
Verified in I20090514-2000.
