Merge pull request #1 from desjoerd/copilot/fix-pr-comments-2645

Address PR microsoft#2645 review comments: improve type checking logic and use JsonNode.DeepEquals

Author: desjoerd

src/Microsoft.OpenApi/Models/OpenApiSchema.cs

@@ -877,20 +877,41 @@ private static (IList<IOpenApiSchema>? effective, JsonSchemaType? inferredType,
 
             if (nonNullSchemas.Count > 0)
             {
-                JsonSchemaType commonType = 0;
+                // Check if all schemas have the same type (excluding null)
+                JsonSchemaType? firstType = null;
+                bool allSameType = true;
 
                 foreach (var schema in nonNullSchemas)
                 {
-                    commonType |= schema.Type.GetValueOrDefault() & ~JsonSchemaType.Null;
+                    var schemaType = schema.Type;
+                    if (schemaType.HasValue)
+                    {
+                        // Remove null from the type using bitwise operator
+                        var typeWithoutNull = schemaType.Value & ~JsonSchemaType.Null;
+                        
+                        if (typeWithoutNull != 0)
+                        {
+                            if (firstType == null)
+                            {
+                                firstType = typeWithoutNull;
+                            }
+                            else if (firstType != typeWithoutNull)
+                            {
+                                allSameType = false;
+                                break;
+                            }
+                        }
+                    }
                 }
 
-                if (System.Enum.IsDefined(commonType))
+                if (allSameType && firstType.HasValue)
                 {
-                    // Single common type
-                    return (nonNullSchemas, commonType, true);
+                    // All schemas share the same type
+                    return (nonNullSchemas, firstType.Value, true);
                 }
                 else
                 {
+                    // Multiple different types
                     return (nonNullSchemas, null, true);
                 }
 

test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs

@@ -814,7 +814,7 @@ public async Task SerializeOneOfWithNullAsV3ShouldUseNullableAsync()
             schema.SerializeAsV3(writer);
             await writer.FlushAsync();
 
-            var v3Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v3Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV3Schema =
                 """
@@ -828,10 +828,10 @@ public async Task SerializeOneOfWithNullAsV3ShouldUseNullableAsync()
                   ],
                   "nullable": true
                 }
-                """.MakeLineBreaksEnvironmentNeutral();
+                """;
 
             // Assert
-            Assert.Equal(expectedV3Schema, v3Schema);
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV3Schema), JsonNode.Parse(v3Schema)));
         }
 
         [Fact]
@@ -855,7 +855,7 @@ public async Task SerializeOneOfWithNullAndMultipleSchemasAsV3ShouldMarkItAsNull
             schema.SerializeAsV3(writer);
             await writer.FlushAsync();
 
-            var v3Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v3Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV3Schema =
                 """
@@ -870,10 +870,10 @@ public async Task SerializeOneOfWithNullAndMultipleSchemasAsV3ShouldMarkItAsNull
                   ],
                   "nullable": true
                 }
-                """.MakeLineBreaksEnvironmentNeutral();
+                """;
 
             // Assert
-            Assert.Equal(expectedV3Schema, v3Schema);
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV3Schema), JsonNode.Parse(v3Schema)));
         }
 
         [Fact]
@@ -903,7 +903,7 @@ public async Task SerializeAnyOfWithNullAsV3ShouldUseNullableAsync()
             schema.SerializeAsV3(writer);
             await writer.FlushAsync();
 
-            var v3Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v3Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV3Schema =
                 """
@@ -921,8 +921,9 @@ public async Task SerializeAnyOfWithNullAsV3ShouldUseNullableAsync()
                   ],
                   "nullable": true
                 }
-                """.MakeLineBreaksEnvironmentNeutral();            // Assert
-            Assert.Equal(expectedV3Schema, v3Schema);
+                """;
+            // Assert
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV3Schema), JsonNode.Parse(v3Schema)));
         }
 
         [Fact]
@@ -946,7 +947,7 @@ public async Task SerializeAnyOfWithNullAndMultipleSchemasAsV3ShouldApplyNullabl
             schema.SerializeAsV3(writer);
             await writer.FlushAsync();
 
-            var v3Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v3Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV3Schema =
                 """
@@ -962,10 +963,10 @@ public async Task SerializeAnyOfWithNullAndMultipleSchemasAsV3ShouldApplyNullabl
                   ],
                   "nullable": true
                 }
-                """.MakeLineBreaksEnvironmentNeutral();
+                """;
 
             // Assert
-            Assert.Equal(expectedV3Schema, v3Schema);
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV3Schema), JsonNode.Parse(v3Schema)));
         }
 
         [Fact]
@@ -987,17 +988,17 @@ public async Task SerializeOneOfWithOnlyNullAsV3ShouldJustBeNullableAsync()
             schema.SerializeAsV3(writer);
             await writer.FlushAsync();
 
-            var v3Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v3Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV3Schema =
                 """
                 {
                   "nullable": true
                 }
-                """.MakeLineBreaksEnvironmentNeutral();
+                """;
 
             // Assert
-            Assert.Equal(expectedV3Schema, v3Schema);
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV3Schema), JsonNode.Parse(v3Schema)));
         }
 
         [Fact]
@@ -1020,7 +1021,7 @@ public async Task SerializeOneOfWithNullAsV31ShouldNotChangeAsync()
             schema.SerializeAsV31(writer);
             await writer.FlushAsync();
 
-            var v31Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v31Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV31Schema =
                 """
@@ -1034,10 +1035,10 @@ public async Task SerializeOneOfWithNullAsV31ShouldNotChangeAsync()
                     }
                   ]
                 }
-                """.MakeLineBreaksEnvironmentNeutral();
+                """;
 
             // Assert
-            Assert.Equal(expectedV31Schema, v31Schema);
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV31Schema), JsonNode.Parse(v31Schema)));
         }
 
         [Fact]
@@ -1084,7 +1085,7 @@ public async Task SerializeOneOfWithNullAndRefAsV3ShouldUseNullableAsync()
             schema.SerializeAsV3(writer);
             await writer.FlushAsync();
 
-            var v3Schema = outputStringWriter.GetStringBuilder().ToString().MakeLineBreaksEnvironmentNeutral();
+            var v3Schema = outputStringWriter.GetStringBuilder().ToString();
 
             var expectedV3Schema =
                 """
@@ -1097,10 +1098,10 @@ public async Task SerializeOneOfWithNullAndRefAsV3ShouldUseNullableAsync()
                   ],
                   "nullable": true
                 }
-                """.MakeLineBreaksEnvironmentNeutral();
+                """;
 
             // Assert
-            Assert.Equal(expectedV3Schema, v3Schema);
+            Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expectedV3Schema), JsonNode.Parse(v3Schema)));
         }
 
         internal class SchemaVisitor : OpenApiVisitorBase