From 5beb737d437a3ffc47b12b2d10855f34a4757247 Mon Sep 17 00:00:00 2001
From: Shad Storhaug <shad@shadstorhaug.com>
Date: Fri, 23 Apr 2021 17:01:21 +0700
Subject: [PATCH] Updated NullPointerException with static factory methods and
 added documentation to explain how to convert (see #446).

---
 .../RuntimeExceptions/NullPointerException.cs | 65 ++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/src/Lucene.Net/Support/ExceptionHandling/Exceptions/RuntimeExceptions/NullPointerException.cs b/src/Lucene.Net/Support/ExceptionHandling/Exceptions/RuntimeExceptions/NullPointerException.cs
index 249c2dfce4..955d0b8197 100644
--- a/src/Lucene.Net/Support/ExceptionHandling/Exceptions/RuntimeExceptions/NullPointerException.cs
+++ b/src/Lucene.Net/Support/ExceptionHandling/Exceptions/RuntimeExceptions/NullPointerException.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.CompilerServices;
 using System.Runtime.Serialization;
 
 namespace Lucene
@@ -34,12 +35,44 @@ namespace Lucene
     /// <c>null</c> object. NullPointerException objects may be constructed by the virtual machine
     /// as if suppression were disabled and/or the stack trace was not writable.
     /// <para/>
-    /// This is a Java compatibility exception, and should be thrown in
+    /// This is a Java compatibility exception, and may be thrown in
     /// Lucene.NET everywhere Lucene throws it, however catch blocks should
     /// always use the <see cref="ExceptionExtensions.IsNullPointerException(Exception)"/> method.
     /// <code>
     /// catch (Exception ex) when (ex.IsNullPointerException())
     /// </code>
+    /// The static <see cref="Create()"/> method overloads throw <see cref="ArgumentNullException"/>, which is
+    /// what we should throw in guard clauses. However, there are edge cases where it may make sense to throw
+    /// <see cref="NullReferenceException"/> instead. One example of this is when in Java an <c>Integer</c>
+    /// class is set to a primitive <c>int</c> variable.
+    /// <code>
+    /// Integer someInt = new Integer(43);<br/>
+    /// int primitiveInt = someInt; // Implicit cast by the Java compiler
+    /// </code>
+    /// If <c>someInt</c> in the above example were set to <c>null</c>, this would still compile, but would
+    /// throw <c>NullPointerException</c> at runtime.
+    /// <para/>
+    /// In .NET, <c>Integer</c> is most often translated to <c>int?</c>, making it nullable but keeping it
+    /// a value type. However setting a nullable int to a nullable one in .NET won't compile.
+    /// <code>
+    /// int? someInt = 43;<br/>
+    /// int primitiveInt = someInt; // Compile error
+    /// </code>
+    /// So, to get the same behavior as in Java (provided the nullable cannot be factored away), the
+    /// appropriate translation would be:
+    /// <code>
+    /// int? someInt = 43;<br/>
+    /// int primitiveInt;<br/>
+    /// if (someInt.HasValue)<br/>
+    ///     primitiveInt = someInt.Value;<br/>
+    /// else<br/>
+    ///     throw new NullReferenceException();
+    /// </code>
+    /// However, do note in most cases it would be better to try to refactor so the nullable
+    /// (and therefore the exception) isn't required.
+    /// <para/>
+    /// There are also other edge cases (i.e. <c>null</c> state in the middle of a method where the null value isn't being passed in)
+    /// where throwing <see cref="InvalidOperationException"/> may be more sensible, but this sort of change would need to be tested thoroughly.
     /// </summary>
     // LUCENENET: It is no longer good practice to use binary serialization. 
     // See: https://github.com/dotnet/corefx/issues/23584#issuecomment-325724568
@@ -48,18 +81,22 @@ namespace Lucene
 #endif
     internal class NullPointerException : ArgumentNullException
     {
+        [Obsolete("Use NullPointerException.Create() instead.", error: true)]
         public NullPointerException()
         {
         }
 
+        [Obsolete("Use NullPointerException.Create() instead.", error: true)]
         public NullPointerException(string message) : base(message)
         {
         }
 
+        [Obsolete("Use NullPointerException.Create() instead.", error: true)]
         public NullPointerException(string message, Exception innerException) : base(message, innerException)
         {
         }
 
+        [Obsolete("Use NullPointerException.Create() instead.", error: true)]
         public NullPointerException(Exception cause)
             : base(cause?.ToString(), cause)
         {
@@ -76,5 +113,31 @@ protected NullPointerException(SerializationInfo info, StreamingContext context)
         {
         }
 #endif
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static Exception Create() => new ArgumentNullException();
+
+        /// <summary>
+        /// LUCENENET: This overload is for a "direct" translation without passing the name of the argument. In cases where
+        /// there is no message and there is a useful argument name, it would make more senes to call <c>new ArgumentNullExcpetion()</c> directly.
+        /// Since this class is basically intended as training wheels for those who don't want to bother looking up exception types,
+        /// this is probably a reasonable default.
+        /// </summary>
+        /// <param name="message"></param>
+        /// <returns></returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static Exception Create(string message) => new ArgumentNullException(paramName: null, message);
+
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static Exception Create(string paramName, string message) => new ArgumentNullException(paramName, message);
+
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static Exception Create(string message, Exception innerException) => new ArgumentNullException(message, innerException);
+
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static Exception Create(Exception cause) => new ArgumentNullException(cause.Message, cause);
     }
 }