RFR: JDK-8311264: JavaDoc index comparator is not transitive

[Pavel Rappo]

On Wed, 5 Jul 2023 15:56:51 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

> Please review a fix to make the comparator used by the JavaDoc index pages to be transitive (see JBS issue for a description and example of the problem).
> 
> I fix the bug by creating method `getIndexElementKey(Element)` to extract the key for comparing elements in `Comparator.makeIndexElementComparator`, and allowing `IndexBuilder` to reuse that extractor method when comparing element index items to search tag index items. The rationale for this approach was to preserve the current order for sorting elements in the index, and to keep the change simple (considering possible backport to 21).
> 
> In the process I also simplified some parts of the code a bit (simpler logic to compare names in element comparator, no need for composite comparator for classes-only index). 
> 
> For the test, I added various elements and search tags that trigger the condition to the existing `TestIndex` page and replaced `checkOutput` with `checkOrder` to test the order of index page items.

I can see that I'm late to the party. FWIW, the change looks good to me.

There's probably one additional problem that predates this PR. I see that Comparators.makeIndexElementComparator breaks ties by comparing fully qualified names. Although unlikely to happen in practise, in theory it might break when one has a static nested class and a class in the default package (hence not applicable to JDK) that share FQN. Here's an example I used for one of our tests earlier (you might need to scroll it if reading inline on GitHub):

https://github.com/openjdk/jdk/blob/0616648c59215d001211423402c6444ce228f01e/test/langtools/jdk/javadoc/doclet/testThrowsInheritanceMatching/TestExceptionTypeMatching.java#L350-L396

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14776#issuecomment-1623486580