Add migrating-newtonsoft-to-system-text-json skill by mrsharm · Pull Request #200 · dotnet/skills

mrsharm · 2026-03-04T16:42:33Z

Summary

Adds the migrating-newtonsoft-to-system-text-json skill for migrating from Newtonsoft.Json (Json.NET) to System.Text.Json.

Note: Reopened from #89 (accidentally closed, could not reopen due to EMU restrictions).

Skill Validation Results — migrating-newtonsoft-to-system-text-json

Skill	Test	Baseline	With Skill	Δ	Verdict
migrating-newtonsoft-to-system-text-json	Migrate model with Newtonsoft.Json attributes to System.Text.Json	4.0/5	4.3/5	+0.3	✅

Overall improvement: +10.8% (3 runs, not statistically significant)

Model: claude-opus-4.6 | Judge: claude-opus-4.6

3 Iterations.

What the Skill Teaches

Complete attribute mapping table (JsonProperty -> JsonPropertyName, NullValueHandling -> JsonIgnoreCondition, etc.)
Behavioral differences that break silently (default casing, strict parsing, case sensitivity, numbers-in-strings)
Custom converter migration (Newtonsoft JsonConverter -> System.Text.Json JsonConverter with Utf8JsonReader/Writer)
JToken/JObject -> JsonDocument/JsonElement (read-only) or JsonNode (mutable DOM)
Polymorphic serialization with [JsonDerivedType] (.NET 7+)
JsonExtensionData Dictionary value type change (JToken -> JsonElement)

Files

src/dotnet/skills/migrating-newtonsoft-to-system-text-json/SKILL.md
src/dotnet/tests/migrating-newtonsoft-to-system-text-json/eval.yaml

…vement) Teaches migration from Newtonsoft.Json to System.Text.Json: attribute mapping differences, behavioral changes (casing, strictness, null handling), custom converter conversion, JToken->JsonElement/JsonNode migration, and polymorphic serialization with JsonDerivedType. Eval results: +13.8% improvement over baseline (threshold: 10%) Includes eval.yaml with migration scenario + negative test.

…NumberHandling

…LL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ewtonsoft-to-stj-skill

…ithub.com/mrsharm/skills into musharm/migrating-newtonsoft-to-stj-skill

Copilot

Pull request overview

Adds a new .NET skill intended to guide migrations from Newtonsoft.Json (Json.NET) to System.Text.Json, plus an evaluation scenario to validate the skill’s impact.

Changes:

Added a new skill doc covering attribute mappings, behavioral differences, converter migration, and DOM migration guidance.
Added a new eval scenario for migrating a Newtonsoft-attributed model and configuring global JSON options.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.

File	Description
src/dotnet/skills/migrating-newtonsoft-to-system-text-json/SKILL.md	New skill documentation for Newtonsoft.Json → System.Text.Json migration guidance
src/dotnet/tests/migrating-newtonsoft-to-system-text-json/eval.yaml	New evaluation scenario exercising attribute/config migration guidance

Comments suppressed due to low confidence (2)

src/dotnet/skills/migrating-newtonsoft-to-system-text-json/SKILL.md:208

The .csproj snippet uses Version="*", which isn't a valid/typical PackageReference version and may confuse readers. Since the intent is "remove these references", consider omitting the Version attribute entirely or showing a realistic pinned/floating version format (e.g., 13.0.3 or 13.*) in examples elsewhere.

```xml
<!-- Remove from .csproj -->
<PackageReference Include="Newtonsoft.Json" Version="*" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="*" />

src/dotnet/tests/migrating-newtonsoft-to-system-text-json/eval.yaml:5

The prompt says the model "uses ... a custom converter", but the snippet only uses StringEnumConverter (a built-in Newtonsoft converter). Either add an actual custom JsonConverter example or reword the prompt to avoid misleading setup.

    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 custom converter. Convert this to System.Text.Json:

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-04T16:50:05Z

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

+| Using `JsonElement` after `JsonDocument` is disposed | JsonElement is invalid after dispose; clone with `element.Clone()` if needed |
+| `[JsonIgnore]` from wrong namespace | Both Newtonsoft and System.Text.Json have `[JsonIgnore]` — wrong `using` = attribute ignored |
+| Custom converter reading past the current token | System.Text.Json reader is strict — must read exactly the right tokens |
+| `JsonExtensionData` with `Dictionary<string, object>` | Must be `Dictionary<string, JsonElement>` — not `object` or `JToken` |


This repeats the earlier claim that JsonExtensionData "must" use Dictionary<string, JsonElement> and that Dictionary<string, object> is invalid. System.Text.Json supports extension data on IDictionary<string, object> as well; if the goal is to prevent accidentally keeping JToken, rephrase this pitfall to focus on avoiding Newtonsoft types and explain the tradeoffs between object and JsonElement/JsonNode.

Suggested change

| `JsonExtensionData` with `Dictionary<string, object>` | Must be `Dictionary<string, JsonElement>` — not `object` or `JToken` |

| `JsonExtensionData` containing `JToken`/Newtonsoft types | Avoid `JToken`/`JObject` in extension data; use System.Text.Json types (`Dictionary<string, JsonElement>`/`JsonNode`) or `Dictionary<string, object>` with only CLR values |

Copilot · 2026-03-04T16:50:05Z

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

+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 custom converter. Convert this to System.Text.Json:
+


This eval is being added under src/dotnet/tests/..., but the repo layout and evaluation workflow expect scenarios under tests/<plugin>/<skill>/eval.yaml and only trigger on tests/** (and plugins/**). Placing this under src/ means CI evaluation and CODEOWNERS folder validation won't run for the new scenario. Consider moving to tests/dotnet/migrating-newtonsoft-to-system-text-json/eval.yaml.

Copilot · 2026-03-04T16:50:06Z

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

+    options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; // Common Newtonsoft setting
+


DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull is not Newtonsoft.Json's default (Json.NET includes nulls unless configured). Since this method is labeled "Match Newtonsoft.Json default behavior", consider separating true defaults from optional/common settings (or adjust the wording) so the guidance isn't misleading.

Suggested change

options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; // Common Newtonsoft setting

// Additional commonly used Newtonsoft.Json-like settings (not defaults):

options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; // Common Newtonsoft setting (NullValueHandling.Ignore)

Copilot · 2026-03-04T16:50:06Z

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

+      - "Changed [JsonExtensionData] Dictionary value type from JToken to JsonElement (critical difference!)"
+      - "Configured PropertyNameCaseInsensitive = true to match Newtonsoft default case-insensitive behavior"
+      - "Configured AllowTrailingCommas = true and NumberHandling = AllowReadingFromString for Newtonsoft compatibility"
+      - "Warned about behavioral differences (default PascalCase casing in STJ vs camelCase in Newtonsoft, strict parsing)"


The rubric's casing note is inconsistent with the skill doc: it expects a warning about "PascalCase in STJ vs camelCase in Newtonsoft". If you want to test casing differences, consider aligning this with the skill's own statements and/or explicitly scoping it to ASP.NET Core defaults vs library defaults to avoid grading correct answers as wrong.

Suggested change

- "Warned about behavioral differences (default PascalCase casing in STJ vs camelCase in Newtonsoft, strict parsing)"

- "Warned about behavioral differences (for example, stricter parsing behavior in System.Text.Json compared to Newtonsoft.Json)"

Copilot · 2026-03-04T16:50:06Z

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

+      - "Configured PropertyNameCaseInsensitive = true to match Newtonsoft default case-insensitive behavior"
+      - "Configured AllowTrailingCommas = true and NumberHandling = AllowReadingFromString for Newtonsoft compatibility"
+      - "Warned about behavioral differences (default PascalCase casing in STJ vs camelCase in Newtonsoft, strict parsing)"
+    expect_tools: ["bash"]


expect_tools: ["bash"] forces the model to make at least one bash tool call or the scenario fails. This prompt is primarily a code-migration explanation and doesn't require shell usage, so this constraint may introduce unnecessary flakiness. Consider removing expect_tools (or only using it when the scenario includes a setup/fixture that actually needs bash).

Suggested change

expect_tools: ["bash"]

Copilot · 2026-03-04T16:50:07Z

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

+**System.Text.Json (.NET 7+ — type discriminators):**
+```csharp
+[JsonDerivedType(typeof(CreditCardPayment), typeDiscriminator: "credit")]
+[JsonDerivedType(typeof(BankTransferPayment), typeDiscriminator: "bank")]
+public abstract class Payment


The polymorphism section implies System.Text.Json will automatically emit a discriminator for [JsonDerivedType] types. STJ polymorphism requires explicit opt-in/configuration (e.g., JsonPolymorphic / JsonPolymorphismOptions / type-info resolver setup). Consider updating this snippet to show the required opt-in so readers don't assume the attributes alone are sufficient.

Copilot · 2026-03-04T16:50:07Z

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

+```skill
+---
+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.
+---


These files are being added under src/dotnet/..., but the repo's documented layout and CI evaluation workflows expect skills under plugins/<plugin>/skills/<skill>/SKILL.md and scenarios under tests/<plugin>/<skill>/eval.yaml. As-is, the evaluation workflow (and CODEOWNERS folder validation) won't run because it only triggers on plugins/** and tests/**. Consider moving this skill to plugins/dotnet/skills/migrating-newtonsoft-to-system-text-json/ and the eval to tests/dotnet/migrating-newtonsoft-to-system-text-json/.

Copilot · 2026-03-04T16:50:07Z

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

+static void ConfigureJsonOptions(JsonSerializerOptions options)
+{
+    // Match Newtonsoft.Json default behavior:
+    options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; // Newtonsoft default
+    options.PropertyNameCaseInsensitive = true;                 // Newtonsoft default


The doc contradicts itself on default property naming: Step 1 says both Newtonsoft.Json and System.Text.Json default to PascalCase/as-declared, but Step 2 config sets PropertyNamingPolicy = CamelCase and labels it "Newtonsoft default". Please clarify the distinction (Json.NET library defaults vs ASP.NET Core defaults vs any custom ContractResolver/NamingPolicy) and make the guidance consistent.

Copilot · 2026-03-04T16:50:07Z

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

+| `[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]` + must be `Dictionary<string, JsonElement>` (NOT `JToken`) |


[JsonExtensionData] doesn't have to be Dictionary<string, JsonElement> only—System.Text.Json also supports extension data on other shapes (e.g., IDictionary<string, object> and, in newer TFMs, JsonNode/JsonObject variants). If the intent is to recommend JsonElement as the closest replacement for JToken, consider rephrasing this as guidance rather than a hard requirement so the doc stays technically correct.

Suggested change

| `[JsonExtensionData]` | `[JsonExtensionData]` + must be `Dictionary<string, JsonElement>` (NOT `JToken`) |

| `[JsonExtensionData]` | `[JsonExtensionData]` + typically use `Dictionary<string, JsonElement>` as closest to `JToken` (other supported shapes are also valid) |

Copilot · 2026-03-04T16:50:08Z

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

+var settings = new JsonSerializerSettings
+{
+    TypeNameHandling = TypeNameHandling.Auto // SECURITY RISK!
+};


This example configures JsonSerializerSettings with TypeNameHandling = TypeNameHandling.Auto, which is a known unsafe pattern when used with untrusted JSON because it lets an attacker control the concrete .NET type to instantiate, enabling gadget-based code execution or data tampering. Even though it is labeled as a security risk, including this as compilable example code may lead readers to copy it into production; consider replacing it with a non-usable anti-pattern illustration or a safer configuration that avoids TypeNameHandling for untrusted data.

Suggested change

var settings = new JsonSerializerSettings

{

TypeNameHandling = TypeNameHandling.Auto // SECURITY RISK!

};

// ❌ Insecure pattern shown for illustration only — DO NOT COPY THIS INTO PRODUCTION CODE.

// The following configuration enables $type-based polymorphic deserialization, which is

// vulnerable when used with untrusted JSON input:

//

// var settings = new JsonSerializerSettings

// {

// TypeNameHandling = TypeNameHandling.Auto

// };

GrabYourPitchforks · 2026-03-04T18:00:12Z

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

+    options.Converters.Add(new JsonStringEnumConverter(JsonNamingPolicy.CamelCase));
+
+    // Handle circular references (replaces PreserveReferencesHandling)
+    options.ReferenceHandler = ReferenceHandler.IgnoreCycles; // or Preserve for $ref/$id


Setting Preserve significantly increases the attack surface of JSON deserialization. Most call sites do not know how to properly reason about the increased attack surface and could find themselves in a vulnerable state. This is why S.T.J does not support $ref / $id in its default configuration.

It's fine if people want to set this, but they should do it only if they truly need it and have thought through the consequences of setting it. It shouldn't be promoted as a "try it and see if it solves your problem" mechanism. (What happens in practice is that somebody will set it, find that it doesn't solve their problem, then move on to some other diagnostic step without reverting this change.)

To defend against this attack, the application should consider what happens if the adversary controls the edges in the object graph, not just the values of the nodes in the graph. For example, if you're deserializing a graph that represents a family tree, what happens if a node sets itself as its own parent? What happens if a node has two children, but both point to the same object in memory? (This last example isn't recursive; it's an example of a graph that has a many:1 relationship between edges and nodes.)

Both examples could violate business logic that the application otherwise tries to uphold. Allowing an untrusted client to control the edges in the graph represents a broader attack surface and involves more subtle reasoning compared to an untrusted client that can only control values within the graph.

GrabYourPitchforks · 2026-03-04T18:05:30Z

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

+    options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; // Newtonsoft default
+    options.PropertyNameCaseInsensitive = true;                 // Newtonsoft default
+    options.NumberHandling = JsonNumberHandling.AllowReadingFromString; // Newtonsoft coerces
+    options.ReadCommentHandling = JsonCommentHandling.Skip;     // Newtonsoft allows


Similarly, this is a potentially dangerous setting and should not be encouraged unless the application has thought through the consequences. We chose not to allow this by default in S.T.J because it could lead to desynced deserialization attacks.

In these attacks, the frontend and the backend disagree on the contents of the payload. This could be because they disagree on where comments start or end, potentially allowing sensitive values to be smuggled through what appears to be an ignorable comment. (For example, Newtonsoft.Json, JSON5, and S.T.J all have different concepts on what "end of line" means for a single-line comment.)

To defend against this attack, the application should:

ensure that all deserializers in the system are operating in strict RFC compliance mode with the most restrictive settings (e.g., disallowing duplicate property entries in the payload); or

ensure that all components involved in request handling use the same deserializer library, with the same major version and patch level, to ensure consistent payload processing; or

ensure the frontend deserializes the payload to an object graph, then reserializes that object graph to a new payload before sending it on to the backend, so the backend sees only what the frontend believed the request to contain.

ViktorHofer

@mrsharm please fix the path. These files should be under plugins/... not src

JamesNK · 2026-03-04T23:23:01Z

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

+- No `serializer` parameter — use `options` and call `JsonSerializer.Serialize/Deserialize` for nested objects
+- For polymorphic deserialization: use `JsonTypeInfo` and `[JsonDerivedType]` (.NET 7+) instead of custom type handling
+
+### Step 5: Replace JToken/JObject/JArray with JsonDocument/JsonElement


I think just always recommend JsonNode and friends. It's closer than JsonDocument/JsonElement.

JamesNK · 2026-03-04T23:24:37Z

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

+
+**System.Text.Json is NOT a drop-in replacement.** These behaviors differ by default:
+
+| Behavior | Newtonsoft.Json | System.Text.Json | Impact |


STJ also escapes non-ASCII characters by default. Should recommend using relaxed encoder

mrsharm and others added 7 commits February 23, 2026 06:52

Remove negative scenario and expand rubric with AllowTrailingCommas, …

0a64de0

…NumberHandling

Update src/dotnet/skills/migrating-newtonsoft-to-system-text-json/SKI…

a9285a1

…LL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/dotnet/skills/migrating-newtonsoft-to-system-text-json/SKI…

4a507df

…LL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/dotnet/skills/migrating-newtonsoft-to-system-text-json/SKI…

9773e1c

…LL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge remote-tracking branch 'upstream/main' into musharm/migrating-n…

c88d86e

…ewtonsoft-to-stj-skill

Merge branch 'musharm/migrating-newtonsoft-to-stj-skill' of https://g…

2790f37

…ithub.com/mrsharm/skills into musharm/migrating-newtonsoft-to-stj-skill

mrsharm requested review from dbreshears and timheuer as code owners March 4, 2026 16:42

Copilot AI review requested due to automatic review settings March 4, 2026 16:42

Copilot started reviewing on behalf of mrsharm March 4, 2026 16:43 View session

mrsharm requested a review from artl93 March 4, 2026 16:47

Copilot AI reviewed Mar 4, 2026

View reviewed changes

artl93 requested review from JamesNK, PranavSenthilnathan, eiriktsarpalis and jeffhandley March 4, 2026 16:59

GrabYourPitchforks reviewed Mar 4, 2026

View reviewed changes

ViktorHofer requested changes Mar 4, 2026

View reviewed changes

JamesNK reviewed Mar 4, 2026

View reviewed changes

	\| `JsonExtensionData` with `Dictionary<string, object>` \| Must be `Dictionary<string, JsonElement>` — not `object` or `JToken` \|
	\| `JsonExtensionData` containing `JToken`/Newtonsoft types \| Avoid `JToken`/`JObject` in extension data; use System.Text.Json types (`Dictionary<string, JsonElement>`/`JsonNode`) or `Dictionary<string, object>` with only CLR values \|

		options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; // Common Newtonsoft setting

-    options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; // Common Newtonsoft setting
+    // Additional commonly used Newtonsoft.Json-like settings (not defaults):
+    options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; // Common Newtonsoft setting (NullValueHandling.Ignore)

	- "Warned about behavioral differences (default PascalCase casing in STJ vs camelCase in Newtonsoft, strict parsing)"
	- "Warned about behavioral differences (for example, stricter parsing behavior in System.Text.Json compared to Newtonsoft.Json)"

	\| `[JsonExtensionData]` \| `[JsonExtensionData]` + must be `Dictionary<string, JsonElement>` (NOT `JToken`) \|
	\| `[JsonExtensionData]` \| `[JsonExtensionData]` + typically use `Dictionary<string, JsonElement>` as closest to `JToken` (other supported shapes are also valid) \|

-var settings = new JsonSerializerSettings
-{
-    TypeNameHandling = TypeNameHandling.Auto // SECURITY RISK!
-};
+// ❌ Insecure pattern shown for illustration only — DO NOT COPY THIS INTO PRODUCTION CODE.
+// The following configuration enables $type-based polymorphic deserialization, which is
+// vulnerable when used with untrusted JSON input:
+//
+// var settings = new JsonSerializerSettings
+// {
+//     TypeNameHandling = TypeNameHandling.Auto
+// };


		System.Text.Json is NOT a drop-in replacement. These behaviors differ by default:

		\| Behavior \| Newtonsoft.Json \| System.Text.Json \| Impact \|

Conversation

mrsharm commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Skill Validation Results — migrating-newtonsoft-to-system-text-json

What the Skill Teaches

Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

GrabYourPitchforks Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

GrabYourPitchforks Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

GrabYourPitchforks Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

GrabYourPitchforks Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

ViktorHofer left a comment

Choose a reason for hiding this comment

Uh oh!

JamesNK Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

JamesNK Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mrsharm commented Mar 4, 2026 •

edited

Loading