BREAKING: Lucene.Net.Index.IndexWriter: Fixed Dispose() overloads so …

…there is no method signature conflict between the public Dispose(waitForMerges) method and the protected Dispose(disposing) method that can be overridden and called from a finalizer. See apache#265.

src/Lucene.Net/Index/IndexWriter.cs

@@ -1,14 +1,11 @@
 using J2N;
-using J2N.Text;
-using J2N.Threading;
 using J2N.Threading.Atomic;
 using Lucene.Net.Diagnostics;
 using Lucene.Net.Support;
 using Lucene.Net.Support.Threading;
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.ComponentModel;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.IO;
@@ -1045,7 +1042,7 @@ private void MessageState()
         /// This is a "slow graceful shutdown" which may take a long time
         /// especially if a big merge is pending: If you only want to close
         /// resources use <see cref="Rollback()"/>. If you only want to commit
-        /// pending changes and close resources see <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/>.
+        /// pending changes and close resources see <see cref="Dispose(bool)"/>.
         /// <para/>
         /// Note that this may be a costly
         /// operation, so, try to re-use a single writer instead of
@@ -1092,36 +1089,42 @@ private void MessageState()
         [MethodImpl(MethodImplOptions.NoInlining)]
         public void Dispose()
         {
-            Dispose(true);
+            Dispose(disposing: true, waitForMerges: true);
             GC.SuppressFinalize(this);
         }
 
         /// <summary>
-        /// Disposes the index while waiting for currently
-        /// running merges to finish and optionally releases the managed resources.
+        /// Disposes the index with or without waiting for currently
+        /// running merges to finish.  This is only meaningful when
+        /// using a <see cref="MergeScheduler"/> that runs merges in background
+        /// threads.
         ///
         /// <para><b>NOTE</b>: If this method hits an <see cref="OutOfMemoryException"/>
         /// you should immediately dispose the writer, again.  See 
         /// <see cref="IndexWriter"/> for details.</para>
         ///
-        /// <para><b>NOTE</b>: Always override <see cref="Dispose(bool, bool)"/> instead of this overload.</para>
+        /// <para><b>NOTE</b>: It is dangerous to always call
+        /// <c>Dispose(false)</c>, especially when <see cref="IndexWriter"/> is not open
+        /// for very long, because this can result in "merge
+        /// starvation" whereby long merges will never have a
+        /// chance to finish.  This will cause too many segments in
+        /// your index over time.</para>
         ///
-        /// <para><b>NOTE</b>: The public extension method <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/>
-        /// has the same method signature as this method. However, that method is intended for external calls by Lucene users,
-        /// where this one is intended to be used by subclasses to complete the
-        /// <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose#the-disposebool-method-overload">
-        /// .NET dispose pattern</a>.</para>
+        /// <para><b>NOTE</b>: This overload should not be called when implementing a finalizer.
+        /// Instead, call <see cref="Dispose(bool, bool)"/> with <c>disposing</c> set to
+        /// <c>false</c> and <c>waitForMerges</c> set to <c>true</c>.</para>
         /// </summary>
-        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources;
-        /// <c>false</c> to release only unmanaged resources. </param>
-        // LUCENENET specific - This overload only exists to implement the dispose pattern, and should not be overridden because
-        // it doesn't necessarily capture all of the Dispose() calls like Dispose(bool disposing, bool waitForMerges) does.
-        // This only exists to keep code analyzers from complaining that we are out of spec. Finalizers and should call Dispose(true, true).
+        /// <param name="waitForMerges"> If <c>true</c>, this call will block
+        /// until all merges complete; else, it will ask all
+        /// running merges to abort, wait until those merges have
+        /// finished (which should be at most a few seconds), and
+        /// then return. </param>
         [MethodImpl(MethodImplOptions.NoInlining)]
-        [EditorBrowsable(EditorBrowsableState.Never)]
-        protected virtual void Dispose(bool disposing)
+        [SuppressMessage("Usage", "CA1816:Dispose methods should call SuppressFinalize", Justification = "This is Lucene's alternate path to Dispose() and we must suppress the finalizer here.")]
+        public void Dispose(bool waitForMerges)
         {
-            Dispose(disposing, waitForMerges: true);
+            Dispose(disposing: true, waitForMerges);
+            GC.SuppressFinalize(this);
         }
 
         /// <summary>
@@ -1137,8 +1140,8 @@ protected virtual void Dispose(bool disposing)
         /// then return.
         /// </para>
         ///
-        /// <para><b>NOTE</b>: When overriding, this overload should always be used instead of <see cref="Dispose(bool)"/>
-        /// to ensure calls from <see cref="Dispose()"/> and <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/>
+        /// <para><b>NOTE</b>: When overriding this overload should always be used
+        /// instead of <see cref="Dispose(bool)"/> to ensure calls from <see cref="Dispose()"/> and <see cref="Dispose(bool)"/>
         /// are both handled. Always be sure to call <c>base.Dispose(disposing, waitForMerges)</c> when overriding this method.</para>
         ///
         /// <para><b>NOTE</b>: If this method hits an <see cref="OutOfMemoryException"/>
@@ -1163,14 +1166,15 @@ protected virtual void Dispose(bool disposing)
         // LUCENENET specific - Added this overload to allow subclasses to dispose resoruces
         // in one place without also having to override Dispose(bool).
         [MethodImpl(MethodImplOptions.NoInlining)]
-        protected internal virtual void Dispose(bool disposing, bool waitForMerges)
+        protected virtual void Dispose(bool disposing, bool waitForMerges)
         {
             if (disposing)
             {
                 Close(waitForMerges);
             }
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void Close(bool waitForMerges)
         {
             // Ensure that only one thread actually gets to do the
@@ -2250,7 +2254,7 @@ internal string NewSegmentName()
         /// you should immediately dispose the writer.  See
         /// <see cref="IndexWriter"/> for details.</para>
         ///
-        /// <para><b>NOTE</b>: if you call <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/>
+        /// <para><b>NOTE</b>: if you call <see cref="Dispose(bool)"/>
         /// with <c>false</c>, which aborts all running merges,
         /// then any thread still running this method might hit a
         /// <see cref="MergePolicy.MergeAbortedException"/>.</para>
@@ -2432,7 +2436,7 @@ private bool MaxNumSegmentsMergesPending()
         /// you should immediately dispose the writer.  See
         /// <see cref="IndexWriter"/> for details.</para>
         ///
-        /// <para><b>NOTE</b>: if you call <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/>
+        /// <para><b>NOTE</b>: if you call <see cref="Dispose(bool)"/>
         /// with <c>false</c>, which aborts all running merges,
         /// then any thread still running this method might hit a
         /// <see cref="MergePolicy.MergeAbortedException"/>.</para>
@@ -3488,7 +3492,7 @@ public virtual void AddIndexes(params Directory[] dirs)
         /// <see cref="DirectoryReader.Open(Directory, int)"/>.
         ///
         /// <para/>
-        /// <b>NOTE</b>: if you call <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/> with <c>false</c>, which
+        /// <b>NOTE</b>: if you call <see cref="Dispose(bool)"/> with <c>false</c>, which
         /// aborts all running merges, then any thread still running this method might
         /// hit a <see cref="MergePolicy.MergeAbortedException"/>.
         /// </summary>
@@ -6466,46 +6470,4 @@ private static bool SlowFileExists(Directory dir, string fileName)
             }
         }
     }
-
-    /// <summary>
-    /// Extensions to <see cref="IndexWriter"/>.
-    /// </summary>
-    public static class IndexWriterExtensions
-    {
-        /// <summary>
-        /// Disposes the index with or without waiting for currently
-        /// running merges to finish.  This is only meaningful when
-        /// using a <see cref="MergeScheduler"/> that runs merges in background
-        /// threads.
-        ///
-        /// <para><b>NOTE</b>: If this method hits an <see cref="OutOfMemoryException"/>
-        /// you should immediately dispose the writer, again.  See 
-        /// <see cref="IndexWriter"/> for details.</para>
-        ///
-        /// <para><b>NOTE</b>: It is dangerous to always call
-        /// <c>Dispose(false)</c>, especially when <see cref="IndexWriter"/> is not open
-        /// for very long, because this can result in "merge
-        /// starvation" whereby long merges will never have a
-        /// chance to finish.  This will cause too many segments in
-        /// your index over time.</para>
-        /// </summary>
-        /// <param name="waitForMerges"> If <c>true</c>, this call will block
-        /// until all merges complete; else, it will ask all
-        /// running merges to abort, wait until those merges have
-        /// finished (which should be at most a few seconds), and
-        /// then return. </param>
-        /// <exception cref="ArgumentNullException"><paramref name="indexWriter"/> is <c>null</c>.</exception>
-        // LUCENENET specific - added to allow a public call to Dispose(bool) (where the parameter means
-        // waitForMerges) for external use, while still using a protected Dispose(bool) (where the parameter
-        // means being called from Dispose() vs finalizer) when implementing subclasses of IndexWriter.
-        [SuppressMessage("Usage", "CA1816:Dispose methods should call SuppressFinalize", Justification = "This is Lucene's alternate path to Dispose() and we must suppress the finalizer here.")]
-        public static void Dispose(this IndexWriter indexWriter, bool waitForMerges)
-        {
-            if (indexWriter is null)
-                throw new ArgumentNullException(nameof(indexWriter));
-
-            indexWriter.Dispose(disposing: true, waitForMerges);
-            GC.SuppressFinalize(indexWriter);
-        }
-    }
 }

src/Lucene.Net/Index/MergePolicy.cs

@@ -540,7 +540,7 @@ protected MergeException(SerializationInfo info, StreamingContext context)
 
         /// <summary>
         /// Thrown when a merge was explicity aborted because
-        /// <see cref="IndexWriterExtensions.Dispose(IndexWriter, bool)"/> was called with
+        /// <see cref="IndexWriter.Dispose(bool)"/> was called with
         /// <c>false</c>.  Normally this exception is
         /// privately caught and suppresed by <see cref="IndexWriter"/>.
         /// </summary>