Community
Participate
Working Groups
org.eclipse.jdt.core.tests.model.CompletionTestsRequestor2.getReversedResults() method has incorrect comparator: === cut === Arrays.sort(this.proposals, new Comparator() { public int compare(Object o1, Object o2) { if (o1 instanceof CompletionProposal && o2 instanceof CompletionProposal) { CompletionProposal p1 = (CompletionProposal) o1; CompletionProposal p2 = (CompletionProposal) o2; int relDif = p2.getRelevance() - p1.getRelevance(); if(relDif != 0) return relDif; String name1 = getElementName(p1); String name2 = getElementName(p2); return name1.compareTo(name2); } return -1; } }); === cut === Provided comparator is incorrect for null elements. It returns -1 if any of compared objects is null. But according to spec "The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y." [1] This error is the cause of Eclipse Unit test failures on Apache Harmony [2] Sorry for probably wrong component selection. [1] http://java.sun.com/j2se/1.5.0/docs/api/java/util/Comparator.html#compare(T,%20T) [2] http://harmony.apache.org
Created attachment 67444 [details] Proposed fix I'll run all the tests with this patch.
Olivier, thanks for the patch. I would change the initial null checks in the following way: === cut === if (o1 == o2) return 0; if (o1 == null) return 1; if (o2 == null) return -1; === cut === It looks more clear and first check could avoid all other comparisons for non-null cases. What do you think?
Yes, even better. I'll release it later today for 6pm build.
Created attachment 67468 [details] Better patch
Kent, please review.
Thank you.
Released for 3.3RC1. No regression test. Alexey, could you please put a note to say this is fixed with next integration build?
Yes, sure. I'll test it tomorrow. It's almost night here :) Thanks, guys, for so quick response.
Alexey, once I have your confirmation that it works now I'll close it as verified.
The patch works. Thanks again.
