Community
Participate
Working Groups
Build ID: 3.3M4 Steps To Reproduce: 1. pass an argument of type diffrent than IResource to #isConsistentItem(Object) method, it will throw ClassCastException 2. same thing with #matchItem(Object) More information: Misleading Javadoc contract for #isConsistentItem(Object) and #matchItem(Object) in org.eclipse.ui.dialogs.FilteredResourcesSelectionDialog.ResourceFilter. Although any type of argument can be passed, both methods should check if passed argument is IResource. This fact should be also indicated in Javadoc.
Created attachment 57332 [details] This is patch file with more accurate javadoc and type checking condition.
IMO, a better way to deal with these "Object item" variables would be to add Javadoc contracts to FilteredItemsSelectionDialog to make clear that these items are guaranteed to be only items that the client passed to the contentProvider of fillContentProvider(..). Likewise, the ItemsFilters passed around should be guaranteed to be those returned from createFilter(). This would avoid all the superfluous checks in the performance-critical parts of the FilteredItemsSelectionDialog. This approach should also make it possible to generify the type later on (and even if Java 5.0 cannot be used, documentary comments such as /*<T>*/ for variables of type Object will make the code much easier to understand).
We could only check instanceof input object and add comments as you propose inside implementation(FilteredResourcesSelectionDialog). We couldn't change signature of this method because we don't have generalization of searching objects. In FilteredResourcesSelectionDialog we are searching IResources and in other implementation FilteredTypesSelectionDialog we are searching TypeNameMatch. Now we don't have a interface which generalize searching objects. Moreover we used this methods in abstract FilteredItemsSelectionDialog.
My point was that you could turn FilteredItemsSelectionDialog into something like FilteredItemsSelectionDialog<I, F extends ItemsFilter<? extends I> if you could use generics. Since you can't switch to Java 5, the second-best option would be to keep the implementation as it is, but add Javadoc contracts that have the same effect. Thus, subclasses of FilteredItemsSelectionDialog could avoid the instanceof checks everywhere and just do casts. Note that your implementation already relies on that at many places, e.g. in FilteredResourcesSelectionDialog.getItemsComparator(): public int compare(Object o1, Object o2) { Collator collator = Collator.getInstance(); IResource resource1 = (IResource) o1; IResource resource2 = (IResource) o2; [..] }
Please see: attachment 58121 [details] to bug 171797 . The patch is targeted to both bugs.
Tomasz this patch is out of date with HEAD. Could you update please? You likely want to synch up with Kryzstof as he is also preparing a patch right now.
Comment on attachment 57332 [details] This is patch file with more accurate javadoc and type checking condition. Patch for bug 171797 fixes also this bug. Krzysztof, could you verify it?
Bug was fixed by Tomasz patch to bug 171797.
