Use NSwag's built-in System.Text.Json polymorphic serialization

[0xced]

Simplify the polymorphic serialization code now that NSwag/NJsonSchema.CodeGeneration.CSharp supports it.

Get rid of the custom templates and just tweak what's necessary in code to ease future NSwag upgrades.

Ping @ryanheath who worked on the original implementation in #462.

Summary by CodeRabbit

• New Features

  • Added a selectable polymorphic serialization style (two engines) for generated clients.

• Bug Fixes

  • Better handling of unknown derived types and unrecognized discriminators (fallback/ignore behavior).

• Refactor

  • Simplified generation pipeline and removed legacy polymorphic converter templates and custom template artifacts.

• Documentation

  • Updated docs/guidance around polymorphic attributes and planned removal criteria.

• Chores

  • Upgraded NSwag dependencies to 14.6.0.

[coderabbitai]

Walkthrough

Refactors the template factory to a single-parameter CustomTemplateFactory that post-processes only the "Class" template, removes embedded Class and polymorphic converter templates, simplifies the custom C# client generator by removing polymorphic-only code, and upgrades NSwag package references to 14.6.0.

Changes

Cohort / File(s)	Summary of changes
Generator factory & settings src/Refitter.Core/CSharpClientGeneratorFactory.cs	Add JsonPolymorphicSerializationStyle selection; set TemplateFactory = new CustomTemplateFactory(settings) and wire generator settings.
Custom template factory src/Refitter.Core/CustomTemplateFactory.cs (nested in CSharpClientGeneratorFactory)	Constructor changed to CustomTemplateFactory(CodeGeneratorSettingsBase settings) (default assemblies passed to base). GetEmbeddedLiquidTemplate delegates to base and injects UnknownDerivedTypeHandling = FallBackToBaseType and IgnoreUnrecognizedTypeDiscriminators = true only for the "Class" template.
Custom generator simplification src/Refitter.Core/CustomCSharpClientGenerator.cs	Removed usePolymorphicSerialization constructor parameter and polymorphic operation-model wiring; now calls base.CreateOperationModel.
Templates removed src/Refitter.Core/Templates/Class.liquid, src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid, src/Refitter.Core/Templates/JsonInheritanceConverter.liquid	Deleted embedded Class.liquid and polymorphic inheritance/converter templates (System.Text.Json and Newtonsoft.Json paths).
Core project updates src/Refitter.Core/Refitter.Core.csproj	Bumped NSwag.CodeGeneration.CSharp and NSwag.Core.Yaml to 14.6.0; removed EmbeddedResource entries for Templates/*.liquid.
Source generator project updates src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj	Bumped NSwag.CodeGeneration.CSharp and NSwag.Core.Yaml to 14.6.0; removed embedded template resources reference.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller as Caller
  participant Factory as CSharpClientGeneratorFactory
  participant TFactory as CustomTemplateFactory
  participant Base as Embedded Template Provider
  participant Gen as CSharpClientGenerator

  Caller->>Factory: Create generator(document, settings)
  Factory->>Factory: Determine JsonPolymorphicSerializationStyle
  Factory->>TFactory: Instantiate with settings
  Gen->>TFactory: Request template "Class"
  TFactory->>Base: Get embedded "Class" template
  Base-->>TFactory: Return base template
  TFactory->>TFactory: Inject JsonPolymorphic flags into "Class" template
  TFactory-->>Gen: Return processed template
  Gen-->>Caller: Generate client & DTOs (polymorphic converters/templates removed)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Use fully qualified type name in Class template #686 — Related changes to how JsonPolymorphic attributes and UnknownDerivedTypeHandling are emitted in the Class template.
• Revert NSwag back to v14.4.0 #734 — Related NSwag package version changes touching the same project files.

Suggested labels

enhancement, .NET, dependencies

Suggested reviewers

• christianhelle

Poem

A rabbit taps on keys so light,
Templates shrink and take to flight.
Converters gone, the class is trim,
Factory tweaks a tiny bit for him.
NSwag hops up—fourteen-six tonight 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly captures the core change of the pull request, namely replacing the existing custom polymorphic serialization implementation with NSwag’s built-in System.Text.Json support, and directly reflects the PR’s objective to simplify future upgrades without including extraneous details.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (5)

src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj (1)

33-34: Remove stale EmbeddedResource for Templates/*.liquid.
These templates appear to be removed/unused after switching to NSwag’s built-in STJ polymorphism. Keeping this block may package nothing or confuse future maintainers.

Apply:

-  <ItemGroup>
-    <EmbeddedResource Include="../Refitter.Core/Templates/*.liquid" LinkBase="Templates" />
-  </ItemGroup>

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

10-11: Prefer explicit base call for clarity and future-proofing.
Qualifying the call avoids any accidental recursion if overloads change.

-    internal CSharpOperationModel CreateOperationModel(OpenApiOperation operation) =>
-        CreateOperationModel(operation, Settings);
+    internal CSharpOperationModel CreateOperationModel(OpenApiOperation operation) =>
+        base.CreateOperationModel(operation, Settings);

src/Refitter.Core/CSharpClientGeneratorFactory.cs (3)

50-50: Avoid overriding a user-provided TemplateFactory and gate to STJ-only.

If a user supplies a TemplateFactory via settings.CodeGeneratorSettings, MapCSharpGeneratorSettings can overwrite this assignment. Also, post-processing is only needed for STJ style.

- csharpClientGeneratorSettings.CSharpGeneratorSettings.TemplateFactory = new CustomTemplateFactory(csharpClientGeneratorSettings.CSharpGeneratorSettings);
+ if (csharpClientGeneratorSettings.CSharpGeneratorSettings.JsonPolymorphicSerializationStyle
+     == CSharpJsonPolymorphicSerializationStyle.SystemTextJson
+     && csharpClientGeneratorSettings.CSharpGeneratorSettings.TemplateFactory is null)
+ {
+     csharpClientGeneratorSettings.CSharpGeneratorSettings.TemplateFactory =
+         new CustomTemplateFactory(csharpClientGeneratorSettings.CSharpGeneratorSettings);
+ }

Confirm whether MapCSharpGeneratorSettings(...) can overwrite this; if yes, consider setting the factory after the mapping step or warning when a custom factory is supplied.

---

99-101: Doc: note STJ version requirement and future-removal condition.

Consider adding that this tweak requires STJ’s polymorphism (net7+) and is temporary until NSwag exposes UnknownDerivedTypeHandling and IgnoreUnrecognizedTypeDiscriminators as settings.

---

104-108: Use array literal instead of C# 12 collection expression for wider compatibility
No <LangVersion> is set and Refitter.Core targets netstandard2.0, so builds may use C# versions < 12. Replace the C# 12 collection expression with a traditional array literal, or alternatively pin to C# 12 via <LangVersion>12.

- : base(settings, [typeof(CSharpGenerator).Assembly, typeof(CSharpGeneratorBaseSettings).Assembly])
+ : base(settings, new[] { typeof(CSharpGenerator).Assembly, typeof(CSharpGeneratorBaseSettings).Assembly })

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd541e2 and 595d319.

⛔ Files ignored due to path filters (6)

• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/Refitter.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreOptionalParameters.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithHttpResilience.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithPolly.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/UseJsonInheritanceConverter.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/UsePolymorphicSerialization.g.cs is excluded by !**/generated/**

📒 Files selected for processing (7)

• src/Refitter.Core/CSharpClientGeneratorFactory.cs (3 hunks)
• src/Refitter.Core/CustomCSharpClientGenerator.cs (1 hunks)
• src/Refitter.Core/Refitter.Core.csproj (1 hunks)
• src/Refitter.Core/Templates/Class.liquid (0 hunks)
• src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid (0 hunks)
• src/Refitter.Core/Templates/JsonInheritanceConverter.liquid (0 hunks)
• src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj (1 hunks)

💤 Files with no reviewable changes (3)

• src/Refitter.Core/Templates/Class.liquid
• src/Refitter.Core/Templates/JsonInheritanceConverter.liquid
• src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.cs: Follow existing C# coding conventions defined in .editorconfig
Use PascalCase for public members
Use camelCase for parameters and local variables
Include XML documentation for public APIs
Use meaningful variable and method names

Files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs
• src/Refitter.Core/CustomCSharpClientGenerator.cs

🧠 Learnings (2)

📚 Learning: 2025-09-17T20:15:55.708Z

Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-17T20:15:55.708Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to src/Refitter.Core/Settings/RefitGeneratorSettings.cs for new CLI options

Applied to files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs

📚 Learning: 2025-09-17T20:15:55.708Z

Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-17T20:15:55.708Z
Learning: Applies to src/Refitter.Core/RefitInterfaceGenerator.cs : Implement generation logic for new options in src/Refitter.Core/RefitInterfaceGenerator.cs (or related generator classes)

Applied to files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs
• src/Refitter.Core/CustomCSharpClientGenerator.cs

🧬 Code graph analysis (2)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

• CustomCSharpClientGenerator (7-12)

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

• CustomCSharpClientGenerator (11-61)

🔇 Additional comments (5)

src/Refitter.Core/Refitter.Core.csproj (1)

15-16: All NSwag references are now 14.6.0 in both Core and SourceGenerator csproj files. No older versions detected.

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

7-9: Confirm SDK supports C# 12 primary constructors or revert to an explicit ctor. No global.json or <LangVersion> was found, so the default SDK controls the C# version—ensure your CI uses an SDK with C# 12 support or replace the primary‐constructor syntax with a classic constructor.

src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj (1)

19-20: All NSwag package references are consistently updated to 14.6.0 across Refitter.Core and Refitter.SourceGenerator; no stale Template EmbeddedResource entries remain.

src/Refitter.Core/CSharpClientGeneratorFactory.cs (2)

54-54: LGTM on generator wiring.

Constructor usage aligns with the simplified CustomCSharpClientGenerator signature.

---

32-32: Polymorphism style gate is correct. CLI targets net8.0/net9.0, so System.Text.Json-based polymorphic attributes are available at runtime, and the project references NSwag.CodeGeneration.CSharp 14.6.0, which includes the JsonPolymorphicSerializationStyle enum. No changes needed.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Template patch is brittle to upstream formatting changes; make it resilient and idempotent.

Exact Replace(...) can silently fail if NSwag alters spacing/ordering, undermining the PR’s goal to ease future upgrades. Use a regex that:

• Detects [JsonPolymorphic(...)] regardless of whitespace/arg order,
• Adds the two arguments only if missing.

-        protected override string GetEmbeddedLiquidTemplate(string language, string template)
-        {
-            var templateText = base.GetEmbeddedLiquidTemplate(language, template);
-            return template switch
-            {
-                "Class" => templateText
-                    .Replace(
-                        "[System.Text.Json.Serialization.JsonPolymorphic(TypeDiscriminatorPropertyName = \"{{ Discriminator }}\")]",
-                        "[System.Text.Json.Serialization.JsonPolymorphic(TypeDiscriminatorPropertyName = \"{{ Discriminator }}\", UnknownDerivedTypeHandling = System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling.FallBackToBaseType, IgnoreUnrecognizedTypeDiscriminators = true)]")
-                _ => templateText,
-            };
-        }
+        protected override string GetEmbeddedLiquidTemplate(string language, string template)
+        {
+            var text = base.GetEmbeddedLiquidTemplate(language, template);
+            if (!string.Equals(template, "Class", StringComparison.Ordinal))
+                return text;
+
+            // Patch [JsonPolymorphic(...)] robustly
+            const string pattern = @"\[\s*System\.Text\.Json\.Serialization\.JsonPolymorphic\s*\((?<args>[^)]*)\)\s*\]";
+            return System.Text.RegularExpressions.Regex.Replace(text, pattern, m =>
+            {
+                var args = m.Groups["args"].Value;
+                // Idempotent: if either arg already present, do nothing
+                if (args.Contains("UnknownDerivedTypeHandling") || args.Contains("IgnoreUnrecognizedTypeDiscriminators"))
+                    return m.Value;
+
+                var insert = string.IsNullOrWhiteSpace(args) ? string.Empty : args.Trim() + ", ";
+                insert += "UnknownDerivedTypeHandling = System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling.FallBackToBaseType, IgnoreUnrecognizedTypeDiscriminators = true";
+                return $"[System.Text.Json.Serialization.JsonPolymorphic({insert})]";
+            });
+        }

Add once at the top of the file:

using System.Text.RegularExpressions;

• Confirm the enum member spelling in STJ (Fallback vs. FallBack).

---

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

50-51: Gate the template patch to when STJ polymorphism is enabled.

Avoid touching templates when UsePolymorphicSerialization == false. Set TemplateFactory only in that case.

-        csharpClientGeneratorSettings.CSharpGeneratorSettings.TemplateFactory = new CustomTemplateFactory(csharpClientGeneratorSettings.CSharpGeneratorSettings);
+        if (settings.UsePolymorphicSerialization)
+        {
+            csharpClientGeneratorSettings.CSharpGeneratorSettings.TemplateFactory =
+                new CustomTemplateFactory(csharpClientGeneratorSettings.CSharpGeneratorSettings);
+        }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 595d319 and 77adff8.

📒 Files selected for processing (7)

• src/Refitter.Core/CSharpClientGeneratorFactory.cs (3 hunks)
• src/Refitter.Core/CustomCSharpClientGenerator.cs (1 hunks)
• src/Refitter.Core/Refitter.Core.csproj (1 hunks)
• src/Refitter.Core/Templates/Class.liquid (0 hunks)
• src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid (0 hunks)
• src/Refitter.Core/Templates/JsonInheritanceConverter.liquid (0 hunks)
• src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj (1 hunks)

💤 Files with no reviewable changes (3)

• src/Refitter.Core/Templates/JsonInheritanceConverter.liquid
• src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid
• src/Refitter.Core/Templates/Class.liquid

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.cs: Follow existing C# coding conventions defined in .editorconfig
Use PascalCase for public members
Use camelCase for parameters and local variables
Include XML documentation for public APIs
Use meaningful variable and method names

Files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs
• src/Refitter.Core/CustomCSharpClientGenerator.cs

🧠 Learnings (2)

📚 Learning: 2025-09-17T20:15:55.708Z

Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-17T20:15:55.708Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to src/Refitter.Core/Settings/RefitGeneratorSettings.cs for new CLI options

Applied to files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs

📚 Learning: 2025-09-17T20:15:55.708Z

Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-17T20:15:55.708Z
Learning: Applies to src/Refitter.Core/RefitInterfaceGenerator.cs : Implement generation logic for new options in src/Refitter.Core/RefitInterfaceGenerator.cs (or related generator classes)

Applied to files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs
• src/Refitter.Core/CustomCSharpClientGenerator.cs

🧬 Code graph analysis (2)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

• CustomCSharpClientGenerator (7-12)

src/Refitter.Core/CustomCSharpClientGenerator.cs (2)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

• CustomCSharpClientGenerator (11-61)

src/Refitter/Settings.cs (1)

• Settings (7-256)

🔇 Additional comments (7)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (4)

32-32: Good: toggle built‑in STJ polymorphism via settings.

Nice use of JsonPolymorphicSerializationStyle to switch STJ/NJsonSchema based on UsePolymorphicSerialization.

Please confirm this property name/API exists in 14.6.0 (build should catch if it moved/renamed).

---

97-101: Doc comment is clear and actionable.

Keeping this as a temporary shim with a removal note is good hygiene.

---

114-122: Make the JsonPolymorphic patch resilient and idempotent (prior feedback).

Exact Replace(...) is brittle to whitespace/argument ordering changes in upstream templates. Prefer a Regex that adds the two arguments only if missing. Also keeps future NSwag upgrades safer.

-            return template switch
-            {
-                "Class" => templateText
-                    .Replace(
-                        "[System.Text.Json.Serialization.JsonPolymorphic(TypeDiscriminatorPropertyName = \"{{ Discriminator }}\")]",
-                        "[System.Text.Json.Serialization.JsonPolymorphic(TypeDiscriminatorPropertyName = \"{{ Discriminator }}\", UnknownDerivedTypeHandling = System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling.FallBackToBaseType, IgnoreUnrecognizedTypeDiscriminators = true)]")
-                _ => templateText,
-            };
+            return template == "Class"
+                ? System.Text.RegularExpressions.Regex.Replace(
+                    templateText,
+                    @"\[\s*System\.Text\.Json\.Serialization\.JsonPolymorphic\s*\((?<args>[^)]*)\)\s*\]",
+                    m => {
+                        var args = m.Groups["args"].Value;
+                        if (args.Contains("UnknownDerivedTypeHandling") || args.Contains("IgnoreUnrecognizedTypeDiscriminators"))
+                            return m.Value; // idempotent
+                        var insert = string.IsNullOrWhiteSpace(args) ? string.Empty : args.Trim() + ", ";
+                        insert += "UnknownDerivedTypeHandling = System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling.FallBackToBaseType, IgnoreUnrecognizedTypeDiscriminators = true";
+                        return $"[System.Text.Json.Serialization.JsonPolymorphic({insert})]";
+                    })
+                : templateText;

Add at file top if missing:

using System.Text.RegularExpressions;

---

107-108: Template assembly list is sufficient. NSwag’s embedded CSharp templates are covered by the current typeof(CSharpGenerator).Assembly and typeof(CSharpGeneratorBaseSettings).Assembly, and TemplateFactory is correctly set in CSharpClientGeneratorFactory.cs. No additional assemblies needed.

src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj (1)

19-20: Analyzer packaging path intact
Verified that Refitter.SourceGenerator.csproj still includes <None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" …/>, ensuring the generator DLL is packed under analyzers/dotnet/cs as before.

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

7-12: Verified no remaining 3-argument constructor calls. All instantiations of CustomCSharpClientGenerator use the updated two-parameter signature.

src/Refitter.Core/Refitter.Core.csproj (1)

15-16: Verify NSwag 14.6.0 TFM compatibility and codegen output

Ensure NSwag.CodeGeneration.CSharp 14.6.0 ships lib/netstandard2.0 assets (inspect the nupkg) and that generated DTO code (JsonPolymorphic attributes, inheritance converters, discriminators) remains unchanged for your supported OpenAPI features.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

104-108: Avoid C# 12 collection expressions
Replace the collection-expression syntax with an explicit array initializer to ensure compatibility with older compilers:

-        public CustomTemplateFactory(CodeGeneratorSettingsBase settings)
-            : base(settings, [typeof(CSharpGenerator).Assembly, typeof(CSharpGeneratorBaseSettings).Assembly])
+        public CustomTemplateFactory(CodeGeneratorSettingsBase settings)
+            : base(settings, new[] { typeof(CSharpGenerator).Assembly, typeof(CSharpGeneratorBaseSettings).Assembly })

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77adff8 and 0a21de3.

⛔ Files ignored due to path filters (5)

• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/Refitter.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreOptionalParameters.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithHttpResilience.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithPolly.g.cs is excluded by !**/generated/**
• src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/UseJsonInheritanceConverter.g.cs is excluded by !**/generated/**

📒 Files selected for processing (7)

• src/Refitter.Core/CSharpClientGeneratorFactory.cs (3 hunks)
• src/Refitter.Core/CustomCSharpClientGenerator.cs (1 hunks)
• src/Refitter.Core/Refitter.Core.csproj (1 hunks)
• src/Refitter.Core/Templates/Class.liquid (0 hunks)
• src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid (0 hunks)
• src/Refitter.Core/Templates/JsonInheritanceConverter.liquid (0 hunks)
• src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj (1 hunks)

💤 Files with no reviewable changes (3)

• src/Refitter.Core/Templates/JsonInheritanceAttribute.liquid
• src/Refitter.Core/Templates/JsonInheritanceConverter.liquid
• src/Refitter.Core/Templates/Class.liquid

🚧 Files skipped from review as they are similar to previous changes (1)

• src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.cs: Follow existing C# coding conventions defined in .editorconfig
Use PascalCase for public members
Use camelCase for parameters and local variables
Include XML documentation for public APIs
Use meaningful variable and method names

Files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs
• src/Refitter.Core/CustomCSharpClientGenerator.cs

🧠 Learnings (2)

📚 Learning: 2025-09-17T20:15:55.708Z

Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-17T20:15:55.708Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to src/Refitter.Core/Settings/RefitGeneratorSettings.cs for new CLI options

Applied to files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs

📚 Learning: 2025-09-17T20:15:55.708Z

Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-17T20:15:55.708Z
Learning: Applies to src/Refitter.Core/RefitInterfaceGenerator.cs : Implement generation logic for new options in src/Refitter.Core/RefitInterfaceGenerator.cs (or related generator classes)

Applied to files:

• src/Refitter.Core/CSharpClientGeneratorFactory.cs
• src/Refitter.Core/CustomCSharpClientGenerator.cs

🧬 Code graph analysis (2)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

• CustomCSharpClientGenerator (7-12)

src/Refitter.Core/CustomCSharpClientGenerator.cs (2)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (1)

• CustomCSharpClientGenerator (11-61)

src/Refitter/Settings.cs (1)

• Settings (7-256)

🔇 Additional comments (5)

src/Refitter.Core/CSharpClientGeneratorFactory.cs (3)

32-32: Polymorphic style toggle: LGTM.

Switching based on UsePolymorphicSerialization aligns with the PR goal. No change requested.

---

111-122: Make the JsonPolymorphic patch resilient and idempotent.

Exact Replace() is brittle to upstream whitespace/arg-order changes and can silently fail. Use a Regex and only add args if missing.

+using System.Text.RegularExpressions;
@@
-        protected override string GetEmbeddedLiquidTemplate(string language, string template)
-        {
-            var templateText = base.GetEmbeddedLiquidTemplate(language, template);
-            return template switch
-            {
-                "Class" => templateText
-                    .Replace(
-                        "[System.Text.Json.Serialization.JsonPolymorphic(TypeDiscriminatorPropertyName = \"{{ Discriminator }}\")]",
-                        "[System.Text.Json.Serialization.JsonPolymorphic(TypeDiscriminatorPropertyName = \"{{ Discriminator }}\", UnknownDerivedTypeHandling = System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling.FallBackToBaseType, IgnoreUnrecognizedTypeDiscriminators = true)]"),
-                _ => templateText,
-            };
-        }
+        protected override string GetEmbeddedLiquidTemplate(string language, string template)
+        {
+            var text = base.GetEmbeddedLiquidTemplate(language, template);
+            if (!string.Equals(template, "Class", StringComparison.Ordinal))
+                return text;
+
+            const string pattern = @"\[\s*System\.Text\.Json\.Serialization\.JsonPolymorphic\s*\((?<args>[^)]*)\)\s*\]";
+            return Regex.Replace(text, pattern, m =>
+            {
+                var args = m.Groups["args"].Value;
+                // Idempotent: if either argument already exists, return original
+                if (args.Contains("UnknownDerivedTypeHandling") || args.Contains("IgnoreUnrecognizedTypeDiscriminators"))
+                    return m.Value;
+                var insert = string.IsNullOrWhiteSpace(args) ? string.Empty : args.Trim() + ", ";
+                insert += "UnknownDerivedTypeHandling = System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling.FallBackToBaseType, IgnoreUnrecognizedTypeDiscriminators = true";
+                return $"[System.Text.Json.Serialization.JsonPolymorphic({insert})]";
+            });
+        }

Confirm:
1) Official docs for System.Text.Json.Serialization.JsonUnknownDerivedTypeHandling member spelling ("FallBackToBaseType").
2) In which System.Text.Json version was JsonPolymorphicAttribute.IgnoreUnrecognizedTypeDiscriminators introduced?
3) Does NSwag 14.6.0’s "Class" template still emit [JsonPolymorphic(TypeDiscriminatorPropertyName = "{{ Discriminator }}")] exactly, or did formatting change?

---

50-50: Verify TemplateFactory coverage

CustomTemplateFactory override is fine, but manually confirm it probes NSwag’s template assemblies (e.g., the “Class” template). If NSwag reorganizes templates, fall back to the base TemplateFactory defaults.

src/Refitter.Core/Refitter.Core.csproj (1)

15-16: NSwag 14.6.0 upgrade verified: no 14.4.x references remain across the solution.

src/Refitter.Core/CustomCSharpClientGenerator.cs (1)

7-12: Confirm no boolean parameter in constructor calls
All new CustomCSharpClientGenerator invocations pass only document and settings; the removed boolean argument is not used.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Doc comment intent is clear. Add a safety note about STJ version requirement.

Generated attributes with IgnoreUnrecognizedTypeDiscriminators may require STJ v8+. Consider documenting the minimum required STJ version to avoid consumer build breaks.

---

🌐 Web query:

Which minimum System.Text.Json package version exposes JsonPolymorphicAttribute.IgnoreUnrecognizedTypeDiscriminators?

💡 Result:

The property appears starting in System.Text.Json package version 9.0.0. [1][2]

Sources:
[1] Microsoft Docs — JsonPolymorphicAttribute.IgnoreUnrecognizedTypeDiscriminators (shows Package: System.Text.Json v9.0.0).
[2] Microsoft Docs — JsonPolymorphicAttribute class (includes the property).

---

Doc comment intent is clear. Document minimum STJ version. Generated attributes with IgnoreUnrecognizedTypeDiscriminators require System.Text.Json v9.0.0 or later; include this in the doc comment to prevent consumer build breaks. cite12

🤖 Prompt for AI Agents

In src/Refitter.Core/CSharpClientGeneratorFactory.cs around lines 99 to 101, the
XML doc comment describing why the helper class exists should also state the
minimum required System.Text.Json version; update the comment to mention that
generated attributes using IgnoreUnrecognizedTypeDiscriminators require
System.Text.Json v9.0.0 or later (or specify exact minimum supported version),
so consumers know to upgrade their STJ package to avoid build breaks.

[christianhelle]

@0xced thanks for taking the time to implement this.

The changes look good to me and the tests pass so I'm ok with this

That said, I never used this feature myself, and it was @ryanheath who drove the implementation of this feature. Are you ok with these changes @ryanheath ?

[christianhelle]

@all-contributors please add @0xced for code

[allcontributors]

@christianhelle

I've put up a pull request to add @0xced! 🎉

[ryanheath]

@0xced thanks for taking the time to implement this.

The changes look good to me and the tests pass so I'm ok with this

That said, I never used this feature myself, and it was @ryanheath who drove the implementation of this feature. Are you ok with these changes @ryanheath ?

Yes, changes looks great!
And the tests are still green 😎