Add option to remove [JsonConverter(typeof(JsonStringEnumConverter))] from generated contracts by christianhelle · Pull Request #786 · christianhelle/refitter

christianhelle · 2025-10-18T10:04:51Z

This implements #652

The changes here introduce the --no-inline-json-converters CLI tool argument and the InlineJsonConverters settings. When InlineJsonConverters are disabled, Refitter will remove all instances of the [JsonConverter(typeof(JsonStringEnumConverter))] attribute in the NSwag generated contracts, allowing projects to use global JSON serialization configuration instead.

CLI Usage:

refitter ./openapi.json --no-inline-json-converters

.refitter settings option:

{
  "codeGeneratorSettings": {
    "inlineJsonConverters": false  // Set to false to disable JsonConverter attributes
  }
}

Summary by CodeRabbit

New Features
- Added CLI flag --no-inline-json-converters and a new inlineJsonConverters .refitter setting (defaults to true) to control generation of JsonConverter attributes for enum properties.
Documentation
- Updated CLI docs, configuration schema, and README/examples to document the new flag/setting and its behavior.
Tests
- Added tests validating generation and build behavior with inline JSON converters enabled and disabled.

…(JsonStringEnumConverter))] from generated contracts

coderabbitai · 2025-10-18T10:05:01Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a toggle to disable inlining JsonConverter attributes for enum properties via a new CLI flag --no-inline-json-converters, a .refitter setting inlineJsonConverters, conditional sanitization in the generator to remove those attributes, tests validating both behaviors, and documentation updates.

Changes

Cohort / File(s)	Summary
Documentation `README.md`, `src/Refitter/README.md`, `src/Refitter.SourceGenerator/README.md`, `docs/docfx_project/articles/cli-tool.md`, `docs/docfx_project/articles/refitter-file-format.md`	Added documentation and examples for new CLI flag `--no-inline-json-converters` and `.refitter` option `inlineJsonConverters`; updated schema and descriptions.
CLI Settings `src/Refitter/Settings.cs`	Added `NoInlineJsonConverters` boolean setting with `--no-inline-json-converters` CommandOption and default value.
CodeGenerator Settings `src/Refitter.Core/Settings/CodeGeneratorSettings.cs`	Added public `InlineJsonConverters` boolean property (default true) to control inlining of JsonConverter attributes.
Command wiring `src/Refitter/GenerateCommand.cs`	Wired `NoInlineJsonConverters` into `RefitGeneratorSettings.CodeGeneratorSettings.InlineJsonConverters` when initializing generator settings.
Generation logic `src/Refitter.Core/RefitGenerator.cs`	Added sanitization step in `Generate()` and `GenerateMultipleFiles()` to remove lines that match JsonConverter attribute annotations when `InlineJsonConverters` is false.
Tests `src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs`	New test class verifying generated code contains JsonConverter by default, omits it when disabled, and that generated code builds in both modes.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI
    participant Settings as App Settings
    participant Command as GenerateCommand
    participant CGS as CodeGeneratorSettings
    participant Gen as RefitGenerator
    participant Filter as Sanitizer

    CLI->>Settings: supply --no-inline-json-converters
    Settings->>Command: NoInlineJsonConverters
    Command->>CGS: InlineJsonConverters = !NoInlineJsonConverters
    Command->>Gen: Start generation with CGS
    Gen->>Gen: Generate()/GenerateMultipleFiles()
    alt InlineJsonConverters == false
        Gen->>Filter: sanitize contract lines
        Filter-->>Gen: return contract without JsonConverter attributes
    else InlineJsonConverters == true
        Gen-->>Gen: return original contract
    end
    Gen-->>Command: generated code

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Option to Disable JsonStringEnumConverter Attributes in Refitter Code Generation #652 — Implements the configurable suppression of [JsonConverter(typeof(JsonStringEnumConverter))] via the new settings and sanitization, which matches the issue goal.

Suggested labels

.NET

Poem

"I nibble lines of JSON, tidy and neat,
A rabbit flips a flag and skips the suite. 🐰
Enums stand plain when converters are gone,
Tests hop, docs beam — then I munch on a prawn."

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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 PR title "Add option to remove [JsonConverter(typeof(JsonStringEnumConverter))] from generated contracts" is fully related to the main change in the changeset. The title clearly summarizes the primary objective: adding a feature/option to disable the inlining of JsonConverter attributes in generated contracts. This is supported by all files in the changeset, which collectively implement this feature through a CLI flag (--no-inline-json-converters), configuration settings (inlineJsonConverters), core logic to sanitize contracts, and comprehensive tests. The title is specific and descriptive, avoiding vague terminology.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch disable-json-string-enum-converter

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2dec7f4 and 528db4d.

📒 Files selected for processing (1)

src/Refitter.Core/RefitGenerator.cs (3 hunks)

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

src/Refitter.Core/RefitGenerator.cs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: 👌 Verify build
GitHub Check: 👌 Verify build
GitHub Check: 👌 Verify build
GitHub Check: script
GitHub Check: Analyze (csharp)
GitHub Check: script

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

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

Copilot

Pull Request Overview

Adds a toggle to prevent inlining JsonConverter attributes for enums, enabling consumers to rely on global System.Text.Json configuration instead of per-property attributes.

Introduces --no-inline-json-converters CLI flag and NoInlineJsonConverters setting
Wires the flag into CodeGeneratorSettings.InlineJsonConverters
Implements post-generation stripping of [JsonConverter(typeof(JsonStringEnumConverter))] and adds tests and docs

Reviewed Changes

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

Show a summary per file

File	Description
src/Refitter/Settings.cs	Adds NoInlineJsonConverters CLI option to Settings
src/Refitter/GenerateCommand.cs	Maps CLI flag to CodeGeneratorSettings.InlineJsonConverters
src/Refitter.Core/Settings/CodeGeneratorSettings.cs	Adds InlineJsonConverters property (default true) with documentation
src/Refitter.Core/RefitGenerator.cs	Implements removal of JsonConverter attribute lines when InlineJsonConverters is false (single-file and multi-file paths)
src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs	Adds tests for presence/absence of JsonConverter attributes and successful builds
src/Refitter/README.md	Documents CLI flag and fixes option listing formatting
docs/docfx_project/articles/cli-tool.md	Documents CLI flag
src/Refitter.SourceGenerator/README.md	Documents inlineJsonConverters setting in .refitter
docs/docfx_project/articles/refitter-file-format.md	Documents inlineJsonConverters setting in .refitter
README.md	Top-level docs for CLI flag and .refitter setting

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (1)

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

179-198: Duplicate code - apply the same refactoring.

This is identical to the code at lines 112-132. Once you extract the filtering logic to a private method as suggested in the previous comment, replace this block with a call to that method.

🧹 Nitpick comments (1)

src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs (1)

73-123: Comprehensive test coverage.

The tests validate:

Code generation succeeds

Generated code compiles in both configurations

Default behavior includes JsonConverter attributes

Disabled mode removes attributes while preserving enum types and properties

The use of AssertionScope for multiple related assertions is good practice.

Optional enhancement: Consider adding a test case with multiple enum properties to verify the filter handles multiple occurrences correctly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a75732 and 2478123.

📒 Files selected for processing (10)

README.md (4 hunks)
docs/docfx_project/articles/cli-tool.md (2 hunks)
docs/docfx_project/articles/refitter-file-format.md (2 hunks)
src/Refitter.Core/RefitGenerator.cs (2 hunks)
src/Refitter.Core/Settings/CodeGeneratorSettings.cs (1 hunks)
src/Refitter.SourceGenerator/README.md (2 hunks)
src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs (1 hunks)
src/Refitter/GenerateCommand.cs (1 hunks)
src/Refitter/README.md (2 hunks)
src/Refitter/Settings.cs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

src/**/*.cs