Tighten warning handling across Refitter builds

[christianhelle]

Summary

• enable TreatWarningsAsErrors in Refitter.Core and Refitter.Tests to tighten warning enforcement in the main library and test project
• remove active warning sources across the branch, including the obsolete OpenApiDocument clone path in OpenApiDocumentFactory, README package metadata drift, and a couple of nullable-flow warnings in core/tests
• carry forward the already-committed .squad decision/history sync that is part of build-warnings

Validation

• dotnet build -c Release src\Refitter.slnx
• dotnet test -c Release src\Refitter.slnx
• dotnet format --verify-no-changes src\Refitter.slnx

Summary by CodeRabbit

• New Features

  • Enhanced validation for OpenAPI path settings to surface invalid configuration earlier.

• Bug Fixes

  • Improved cloning and merging to preserve document version markers and key collections across mixed-version merges.

• Tests

  • Added focused clone/merge tests and input-validation tests; added guidance for running specific test classes via the compiled test runner.

• Chores

  • Build stricter: compiler warnings now fail builds; updated packaging settings.

• Documentation

  • Added history, decision, and PR/process guidance entries.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12e22376-ac19-4161-8782-f09358133303

📥 Commits

Reviewing files that changed from the base of the PR and between 671aafa and 9c2fd85.

📒 Files selected for processing (1)

• src/Refitter.Tests/Issue1057SettingsCliRegressionTests.cs

---

📝 Walkthrough

Walkthrough

Replaces obsolete OpenApiDocument.ToJson(SchemaType) cloning with ToJson() + OpenApiDocument.FromJsonAsync(...) round-trip, adds input validation for OpenApiPath(s), enables TreatWarningsAsErrors across projects, updates/refines MSBuild task internals, and adds tests and documentation verifying schema-type preservation and merge behavior.

Changes

Cohort / File(s)	Summary
Squad History & Decisions ​.squad/agents/ash/history.md, ​.squad/agents/lambert/history.md, ​.squad/agents/parker/history.md, ​.squad/agents/ripley/history.md, ​.squad/decisions.md	Adds 2026-04-29 entries documenting the serialization API replacement, coverage and merge decisions, PR scope/coordination notes, and process guidance.
Squad Skills ​.squad/skills/pr-scope-framing/SKILL.md, ​.squad/skills/tunit-test-filtering/SKILL.md	Adds PR-scope framing guidance and a TUnit example for running OpenApiDocumentFactoryMergeTests via the compiled test binary.
Core Implementation src/Refitter.Core/OpenApiDocumentFactory.cs	Replaces direct ToJson(SchemaType) cloning with a CloneDocument helper that uses document.ToJson() and OpenApiDocument.FromJsonAsync(...) for cloning; preserves merge/fail-fast semantics.
Input Validation src/Refitter.Core/RefitGenerator.cs	Adds guard: when OpenApiPaths is absent/empty, OpenApiPath must be non-null/non-whitespace; throws ArgumentException for invalid input.
Build Configuration src/Refitter.Core/Refitter.Core.csproj, src/Refitter.Tests/Refitter.Tests.csproj, src/Refitter/Refitter.csproj	Enables TreatWarningsAsErrors=true in core, tests, and main projects; removes README packing entry from Refitter.csproj.
Tests src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs, src/Refitter.Tests/GenerateCommandTests.cs, src/Refitter.Tests/Issue1057SettingsCliRegressionTests.cs	Adds round-trip and cross-version merge tests asserting SchemaType and collection preservation; adds validation tests for OpenApiPath(s); adjusts nullability in an existing test.
MSBuild Task & Implementation Cleanup src/Refitter.MSBuild/RefitterGenerateTask.cs	Refactors internals: simplifies AsyncLocal usage, converts ProcessExecutionResult to primary-constructor style, materializes GeneratedFiles to array, tightens nullability, modernizes process/timeout handling and related code style.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• Handle equivalent duplicate schemas in multi-spec merge #1076 — Related changes to OpenApiDocumentFactory cloning/merge logic and tests.
• [v2.0 audit] Close remaining verified #1057 regressions #1070 — Similar migration from ToJson(SchemaType) to ToJson() + reparse and associated merge semantics.
• Migrate from Microsoft.OpenApi.Readers 1.x to Microsoft.OpenApi 3.x #907 — Prior work replacing synchronous/obsolete serialization overloads with async FromJson paths in OpenApiDocumentFactory.

Suggested labels

enhancement, .NET, bugfix

Poem

🐇 I nibbled at old calls and found a new way,

Round-tripped the schemas so none go astray.
Tests snugly tucked, warnings set to be stern,
Merges keep order — the petals won't turn.
Hop, patch, and ship — a tidy little day.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'Tighten warning handling across Refitter builds' accurately summarizes the primary objective of the changeset, which is to enable TreatWarningsAsErrors across multiple project files and resolve warning sources.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch build-warnings

---

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

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

[Copilot]

Pull request overview

Tightens build warning enforcement by enabling warnings-as-errors and removing known warning sources (including obsolete NSwag clone usage) while adding targeted regression tests around OpenApiDocument cloning/merge behavior.

Changes:

• Enable TreatWarningsAsErrors in Refitter.Core, Refitter.Tests, and the CLI project.
• Replace obsolete OpenApiDocument.ToJson(SchemaType) clone path with a ToJson() + FromJsonAsync(...) round-trip and add focused merge/clone regression tests.
• Update squad decision/history docs and TUnit filtering skill notes.

Reviewed changes

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

Show a summary per file

File	Description
src/Refitter/Refitter.csproj	Enables warnings-as-errors for the CLI/tool project; removes a redundant README pack item group.
src/Refitter.Tests/Refitter.Tests.csproj	Enables warnings-as-errors for the test project.
src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs	Adds clone/round-trip and mixed-version merge tests validating schema type + key collection preservation.
src/Refitter.Tests/GenerateCommandTests.cs	Adjusts a test assignment to avoid nullable warnings under stricter settings.
src/Refitter.Core/Refitter.Core.csproj	Enables warnings-as-errors for the core library.
src/Refitter.Core/RefitGenerator.cs	Uses null-forgiving for OpenApiPath when selecting the single-path document load route.
src/Refitter.Core/OpenApiDocumentFactory.cs	Introduces CloneDocument using parameterless ToJson(); removes obsolete clone call site.
.squad/skills/tunit-test-filtering/SKILL.md	Documents a TUnit treenode filter for the merge test class.
.squad/decisions.md	Records decisions/directives and rationale related to warning tightening and clone-path changes.
.squad/agents/parker/history.md	Syncs agent history with the obsolete clone-path fix/validation.
.squad/agents/lambert/history.md	Syncs agent history with coverage + clone validation notes.
.squad/agents/ash/history.md	Syncs agent history with clone-path guidance/approval and test filter command.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.88%. Comparing base (cf8b0e4) to head (9c2fd85).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1079      +/-   ##
==========================================
+ Coverage   95.75%   95.88%   +0.12%     
==========================================
  Files          27       27              
  Lines        2570     2578       +8     
==========================================
+ Hits         2461     2472      +11     
+ Misses         39       36       -3     
  Partials       70       70              

Flag	Coverage Δ
unittests	95.88% <100.00%> (+0.12%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

🧹 Nitpick comments (1)

src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs (1)

1513-1615: Optional: parameterize the two base-schema merge tests to reduce duplication.

Both tests follow the same structure and differ mainly in base/incoming schema types and expected marker.

♻️ Possible refactor sketch

- [Test]
- public async Task Merge_Preserves_OpenApi3_Base_SchemaType_For_Cloned_Document() { ... }
-
- [Test]
- public async Task Merge_Preserves_Swagger2_Base_SchemaType_For_Cloned_Document() { ... }
+ [Test]
+ [Arguments(SchemaType.OpenApi3, "openapi", "3.0.0")]
+ [Arguments(SchemaType.Swagger2, "swagger", "2.0")]
+ public async Task Merge_Preserves_Base_SchemaType_For_Cloned_Document(
+     SchemaType expectedSchemaType,
+     string versionField,
+     string expectedVersion)
+ {
+     // Arrange base/incoming docs for expectedSchemaType
+     // Act merge
+     // Assert SchemaType + top-level discriminator
+ }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs` around lines 1513 -
1615, The two tests Merge_Preserves_OpenApi3_Base_SchemaType_For_Cloned_Document
and Merge_Preserves_Swagger2_Base_SchemaType_For_Cloned_Document are duplicate
in structure and should be combined into a single parameterized test; create a
new test (e.g., Merge_Preserves_Base_SchemaType_For_Cloned_Document) that takes
parameters for baseJson, incomingJson, expected SchemaType, and expected marker
key/value, and drive it with [TestCase] or a [TestCaseSource] providing the two
scenarios; inside the test call ParseJsonDocument for both inputs, InvokeMerge,
and assert merged.SchemaType and
JObject.Parse(merged.ToJson())[markerKey].Value<string()>() == markerValue just
as currently done but using the parameters.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs`:
- Around line 1513-1615: The two tests
Merge_Preserves_OpenApi3_Base_SchemaType_For_Cloned_Document and
Merge_Preserves_Swagger2_Base_SchemaType_For_Cloned_Document are duplicate in
structure and should be combined into a single parameterized test; create a new
test (e.g., Merge_Preserves_Base_SchemaType_For_Cloned_Document) that takes
parameters for baseJson, incomingJson, expected SchemaType, and expected marker
key/value, and drive it with [TestCase] or a [TestCaseSource] providing the two
scenarios; inside the test call ParseJsonDocument for both inputs, InvokeMerge,
and assert merged.SchemaType and
JObject.Parse(merged.ToJson())[markerKey].Value<string()>() == markerValue just
as currently done but using the parameters.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29695fbb-ce3a-4709-ab88-700125686bf0

📥 Commits

Reviewing files that changed from the base of the PR and between bdf9b22 and 130aaf0.

📒 Files selected for processing (14)

• .squad/agents/ash/history.md
• .squad/agents/lambert/history.md
• .squad/agents/parker/history.md
• .squad/agents/ripley/history.md
• .squad/decisions.md
• .squad/skills/pr-scope-framing/SKILL.md
• .squad/skills/tunit-test-filtering/SKILL.md
• src/Refitter.Core/OpenApiDocumentFactory.cs
• src/Refitter.Core/RefitGenerator.cs
• src/Refitter.Core/Refitter.Core.csproj
• src/Refitter.Tests/GenerateCommandTests.cs
• src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs
• src/Refitter.Tests/Refitter.Tests.csproj
• src/Refitter/Refitter.csproj

[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