JsonNull return false when comparing against null (#842)

* JsonNull return false when comparing against null

I'ved modified JsonNulls comparison methods to return false when
compared against null, the matching test have also been updated
in change

Fix #820

* Updated xml comments to reflect my changes

I have updated the xml comments to reflect the changes
that was made in JsonNull and removed left over comment
in JsonNullTests

* Applied suggested change

Applied the suggested change to correct xml documentation

Co-Authored-By: Ahson Khan <ahkha@microsoft.com>
* Applied Suggested change

Applied the suggested change to correct xml documentation

Co-Authored-By: Ahson Khan <ahkha@microsoft.com>

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonNull.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonNull.cs
index 472051a..bb48216 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonNull.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonNull.cs
@@ -42,24 +42,44 @@ namespace System.Text.Json
         ///   Compares other JSON null to the value of this instance.
         /// </summary>
         /// <param name="other">The JSON null to compare against.</param>
-        /// <returns><see langword="true"/></returns>
-        public bool Equals(JsonNull other) => true;
+        /// <returns>
+        ///    <see langword="true"/> if <paramref name="other"/> is not null,
+        ///    <see langword="false"/> otherwise.
+        /// </returns>
+        public bool Equals(JsonNull other) => !(other is null);
 
         /// <summary>
         ///   Compares values of two JSON nulls.
         /// </summary>
         /// <param name="left">The JSON null to compare.</param>
         /// <param name="right">The JSON null to compare.</param>
-        /// <returns><see langword="true"/></returns>
-        public static bool operator ==(JsonNull left, JsonNull right) => true;
+        /// <returns>
+        ///    <see langword="true"/> if both instances match,
+        ///    <see langword="false"/> otherwise.
+        /// </returns>
+        public static bool operator ==(JsonNull left, JsonNull right)
+        {
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
+
+            return right.Equals(left);
+        }
 
         /// <summary>
         ///   Compares values of two JSON nulls.
         /// </summary>
         /// <param name="left">The JSON null to compare.</param>
         /// <param name="right">The JSON null to compare.</param>
-        /// <returns><see langword="false"/></returns>
-        public static bool operator !=(JsonNull left, JsonNull right) => false;
+        /// <returns>
+        ///    <see langword="true"/> if both instances do not match,
+        ///    <see langword="false"/> otherwise.
+        /// </returns>
+        public static bool operator !=(JsonNull left, JsonNull right) => !(left == right);
 
         /// <summary>
         ///   Creates a new JSON null that is a copy of the current instance.

diff --git a/src/libraries/System.Text.Json/tests/JsonNullTests.cs b/src/libraries/System.Text.Json/tests/JsonNullTests.cs
index bc0891e..17adada 100644 (file)
--- a/src/libraries/System.Text.Json/tests/JsonNullTests.cs
+++ b/src/libraries/System.Text.Json/tests/JsonNullTests.cs
@@ -45,17 +45,15 @@ namespace System.Text.Json.Tests
             Assert.False(jsonNull1.Equals(new JsonString("null")));
             Assert.False(jsonNull1.Equals(new Exception()));
 
-            // Null comparisons behave this way because of implicit conversion from null to JsonNull:
-           
-            Assert.True(jsonNull1.Equals(null));
-            Assert.True(jsonNull1 == null);
-            Assert.False(jsonNull1 != null);
+            Assert.False(jsonNull1.Equals(null));
+            Assert.False(jsonNull1 == null);
+            Assert.True(jsonNull1 != null);
 
             JsonNull jsonNullNull = null;
 
-            Assert.True(jsonNull1.Equals(jsonNullNull));
-            Assert.True(jsonNull1 == jsonNullNull);
-            Assert.False(jsonNull1 != jsonNullNull);
+            Assert.False(jsonNull1.Equals(jsonNullNull));
+            Assert.False(jsonNull1 == jsonNullNull);
+            Assert.True(jsonNull1 != jsonNullNull);
 
             JsonNull otherJsonNullNull = null;
             Assert.True(jsonNullNull == otherJsonNullNull);
@@ -64,7 +62,7 @@ namespace System.Text.Json.Tests
 
             JsonNode jsonNodeNull = null;
             Assert.False(jsonNull1.Equals(jsonNodeNull));
-          
+
             JsonArray jsonArrayNull = null;
             Assert.False(jsonNull1.Equals(jsonArrayNull));
         }
@@ -75,10 +73,10 @@ namespace System.Text.Json.Tests
             var jsonNull = new JsonNull();
 
             Assert.Equal(jsonNull.GetHashCode(), new JsonNull().GetHashCode());
-            
+
             JsonNode jsonNodeNull = new JsonNull();
             Assert.Equal(jsonNull.GetHashCode(), jsonNodeNull.GetHashCode());
-            
+
             IEquatable<JsonNull> jsonNullIEquatable = new JsonNull();
             Assert.Equal(jsonNull.GetHashCode(), jsonNullIEquatable.GetHashCode());