GITHUB#9967: document the Fields#iterator() order contract and assert it in tests

lucene/CHANGES.txt

@@ -131,6 +131,10 @@ New Features
 
 Improvements
 ---------------------
+* GITHUB#9967: Document that Fields#iterator() must return field names in ascending order, and assert
+  it in the asserting codec so a violating PostingsFormat fails in tests rather than only in
+  CheckIndex. (Serhiy Bzhezytskyy)
+
 * GITHUB#15704: Replace LinkedList with more efficient data structure. (Renato Haeberli)
 
 * GITHUB#15682: Use ArrayDeque instead of LinkedList in CompoundWordTokenFilterBase.java. (Renato Haeberli)

lucene/core/src/java/org/apache/lucene/codecs/FieldsConsumer.java

@@ -59,6 +59,8 @@ protected FieldsConsumer() {}
    *   <li>The provided Fields instance is limited: you cannot call any methods that return
    *       statistics/counts; you cannot pass a non-null live docs when pulling docs/positions
    *       enums.
+   *   <li>Field names arrive in ascending natural order, as {@link Fields#iterator()} requires, so
+   *       an implementation may rely on that and must preserve it in what it writes.
    * </ul>
    */
   public abstract void write(Fields fields, NormsProducer norms) throws IOException;

lucene/core/src/java/org/apache/lucene/codecs/FieldsProducer.java

@@ -24,6 +24,9 @@
 /**
  * Abstract API that produces terms, doc, freq, prox, offset and payloads postings.
  *
+ * <p>Note that {@link Fields#iterator()} must return field names in ascending natural order; see
+ * there for what relies on it and what breaks otherwise.
+ *
  * @lucene.experimental
  */
 public abstract class FieldsProducer extends Fields implements Closeable {

lucene/core/src/java/org/apache/lucene/index/CheckIndex.java

@@ -1479,7 +1479,8 @@ private static Status.TermIndexStatus checkFields(
     String lastField = null;
     for (String field : fields) {
 
-      // MultiFieldsEnum relies upon this order...
+      // MultiFields and PerFieldPostingsFormat merge these iterators with MergedIterator, whose
+      // behaviour is undefined unless every input is sorted; see Fields#iterator().
       if (lastField != null && field.compareTo(lastField) <= 0) {
         throw new CheckIndexException(
             "fields out of order: lastField=" + lastField + " field=" + field);

lucene/core/src/java/org/apache/lucene/index/Fields.java

@@ -31,7 +31,18 @@ public abstract class Fields implements Iterable<String> {
   /** Sole constructor. (For invocation by subclass constructors, typically implicit.) */
   protected Fields() {}
 
-  /** Returns an iterator that will step through all fields names. This will not return null. */
+  /**
+   * Returns an iterator that will step through all fields names. This will not return null.
+   *
+   * <p><b>NOTE</b>: field names must be returned in ascending {@link String#compareTo natural
+   * order}. {@link MultiFields} and {@link
+   * org.apache.lucene.codecs.perfield.PerFieldPostingsFormat} merge several of these iterators with
+   * {@link org.apache.lucene.util.MergedIterator}, whose behaviour is undefined when its inputs are
+   * not sorted, so an implementation that returns field names in another order (for instance from a
+   * {@link java.util.HashMap#keySet()}) silently produces wrong results rather than an error: the
+   * merge stops deduplicating, and a name present in two sub-iterators can be returned twice.
+   * {@link CheckIndex} verifies this for the fields of an index.
+   */
   @Override
   public abstract Iterator<String> iterator();