Address PR review feedback on Collation port, #1343

- Rename CollatedTermAttributeImpl class to CollatedTermAttribute (keep file name)
- Add ICU span fast path (no string alloc) to CollatedTermAttribute.FillBytesRef
  and CollationKeyFilter, gated on new FEATURE_COMPAREINFO_SPAN_SORTKEY; NLS keeps
  the slow path
- Add public CollationUtil.IsICU/IsNLS (statically cached) for runtime backend
  detection and document it in package.md
- package.md: use Farsi "fa" instead of "ar", and demonstrate using on
  IndexWriter/IndexReader examples
- Rename NewCollationAnalyzerTask Implementation.BCL to DotNet (keep jdk/bcl param
  aliases); update TestPerfTasksLogic to impl:dotnet
- CollationTestBase: remove stale icu-net comment, rename FranceResult to
  FrenchResult, reflow sortData to match upstream
- Tests: use "fa" in TestCollationKeyAnalyzer/TestCollationKeyFilter, revert
  TestCollationKeyFilterFactory.TestNormalization to original decomposed input,
  and document why upstream testCustomRules was omitted

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: paulirwin

Directory.Build.targets

@@ -77,6 +77,8 @@
 
     <DefineConstants>$(DefineConstants);FEATURE_MEMORYMARSHAL_GETARRAYDATAREFERENCE</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_READONLYSET</DefineConstants>
+    <!-- CompareInfo.GetSortKey(ReadOnlySpan<char>, Span<byte>, CompareOptions) and GetSortKeyLength(ReadOnlySpan<char>, CompareOptions) -->
+    <DefineConstants>$(DefineConstants);FEATURE_COMPAREINFO_SPAN_SORTKEY</DefineConstants>
 
   </PropertyGroup>
 

src/Lucene.Net.Analysis.Common/Collation/CollationAttributeFactory.cs

@@ -116,8 +116,8 @@ public CollationAttributeFactory(AttributeFactory @delegate, CompareInfo collato
 
         public override Attribute CreateAttributeInstance<T>()
         {
-            return typeof(T).IsAssignableFrom(typeof(CollatedTermAttributeImpl))
-                ? new CollatedTermAttributeImpl(this.collator, this.options)
+            return typeof(T).IsAssignableFrom(typeof(CollatedTermAttribute))
+                ? new CollatedTermAttribute(this.collator, this.options)
                 : this.@delegate.CreateAttributeInstance<T>();
         }
     }

src/Lucene.Net.Analysis.Common/Collation/CollationKeyFilter.cs

@@ -96,8 +96,7 @@ public override bool IncrementToken()
         {
             if (this.m_input.IncrementToken())
             {
-                // LUCENENET: Normalize to NFC for cross-backend consistency. See CollatedTermAttributeImpl.Normalize.
-                byte[] collationKey = this.collator.GetSortKey(TokenAttributes.CollatedTermAttributeImpl.Normalize(this.termAtt.ToString()), this.options).KeyData;
+                byte[] collationKey = GetCollationKey();
                 int encodedLength = IndexableBinaryStringTools.GetEncodedLength(collationKey, 0, collationKey.Length);
                 this.termAtt.ResizeBuffer(encodedLength);
                 this.termAtt.Length = encodedLength;
@@ -110,5 +109,24 @@ public override bool IncrementToken()
                 return false;
             }
         }
+
+        private byte[] GetCollationKey()
+        {
+#if FEATURE_COMPAREINFO_SPAN_SORTKEY
+            // LUCENENET: On .NET 5+ with ICU, CompareInfo has a ReadOnlySpan<char> overload of GetSortKey
+            // that lets us generate the sort key without converting the term to a string first. The NLS
+            // backend does not normalize internally (see CollatedTermAttribute.Normalize), so we still need
+            // the slow path when the app is configured for NLS.
+            if (CollationUtil.IsICU)
+            {
+                ReadOnlySpan<char> source = this.termAtt.Buffer.AsSpan(0, this.termAtt.Length);
+                byte[] collationKey = new byte[this.collator.GetSortKeyLength(source, this.options)];
+                this.collator.GetSortKey(source, collationKey, this.options);
+                return collationKey;
+            }
+#endif
+            // LUCENENET: Normalize to NFC for cross-backend consistency. See CollatedTermAttribute.Normalize.
+            return this.collator.GetSortKey(TokenAttributes.CollatedTermAttribute.Normalize(this.termAtt.ToString()), this.options).KeyData;
+        }
     }
 }

src/Lucene.Net.Analysis.Common/Collation/CollationUtil.cs

@@ -0,0 +1,59 @@
+// Lucene version compatibility level 4.8.1
+using System.Globalization;
+
+namespace Lucene.Net.Collation
+{
+    /*
+     * Licensed to the Apache Software Foundation (ASF) under one or more
+     * contributor license agreements.  See the NOTICE file distributed with
+     * this work for additional information regarding copyright ownership.
+     * The ASF licenses this file to You under the Apache License, Version 2.0
+     * (the "License"); you may not use this file except in compliance with
+     * the License.  You may obtain a copy of the License at
+     *
+     *     http://www.apache.org/licenses/LICENSE-2.0
+     *
+     * Unless required by applicable law or agreed to in writing, software
+     * distributed under the License is distributed on an "AS IS" BASIS,
+     * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     * See the License for the specific language governing permissions and
+     * limitations under the License.
+     */
+
+    /// <summary>
+    /// LUCENENET specific helpers for working with the platform collator
+    /// (<see cref="CompareInfo"/>) used by <see cref="CollationKeyAnalyzer"/> and
+    /// <see cref="CollationAttributeFactory"/>.
+    /// </summary>
+    public static class CollationUtil
+    {
+        /// <summary>
+        /// Indicates whether the active globalization backend is ICU (the default on .NET 5+)
+        /// rather than the legacy NLS implementation. .NET Framework always uses NLS, while .NET 5+
+        /// uses ICU by default but can be configured to use NLS (for example via the
+        /// <c>System.Globalization.UseNls</c> runtime switch or the <c>DOTNET_SYSTEM_GLOBALIZATION_USENLS</c>
+        /// environment variable). NLS and ICU produce different sort keys and orderings, so this value
+        /// (along with the runtime version and culture) is part of what must match between index time
+        /// and query time when searching against stored sort keys.
+        /// <para/>
+        /// See <a href="https://learn.microsoft.com/en-us/dotnet/core/extensions/globalization-icu#determine-if-your-app-is-using-icu">Determine if your app is using ICU</a>.
+        /// </summary>
+        public static bool IsICU { get; } = DetectICU();
+
+        /// <summary>
+        /// Indicates whether the active globalization backend is the legacy NLS implementation
+        /// (always the case on .NET Framework, and the default on Windows when the app opts out of ICU)
+        /// rather than ICU. This is the inverse of <see cref="IsICU"/>.
+        /// </summary>
+        public static bool IsNLS => !IsICU;
+
+        // LUCENENET: Detects whether globalization is backed by ICU. See the link in the IsICU docs.
+        private static bool DetectICU()
+        {
+            SortVersion sortVersion = CultureInfo.InvariantCulture.CompareInfo.Version;
+            byte[] bytes = sortVersion.SortId.ToByteArray();
+            int version = bytes[3] << 24 | bytes[2] << 16 | bytes[1] << 8 | bytes[0];
+            return version != 0 && version == sortVersion.FullVersion;
+        }
+    }
+}

src/Lucene.Net.Analysis.Common/Collation/TokenAttributes/CollatedTermAttributeImpl.cs

@@ -1,6 +1,7 @@
 // Lucene version compatibility level 4.8.1
 using Lucene.Net.Analysis.TokenAttributes;
 using Lucene.Net.Util;
+using System;
 using System.Globalization;
 
 namespace Lucene.Net.Collation.TokenAttributes
@@ -26,25 +27,25 @@ namespace Lucene.Net.Collation.TokenAttributes
     /// Extension of <see cref="CharTermAttribute"/> that encodes the term
     /// text as a binary Unicode collation key instead of as UTF-8 bytes.
     /// </summary>
-    public class CollatedTermAttributeImpl : CharTermAttribute
+    public class CollatedTermAttribute : CharTermAttribute
     {
         private readonly CompareInfo collator;
         private readonly CompareOptions options;
 
         /// <summary>
-        /// Create a new <see cref="CollatedTermAttributeImpl"/> </summary>
+        /// Create a new <see cref="CollatedTermAttribute"/> </summary>
         /// <param name="collator"> Collation key generator </param>
-        public CollatedTermAttributeImpl(CompareInfo collator)
+        public CollatedTermAttribute(CompareInfo collator)
             : this(collator, CompareOptions.None)
         {
         }
 
         /// <summary>
-        /// Create a new <see cref="CollatedTermAttributeImpl"/> </summary>
+        /// Create a new <see cref="CollatedTermAttribute"/> </summary>
         /// <param name="collator"> Collation key generator </param>
         /// <param name="options"> Collation options that control the collation strength and
         /// Unicode normalization (decomposition) of the generated sort key. </param>
-        public CollatedTermAttributeImpl(CompareInfo collator, CompareOptions options)
+        public CollatedTermAttribute(CompareInfo collator, CompareOptions options)
         {
             // LUCENENET: Unlike java.text.Collator, System.Globalization.CompareInfo is
             // immutable and thread-safe, so there is no need to clone it here.
@@ -55,12 +56,32 @@ public CollatedTermAttributeImpl(CompareInfo collator, CompareOptions options)
         public override void FillBytesRef()
         {
             BytesRef bytes = this.BytesRef;
+#if FEATURE_COMPAREINFO_SPAN_SORTKEY
+            // LUCENENET: On .NET 5+ with ICU, CompareInfo has a ReadOnlySpan<char> overload of GetSortKey
+            // that writes directly into a destination buffer, so we can generate the sort key from the term
+            // text without the intermediate string allocation. The NLS backend does not normalize internally
+            // (see Normalize), so we still need the slow path when the app is configured for NLS.
+            if (CollationUtil.IsICU)
+            {
+                ReadOnlySpan<char> source = this.AsSpan();
+                int keyLength = this.collator.GetSortKeyLength(source, this.options);
+                bytes.Bytes = new byte[keyLength];
+                bytes.Offset = 0;
+                bytes.Length = this.collator.GetSortKey(source, bytes.Bytes, this.options);
+                return;
+            }
+#endif
             byte[] keyData = this.collator.GetSortKey(Normalize(this.ToString()), this.options).KeyData;
             bytes.Bytes = keyData;
             bytes.Offset = 0;
             bytes.Length = keyData.Length;
         }
 
+#if FEATURE_COMPAREINFO_SPAN_SORTKEY
+        // LUCENENET: Returns the term text as a span over the backing buffer, avoiding a string allocation.
+        private ReadOnlySpan<char> AsSpan() => this.Buffer.AsSpan(0, this.Length);
+#endif
+
         // LUCENENET: Normalize the term to Unicode Normalization Form C (NFC) before generating the
         // sort key. ICU-backed collators (.NET 5+) normalize internally, but the NLS-backed collator
         // (.NET Framework) does not fully handle decomposed combining sequences (e.g. "I" + U+0307 vs

src/Lucene.Net.Analysis.Common/Collation/package.md

@@ -41,15 +41,16 @@ summary: *content
 ### Farsi Range Queries
 
 ```cs
-CompareInfo collator = CompareInfo.GetCompareInfo("ar");
+CompareInfo collator = CompareInfo.GetCompareInfo("fa");
 CollationKeyAnalyzer analyzer = new CollationKeyAnalyzer(LuceneVersion.LUCENE_48, collator);
 Store.Directory ramDir = new RAMDirectory();
-IndexWriter writer = new IndexWriter(ramDir, new IndexWriterConfig(LuceneVersion.LUCENE_48, analyzer));
-Document doc = new Document();
-doc.Add(new TextField("content", "ساب", Field.Store.YES));
-writer.AddDocument(doc);
-writer.Dispose();
-IndexReader ir = DirectoryReader.Open(ramDir);
+using (IndexWriter writer = new IndexWriter(ramDir, new IndexWriterConfig(LuceneVersion.LUCENE_48, analyzer)))
+{
+    Document doc = new Document();
+    doc.Add(new TextField("content", "ساب", Field.Store.YES));
+    writer.AddDocument(doc);
+}
+using IndexReader ir = DirectoryReader.Open(ramDir);
 IndexSearcher searcher = new IndexSearcher(ir);
 
 QueryParser aqp = new QueryParser(LuceneVersion.LUCENE_48, "content", analyzer);
@@ -58,7 +59,7 @@ aqp.AnalyzeRangeTerms = true;
 // Unicode order would include U+0633 in [ U+062F - U+0698 ], but Farsi
 // orders the U+0698 character before the U+0633 character, so the single
 // indexed Term above should NOT be returned by a TermRangeQuery with a
-// Farsi Collator (or an Arabic one for the case when Farsi is not supported).
+// Farsi Collator.
 ScoreDoc[] result = searcher.Search(aqp.Parse("[ د TO ژ ]"), null, 1000).ScoreDocs;
 assertEquals("The index Term should not be included.", 0, result.Length);
 ```
@@ -68,19 +69,20 @@ assertEquals("The index Term should not be included.", 0, result.Length);
 ```cs
 Analyzer analyzer = new CollationKeyAnalyzer(LuceneVersion.LUCENE_48, CompareInfo.GetCompareInfo("da-DK"));
 Store.Directory indexStore = new RAMDirectory();
-IndexWriter writer = new IndexWriter(indexStore, new IndexWriterConfig(LuceneVersion.LUCENE_48, analyzer));
-string[] tracer = new string[] { "A", "B", "C", "D", "E" };
-string[] data = new string[] { "HAT", "HUT", "HÅT", "HØT", "HOT" };
-string[] sortedTracerOrder = new string[] { "A", "E", "B", "D", "C" };
-for (int i = 0; i < data.Length; ++i)
+using (IndexWriter writer = new IndexWriter(indexStore, new IndexWriterConfig(LuceneVersion.LUCENE_48, analyzer)))
 {
-    Document doc = new Document();
-    doc.Add(new StoredField("tracer", tracer[i]));
-    doc.Add(new TextField("contents", data[i], Field.Store.NO));
-    writer.AddDocument(doc);
+    string[] tracer = new string[] { "A", "B", "C", "D", "E" };
+    string[] data = new string[] { "HAT", "HUT", "HÅT", "HØT", "HOT" };
+    for (int i = 0; i < data.Length; ++i)
+    {
+        Document doc = new Document();
+        doc.Add(new StoredField("tracer", tracer[i]));
+        doc.Add(new TextField("contents", data[i], Field.Store.NO));
+        writer.AddDocument(doc);
+    }
 }
-writer.Dispose();
-IndexReader ir = DirectoryReader.Open(indexStore);
+string[] sortedTracerOrder = new string[] { "A", "E", "B", "D", "C" };
+using IndexReader ir = DirectoryReader.Open(indexStore);
 IndexSearcher searcher = new IndexSearcher(ir);
 Sort sort = new Sort();
 sort.SetSort(new SortField("contents", SortFieldType.STRING));
@@ -101,12 +103,13 @@ CompareInfo collator = CompareInfo.GetCompareInfo("tr-TR");
 Analyzer analyzer = new CollationKeyAnalyzer(LuceneVersion.LUCENE_48, collator,
     CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace);
 Store.Directory ramDir = new RAMDirectory();
-IndexWriter writer = new IndexWriter(ramDir, new IndexWriterConfig(LuceneVersion.LUCENE_48, analyzer));
-Document doc = new Document();
-doc.Add(new TextField("contents", "DIGY", Field.Store.NO));
-writer.AddDocument(doc);
-writer.Dispose();
-IndexReader ir = DirectoryReader.Open(ramDir);
+using (IndexWriter writer = new IndexWriter(ramDir, new IndexWriterConfig(LuceneVersion.LUCENE_48, analyzer)))
+{
+    Document doc = new Document();
+    doc.Add(new TextField("contents", "DIGY", Field.Store.NO));
+    writer.AddDocument(doc);
+}
+using IndexReader ir = DirectoryReader.Open(ramDir);
 IndexSearcher searcher = new IndexSearcher(ir);
 QueryParser parser = new QueryParser(LuceneVersion.LUCENE_48, "contents", analyzer);
 Query query = parser.Parse("dıgy");   // U+0131: dotless i
@@ -118,7 +121,7 @@ assertEquals("The index Term should be included.", 1, result.Length);
 
  __WARNING:__ Make sure you use exactly the same collator (`System.Globalization.CompareInfo` *and* `System.Globalization.CompareOptions`) at index and query time -- `System.Globalization.SortKey`s are only comparable when produced by the same collator. Since the platform collator is not independently versioned, it is unsafe to search against stored `System.Globalization.SortKey`s unless the following are exactly the same (best practice is to store this information with the index and check that they remain the same at query time):
 
-1.  The .NET runtime version, and the active globalization backend. .NET Framework uses Windows NLS, while .NET 5+ uses ICU by default; the two produce different sort keys and orderings.
+1.  The .NET runtime version, and the active globalization backend. .NET Framework uses Windows NLS, while .NET 5+ uses ICU by default; the two produce different sort keys and orderings. You can detect the active backend at runtime via <xref:Lucene.Net.Collation.CollationUtil.IsICU> (or its inverse, <xref:Lucene.Net.Collation.CollationUtil.IsNLS>) and store that value with the index to verify it matches at query time.
 
 2.  The language (and country and variant, if specified) of the culture used when obtaining the collator via `System.Globalization.CompareInfo.GetCompareInfo`.
 

src/Lucene.Net.Benchmark/ByTask/Tasks/NewCollationAnalyzerTask.cs

@@ -33,7 +33,7 @@ public static Type GetAnalyzerType(this NewCollationAnalyzerTask.Implementation
         {
             switch (impl)
             {
-                case NewCollationAnalyzerTask.Implementation.BCL:
+                case NewCollationAnalyzerTask.Implementation.DotNet:
                     return typeof(Lucene.Net.Collation.CollationKeyAnalyzer);
 
                 case NewCollationAnalyzerTask.Implementation.ICU:
@@ -46,8 +46,8 @@ public static Type GetCollatorType(this NewCollationAnalyzerTask.Implementation
         {
             switch (impl)
             {
-                case NewCollationAnalyzerTask.Implementation.BCL:
-                    // LUCENENET: The .NET (BCL) equivalent of the JDK's java.text.Collator is the
+                case NewCollationAnalyzerTask.Implementation.DotNet:
+                    // LUCENENET: The .NET equivalent of the JDK's java.text.Collator is the
                     // platform collator, System.Globalization.CompareInfo.
                     return typeof(CompareInfo);
 
@@ -69,16 +69,16 @@ public class NewCollationAnalyzerTask : PerfTask
         public enum Implementation
         {
             // LUCENENET: This value is named JDK in upstream Lucene (the collator comes from the
-            // Java Development Kit's java.text.Collator). It has been renamed to BCL here because the
-            // equivalent in .NET is the platform collator (System.Globalization.CompareInfo) provided
-            // by the Base Class Library. The "jdk" parameter value is still accepted as a legacy alias.
+            // Java Development Kit's java.text.Collator). It has been renamed to DotNet here because the
+            // equivalent in .NET is the platform collator (System.Globalization.CompareInfo). The "jdk"
+            // (and legacy "bcl") parameter values are still accepted as aliases.
 
             /// <summary>The .NET platform collator (<see cref="CompareInfo"/>), equivalent to the JDK's <c>java.text.Collator</c>.</summary>
-            BCL,
+            DotNet,
             ICU
         }
 
-        private Implementation impl = Implementation.BCL;
+        private Implementation impl = Implementation.DotNet;
 
         public NewCollationAnalyzerTask(PerfRunData runData)
             : base(runData)
@@ -88,7 +88,7 @@ public NewCollationAnalyzerTask(PerfRunData runData)
         internal static Analyzer CreateAnalyzer(CultureInfo locale, Implementation impl)
         {
             // LUCENENET specific - senseless to use reflection here, so we construct the collator
-            // for the chosen implementation directly. The BCL implementation (named JDK in upstream
+            // for the chosen implementation directly. The DotNet implementation (named JDK in upstream
             // Lucene) maps to the .NET platform collator (System.Globalization.CompareInfo); ICU maps
             // to ICU4N's Collator.
             object collator = impl == Implementation.ICU
@@ -139,11 +139,12 @@ public override void SetParams(string @params)
                 {
                     if (value.Equals("icu", StringComparison.OrdinalIgnoreCase))
                         impl = Implementation.ICU;
-                    // LUCENENET: "bcl" is the .NET name for what upstream Lucene calls "jdk"; we accept
-                    // "jdk" as a legacy alias so existing Lucene benchmark .alg files keep working.
-                    else if (value.Equals("bcl", StringComparison.OrdinalIgnoreCase)
-                        || value.Equals("jdk", StringComparison.OrdinalIgnoreCase))
-                        impl = Implementation.BCL;
+                    // LUCENENET: "dotnet" maps to what upstream Lucene calls "jdk"; we accept "jdk" (and
+                    // the legacy "bcl") as aliases so existing Lucene benchmark .alg files keep working.
+                    else if (value.Equals("dotnet", StringComparison.OrdinalIgnoreCase)
+                        || value.Equals("jdk", StringComparison.OrdinalIgnoreCase)
+                        || value.Equals("bcl", StringComparison.OrdinalIgnoreCase))
+                        impl = Implementation.DotNet;
                     else
                         throw RuntimeException.Create("Unknown parameter " + param);
                 }