Add migrating-newtonsoft-to-system-text-json skill

.github/CODEOWNERS

@@ -53,6 +53,9 @@
 /plugins/dotnet-upgrade/skills/migrate-dotnet8-to-dotnet9/ @danmoseley @dotnet/compat
 /tests/dotnet-upgrade/migrate-dotnet8-to-dotnet9/ @danmoseley @dotnet/compat
 
+/plugins/dotnet-upgrade/skills/migrating-newtonsoft-to-system-text-json/ @dotnet/area-system-text-json @mrsharm
+/tests/dotnet-upgrade/migrating-newtonsoft-to-system-text-json/ @dotnet/area-system-text-json @mrsharm
+
 # dotnet-diag (perf investigations, debugging, incident analysis)
 /plugins/dotnet-diag/skills/analyzing-dotnet-performance/ @dotnet/dotnet-diag
 /tests/dotnet-diag/analyzing-dotnet-performance/ @dotnet/dotnet-diag

plugins/dotnet-upgrade/skills/migrating-newtonsoft-to-system-text-json/SKILL.md

@@ -0,0 +1,185 @@
+---
+name: migrating-newtonsoft-to-system-text-json
+description: >
+  Migrate from Newtonsoft.Json to System.Text.Json, handling behavioral differences,
+  custom converters, and common breaking changes. Use when converting a project from
+  Newtonsoft.Json (Json.NET) to the built-in System.Text.Json serializer.
+---
+
+# Migrating from Newtonsoft.Json to System.Text.Json
+
+> **Important:** Migrating serializers is a nontrivial task. System.Text.Json will almost
+> certainly behave differently from Newtonsoft.Json in subtle ways. Always validate
+> serialization output and deserialization behavior thoroughly with real-world data after
+> migrating. Automated and manual testing of all serialization paths is essential.
+
+## When to Use
+
+- Migrating an existing project from Newtonsoft.Json to System.Text.Json
+- Removing the Newtonsoft.Json dependency for performance or AOT compatibility
+- Fixing serialization differences after switching to System.Text.Json
+
+## Inputs
+
+| Input | Required | Description |
+|-------|----------|-------------|
+| Code using Newtonsoft.Json | Yes | Models, serialization calls, custom converters |
+| .NET version | No | Determines which System.Text.Json features are available |
+
+## Workflow
+
+### Step 1: Understand the critical behavioral differences
+
+**System.Text.Json is NOT a drop-in replacement.** These behaviors differ by default:
+
+| Behavior | Newtonsoft.Json | System.Text.Json | Impact |
+|----------|----------------|-------------------|--------|
+| **Character escaping** | Only escapes JSON-spec chars | **Escapes non-ASCII and HTML-sensitive** | Output differs but is equivalent; use `JavaScriptEncoder.UnsafeRelaxedJsonEscaping` if needed |
+| **Trailing commas** | Allowed | **Rejected by default** | Parse errors on valid-looking JSON |
+| **Comments in JSON** | Allowed | **Rejected by default** | Config files break |
+| **Number in string** (`"123"`) | Coerced automatically | **Throws by default** | Deserialization breaks! |
+| **Case sensitivity** | Case-insensitive | **Case-sensitive by default** | Property matching breaks |
+| **Circular references** | `$ref/$id` with PreserveReferencesHandling | `ReferenceHandler.Preserve` (.NET 5+) | API differs |
+
+### Step 2: Configure System.Text.Json — start strict, loosen only what you need
+
+> **Security-first approach:** System.Text.Json's stricter defaults are intentional
+> security boundaries. Start from the strictest configuration and only loosen individual
+> settings when your application specifically requires it. Interview the user about each
+> compatibility requirement rather than applying a blanket compatibility configuration.
+
+```csharp
+// In Program.cs (ASP.NET Core) — configure globally
+builder.Services.ConfigureHttpJsonOptions(options =>
+{
+    ConfigureJsonOptions(options.SerializerOptions);
+});
+
+// Also configure for controllers if using MVC
+builder.Services.AddControllers()
+    .AddJsonOptions(options =>
+    {
+        ConfigureJsonOptions(options.JsonSerializerOptions);
+    });
+
+static void ConfigureJsonOptions(JsonSerializerOptions options)
+{
+    // Start from strict defaults. Only add the settings below that your
+    // application actually needs after reviewing the trade-offs.
+
+    // ── Case sensitivity ──
+    // Newtonsoft is case-insensitive by default; STJ is case-sensitive.
+    // ⚠️ Case-insensitive matching enables multiple JSON properties to map to
+    // one .NET property, which can cause interoperability/desync attacks.
+    // Only enable if your JSON producers use inconsistent casing.
+    // options.PropertyNameCaseInsensitive = true;
+
+    // ── Numbers in strings ──
+    // Newtonsoft coerces "123" to int automatically; STJ rejects by default.
+    // ⚠️ Widens the accepted input surface. Only enable if your data contains
+    // quoted numbers and you cannot fix the producer.
+    // options.NumberHandling = JsonNumberHandling.AllowReadingFromString;
+
+    // ── Comments ──
+    // Newtonsoft allows comments; STJ rejects by default.
+    // ⚠️ SECURITY: Allowing comments risks desynced deserialization attacks.
+    // Different parsers disagree on where comments start/end (e.g., Newtonsoft,
+    // JSON5, and STJ have different definitions of "end of line" for single-line
+    // comments). An attacker can exploit these differences to smuggle values
+    // through what appears to be an ignorable comment. Only enable for trusted
+    // input like config files — never for user-supplied JSON.
+    // options.ReadCommentHandling = JsonCommentHandling.Skip;
+
+    // ── Trailing commas ──
+    // Newtonsoft allows; STJ rejects by default. Low risk.
+    // options.AllowTrailingCommas = true;
+
+    // ── Enum string serialization ──
+    // Replaces Newtonsoft's StringEnumConverter.
+    options.Converters.Add(new JsonStringEnumConverter(JsonNamingPolicy.CamelCase));
+
+    // ── Circular references ──
+    // Use IgnoreCycles to silently break cycles (safe default).
+    // ⚠️ Do NOT use ReferenceHandler.Preserve unless you specifically need
+    // round-trip reference identity AND the JSON comes from a trusted source.
+    // Preserve emits $id/$ref metadata that lets an adversary rewire object
+    // graph edges, potentially violating business logic.
+    options.ReferenceHandler = ReferenceHandler.IgnoreCycles;
+}
+```
+
+> **Note:** `DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull` is a common
+> Newtonsoft.Json *configuration* but is NOT the Newtonsoft default (Json.NET includes
+> nulls by default). Only add this if the existing Newtonsoft code was explicitly
+> configured with `NullValueHandling.Ignore`.
+>
+> **Note:** Both serializers default to using the property name as declared (typically
+> PascalCase). Only set `PropertyNamingPolicy = JsonNamingPolicy.CamelCase` if the
+> existing Newtonsoft code used `CamelCasePropertyNamesContractResolver` or the
+> application specifically requires camelCase output.
+
+### Step 3: Replace attribute mappings
+
+| Newtonsoft.Json Attribute | System.Text.Json Equivalent |
+|--------------------------|----------------------------|
+| `[JsonProperty("name")]` | `[JsonPropertyName("name")]` |
+| `[JsonIgnore]` | `[JsonIgnore]` (same name, different namespace!) |
+| `[JsonProperty(Required = Required.Always)]` | `[JsonRequired]` (.NET 7+) |
+| `[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]` | `[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]` |
+| `[JsonProperty(DefaultValueHandling = DefaultValueHandling.Ignore)]` | `[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]` |
+| `[JsonConverter(typeof(MyConverter))]` | `[JsonConverter(typeof(MyConverter))]` (different base class!) |
+| `[JsonConstructor]` | `[JsonConstructor]` (same name, different namespace) |
+| `[JsonExtensionData]` | `[JsonExtensionData]` — use `Dictionary<string, JsonElement>`, `IDictionary<string, object>`, or `JsonObject` (NOT `JToken`) |
+
+### Step 4: Convert custom JsonConverters
+
+Key API differences from Newtonsoft:
+- Base class: `System.Text.Json.Serialization.JsonConverter<T>`
+- Reader is `ref Utf8JsonReader` (struct by ref); Writer is `Utf8JsonWriter`
+- Methods: `Read`/`Write` (not `ReadJson`/`WriteJson`)
+- Typed write methods: `WriteStringValue`, `WriteNumberValue`, `WriteBooleanValue`
+- Typed read methods: `reader.GetInt64()`, `reader.GetString()` (not casting `reader.Value`)
+- Use `JsonSerializerOptions options` parameter (not `JsonSerializer serializer`)
+
+### Step 5: Replace JToken/JObject/JArray with JsonNode
+
+Use `JsonNode` (System.Text.Json.Nodes) for mutable DOM (replaces LINQ-to-JSON):
+- `JToken.Parse` → `JsonNode.Parse`, `JObject` → `JsonObject`, `JArray` → `JsonArray`
+- Read: `(string)node["key"]!` or `.GetValue<T>()`; Modify: `node["key"] = value`
+- Serialize: `node.ToJsonString()`
+
+For **read-only** scenarios, use `JsonDocument`/`JsonElement` (IDisposable, must clone if keeping past dispose).
+
+### Step 6: Handle polymorphic serialization
+
+⚠️ Newtonsoft's `TypeNameHandling` is a **security risk** (attacker-controlled type instantiation). System.Text.Json uses a secure-by-design approach (.NET 7+):
+- `[JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")]` on base class
+- `[JsonDerivedType(typeof(Subtype), typeDiscriminator: "name")]` for each allowed subtype
+- No arbitrary type instantiation — explicit allow-list only
+
+### Step 7: Update package references
+
+Remove `Newtonsoft.Json` and `Microsoft.AspNetCore.Mvc.NewtonsoftJson` from .csproj. System.Text.Json is in-box for .NET 6+. Replace `using Newtonsoft.Json` / `Newtonsoft.Json.Linq` with `System.Text.Json` / `System.Text.Json.Serialization` / `System.Text.Json.Nodes`.
+
+### Step 8: Write baseline serialization tests
+
+Compare NJ and STJ output for every migrated model. Serialize with both, parse results to `JsonNode`, and assert equality. Also test round-trip deserialization for edge cases (nulls, extra properties, enums).
+
+## Validation
+
+- All `[JsonProperty]` replaced with `[JsonPropertyName]`
+- Custom converters use `System.Text.Json.Serialization.JsonConverter<T>` base
+- `JObject`/`JToken` replaced with `JsonNode` (mutable) or `JsonDocument` (read-only)
+- API responses match previous format; deserialization handles edge cases
+
+## Common Pitfalls
+
+- Missing `PropertyNameCaseInsensitive = true` → deserialization silently returns defaults
+- `[JsonIgnore]` from wrong namespace → attribute silently ignored (both NJ and STJ have it)
+- `JsonDocument` not disposed → memory leak; always use `using`
+- `JsonExtensionData` with `JToken` → use `Dictionary<string, JsonElement>` or `JsonObject`
+
+## References
+
+- [System.Text.Json overview](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/overview)
+- [Migrate from Newtonsoft.Json](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/migrate-from-newtonsoft)

tests/dotnet-upgrade/migrating-newtonsoft-to-system-text-json/eval.yaml

@@ -0,0 +1,124 @@
+scenarios:
+  - name: "Migrate model with Newtonsoft.Json attributes to System.Text.Json"
+    prompt: |
+      I'm migrating our ASP.NET Core 8 project from Newtonsoft.Json to System.Text.Json. Here's a model class that uses Newtonsoft attributes and a built-in converter. Convert this to System.Text.Json:
+
+      ```csharp
+      using Newtonsoft.Json;
+      using Newtonsoft.Json.Converters;
+      using Newtonsoft.Json.Linq;
+
+      public class Order
+      {
+          [JsonProperty("order_id")]
+          public int Id { get; set; }
+
+          [JsonProperty(Required = Required.Always)]
+          public string CustomerName { get; set; }
+
+          [JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
+          public string? Notes { get; set; }
+
+          [JsonConverter(typeof(StringEnumConverter))]
+          public OrderStatus Status { get; set; }
+
+          [JsonExtensionData]
+          public Dictionary<string, JToken>? AdditionalData { get; set; }
+      }
+      ```
+
+      Also show me how to configure the JSON options globally for Newtonsoft compatibility (case insensitivity, trailing commas, number-from-string coercion). What else should I watch out for when completing this migration?
+    reject_tools:
+      - "bash"
+      - "create_file"
+      - "edit"
+    assertions:
+      - type: "output_contains"
+        value: "JsonPropertyName"
+      - type: "output_matches"
+        pattern: "(JsonIgnore.*WhenWritingNull|JsonIgnoreCondition)"
+      - type: "output_matches"
+        pattern: "(PropertyNameCaseInsensitive|CamelCase|PropertyNamingPolicy)"
+      - type: "output_matches"
+        pattern: "(JsonElement|JsonNode|JsonObject)"
+    rubric:
+      - "Replaced [JsonProperty(\"order_id\")] with [JsonPropertyName(\"order_id\")]"
+      - "Replaced NullValueHandling.Ignore with [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]"
+      - "Replaced [JsonConverter(typeof(StringEnumConverter))] with System.Text.Json equivalent (JsonStringEnumConverter)"
+      - "Changed [JsonExtensionData] Dictionary value type from JToken to a System.Text.Json type (JsonElement, JsonNode, or JsonObject)"
+      - "Configured PropertyNameCaseInsensitive = true to match Newtonsoft default case-insensitive behavior"
+      - "Warned about security trade-offs or behavioral differences when loosening System.Text.Json's strict defaults (e.g., comments enabling desync attacks, case insensitivity widening input surface)"
+      - "Mentioned removing the Newtonsoft.Json or Microsoft.AspNetCore.Mvc.NewtonsoftJson NuGet packages as a migration step"
+      - "Noted that NullValueHandling.Ignore is NOT the Newtonsoft default — DefaultIgnoreCondition.WhenWritingNull should only be applied globally if the existing code explicitly used NullValueHandling.Ignore in settings"
+    timeout: 120
+
+  - name: "Review a partially completed Newtonsoft.Json to System.Text.Json migration for correctness"
+    prompt: |
+      I migrated our .NET 8 Web API from Newtonsoft.Json to System.Text.Json over the weekend. It compiles and basic tests pass, but I want a thorough review before we ship. Can you review my migration and point out any issues, risks, or things I missed?
+
+      **Before (Newtonsoft.Json setup in Program.cs):**
+      ```csharp
+      builder.Services.AddControllers()
+          .AddNewtonsoftJson(options =>
+          {
+              options.SerializerSettings.Converters.Add(
+                  new StringEnumConverter(new CamelCaseNamingStrategy()));
+          });
+      ```
+
+      **After (my System.Text.Json migration in Program.cs):**
+      ```csharp
+      using System.Text.Json;
+      using System.Text.Json.Serialization;
+      using Newtonsoft.Json;  // keeping this — we still use JObject in one helper
+
+      builder.Services.AddControllers()
+          .AddJsonOptions(options =>
+          {
+              options.JsonSerializerOptions.DefaultIgnoreCondition =
+                  JsonIgnoreCondition.WhenWritingNull;
+              options.JsonSerializerOptions.Converters.Add(
+                  new JsonStringEnumConverter(JsonNamingPolicy.CamelCase));
+              options.JsonSerializerOptions.PropertyNameCaseInsensitive = true;
+              options.JsonSerializerOptions.ReadCommentHandling =
+                  JsonCommentHandling.Skip;
+              options.JsonSerializerOptions.AllowTrailingCommas = true;
+          });
+      ```
+
+      **Model class (after migration):**
+      ```csharp
+      using System.Text.Json.Serialization;
+
+      public class Customer
+      {
+          [JsonPropertyName("customer_id")]
+          public int Id { get; set; }
+
+          public string Name { get; set; }
+
+          [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
+          public string? PhoneNumber { get; set; }
+
+          [JsonExtensionData]
+          public Dictionary<string, JToken>? Extra { get; set; }
+      }
+      ```
+
+      Our API receives JSON from external partners. What did I get wrong or miss?
+    reject_tools:
+      - "bash"
+      - "create_file"
+      - "edit"
+    assertions:
+      - type: "output_matches"
+        pattern: "(JToken|JsonElement|JsonNode)"
+      - type: "output_matches"
+        pattern: "(NullValueHandling|DefaultIgnoreCondition|WhenWritingNull)"
+    rubric:
+      - "Identified that Dictionary<string, JToken> must be changed to a System.Text.Json type (JsonElement, JsonNode, or JsonObject) because JToken is a Newtonsoft.Json type that won't work with System.Text.Json serialization"
+      - "Flagged that DefaultIgnoreCondition = WhenWritingNull was added but the original Newtonsoft configuration did NOT set NullValueHandling.Ignore — this silently changes API behavior by dropping null values from all responses"
+      - "Warned that ReadCommentHandling = Skip is a security risk for external partner JSON — different parsers disagree on comment boundaries, enabling desync attacks where an attacker smuggles values through what looks like a comment"
+      - "Flagged that the lingering 'using Newtonsoft.Json' import is dangerous — attributes like [JsonIgnore] or [JsonConstructor] exist in both namespaces and can silently resolve to the wrong one if a model file has both usings"
+      - "Mentioned removing the Newtonsoft.Json NuGet package once the JObject helper dependency is eliminated"
+    timeout: 120