Extract RefitterRunner as shared generation workflow#1166
Merged
Conversation
…d Warning types to Refitter.Core - RefitterRunner: new class encapsulating the shared generation workflow - RunResult: result record with generated files, warnings, diagnostics, elapsed, exit code - RunnerDiagnostic: validation/error diagnostic record - IValidator: interface for OpenAPI validation (implemented by OpenApiValidatorAdapter) - Warning: moved to Refitter.Core for cross-form reuse
Tests cover: basic generation, file writer integration, validator integration, warning detection (UseIsoDateFormat, deprecated Polly), multiple files mode, error handling, validation errors, and multiple OpenAPI paths.
- GenerationOrchestrator reduced to thin CLI adapter: parses CLI settings, creates writer/validator, calls runner, formats results via IGenerationReporter - Warning record moved from CLI project to Refitter.Core - Removed FormatFileSize tests (private method deleted) - Added XML doc comments to all new public types - All 2359 tests pass
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughIntroduces ChangesRefitterRunner Orchestration Core
Sequence Diagram(s)sequenceDiagram
participant CLI as GenerationOrchestrator
participant MSB as RefitterGenerateTask
participant SG as RefitterSourceGenerator
participant Runner as RefitterRunner
participant Gen as RefitGenerator
participant Planner as OutputPlannerAdapter
participant Writer as IFileWriter
participant Val as IValidator
CLI->>Runner: RunAsync(settings, CliFileWriter, OpenApiValidatorAdapter)
MSB->>Runner: RunAsync(settings, writer, OpenApiValidatorAdapter or null)
SG->>Runner: RunAsync(settings, SourceGeneratorFileWriter, null)
Runner->>Gen: CreateAsync / GenerateAsync
Gen-->>Runner: generated output
Runner->>Planner: Plan files
Planner-->>Runner: PlannedFiles
Runner->>Writer: WriteAsync(PlannedFile)
loop per OpenAPI path
Runner->>Val: ValidateAsync(openApiPath)
Val-->>Runner: OpenApiValidationResult
end
Runner-->>CLI: RunResult
Runner-->>MSB: RunResult
Runner-->>SG: RunResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
christianhelle
changed the title
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/Refitter.Tests/RefitterRunnerTests.cs (3)
467-476: ⚡ Quick winRename
_resultto remove underscore prefix.Private fields in this repo should use camelCase without a leading underscore.
Suggested refactor
- private readonly OpenApiValidationResult _result; + private readonly OpenApiValidationResult result; ... - _result = result ?? new OpenApiValidationResult( + this.result = result ?? new OpenApiValidationResult( new OpenApiDiagnostic(), new OpenApiStats()); ... - return Task.FromResult(_result); + return Task.FromResult(result);As per coding guidelines, private fields must not be prefixed with underscore and should be camelCase.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Tests/RefitterRunnerTests.cs` around lines 467 - 476, In the MockValidator class, rename the private field _result to result to follow the repository's camelCase naming convention for private fields without underscore prefixes. Update all references to _result throughout the MockValidator class to use the new result name instead.Source: Coding guidelines
42-57: ⚡ Quick winReplace
varwith explicit local types in this test suite.The file repeatedly uses
varwhere the type is explicit and known; repo C# style here discouragesvar.As per coding guidelines,
csharp_style_var_* = false:silentmeans explicit local types are preferred overvarin C# files.Also applies to: 81-97, 115-130, 147-163
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Tests/RefitterRunnerTests.cs` around lines 42 - 57, Replace `var` keyword declarations with explicit type declarations throughout the test methods in RefitterRunnerTests.cs, specifically for variables like `workspace` (returned from CreateTempDirectory), `openApiPath` (returned from CreateOpenApiSpec), `settings` (where applicable), `runner` (RefitterRunner), and `result` (returned from RunAsync), as the repository's C# coding style guidelines prefer explicit types over var declarations. Determine the correct return types for each method call and use those explicit types in place of var.Source: Coding guidelines
16-37: ⚡ Quick winUse
SwaggerFileHelper.CreateSwaggerJsonFilefor temporary JSON fixtures.This helper is required in
src/Refitter.Testsand avoids ad-hoc fixture setup duplication.Suggested refactor
- private static string CreateOpenApiSpec(string directory, string fileName = "spec.json") - { - Directory.CreateDirectory(directory); - var path = Path.Combine(directory, fileName); - File.WriteAllText( - path, - """ - { - "openapi": "3.0.0", - "info": { "title": "Test API", "version": "1.0.0" }, - "paths": { - "/pets": { - "get": { - "operationId": "GetPets", - "responses": { "200": { "description": "ok" } } - } - } - } - } - """); - return path; - } + private static string CreateOpenApiSpec() + { + return SwaggerFileHelper.CreateSwaggerJsonFile( + """ + { + "openapi": "3.0.0", + "info": { "title": "Test API", "version": "1.0.0" }, + "paths": { + "/pets": { + "get": { + "operationId": "GetPets", + "responses": { "200": { "description": "ok" } } + } + } + } + } + """); + }As per coding guidelines, unit tests in
src/Refitter.Tests/**/*.csshould useSwaggerFileHelper.CreateSwaggerFile(contents)for YAML andCreateSwaggerJsonFilefor JSON fixtures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Tests/RefitterRunnerTests.cs` around lines 16 - 37, The CreateOpenApiSpec method is manually creating and writing a temporary JSON file instead of using the standardized test fixture helper. Replace the Directory.CreateDirectory and File.WriteAllText logic in the CreateOpenApiSpec method with a call to SwaggerFileHelper.CreateSwaggerJsonFile, passing the OpenAPI specification JSON content string to this helper method. This helper will handle the temporary file creation and path management according to project coding guidelines for test fixtures.Source: Coding guidelines
src/Refitter.SourceGenerator/RefitterSourceGenerator.cs (1)
106-112: ⚡ Quick winConsider handling all runner warnings, not just "Date Format Override".
RefitterRunner.DetectWarningsalso emits a "Deprecated Setting" warning whenusePollyis configured. This warning will be silently dropped because only"Date Format Override"is translated to a diagnostic. The CLI reports allresult.Warningsviareporter.ReportConfigurationWarnings.To maintain parity, consider translating other warning types or adding a generic warning diagnostic:
♻️ Suggested approach
foreach (var warning in result.Warnings) { if (warning.Title == "Date Format Override") { diagnostics.Add(CreateIsoDateFormatOverrideDiagnostic()); } + else + { + diagnostics.Add(new GeneratedDiagnostic( + "REFITTER006", + warning.Title, + warning.Description, + DiagnosticSeverity.Warning)); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.SourceGenerator/RefitterSourceGenerator.cs` around lines 106 - 112, The foreach loop over result.Warnings only handles the "Date Format Override" warning and silently ignores other warnings emitted by RefitterRunner.DetectWarnings such as "Deprecated Setting". To maintain parity with the CLI behavior that reports all configuration warnings via reporter.ReportConfigurationWarnings, extend the warning handling logic to either add specific diagnostic methods for other warning types (similar to CreateIsoDateFormatOverrideDiagnostic) or add a generic fallback case that creates a diagnostic for any warning title that is not specifically handled by an existing condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Refitter.Core/RefitterRunner.cs`:
- Around line 26-131: The RunAsync method exceeds SonarCloud's cognitive
complexity threshold (22 vs allowed 15) due to multiple nested conditional
blocks and branching logic. Extract the main logical sections into focused
private helper methods: create a private method to handle the code generation
logic (the GenerateMultipleFiles vs Generate branching), a private async method
to handle validation when validator is not null, a private async method to
handle file writing when writer is not null, and consider extracting the path
filtering check logic. This will reduce the complexity of the RunAsync method
itself while maintaining the same functionality and error handling behavior in
the catch block.
- Around line 52-53: The line ending normalization in RefitterRunner.cs is
converting all line endings to LF only (backslash n), but the repository's
coding guidelines require CRLF (carriage return plus line feed) for C# files.
Replace the two chained Replace calls that normalize to LF with Replace calls
that normalize to CRLF instead, ensuring the generated C# output complies with
the repository's line-ending standards.
In `@src/Refitter.Tests/RefitterRunnerTests.cs`:
- Around line 366-367: Replace the hardcoded Windows path literal assigned to
the OpenApiPath property with a cross-platform path construction method. Use
Path.Combine to build the path dynamically, or alternatively use a
guaranteed-missing temporary path that will work reliably across Windows, Linux,
and macOS runners in CI environments.
---
Nitpick comments:
In `@src/Refitter.SourceGenerator/RefitterSourceGenerator.cs`:
- Around line 106-112: The foreach loop over result.Warnings only handles the
"Date Format Override" warning and silently ignores other warnings emitted by
RefitterRunner.DetectWarnings such as "Deprecated Setting". To maintain parity
with the CLI behavior that reports all configuration warnings via
reporter.ReportConfigurationWarnings, extend the warning handling logic to
either add specific diagnostic methods for other warning types (similar to
CreateIsoDateFormatOverrideDiagnostic) or add a generic fallback case that
creates a diagnostic for any warning title that is not specifically handled by
an existing condition.
In `@src/Refitter.Tests/RefitterRunnerTests.cs`:
- Around line 467-476: In the MockValidator class, rename the private field
_result to result to follow the repository's camelCase naming convention for
private fields without underscore prefixes. Update all references to _result
throughout the MockValidator class to use the new result name instead.
- Around line 42-57: Replace `var` keyword declarations with explicit type
declarations throughout the test methods in RefitterRunnerTests.cs, specifically
for variables like `workspace` (returned from CreateTempDirectory),
`openApiPath` (returned from CreateOpenApiSpec), `settings` (where applicable),
`runner` (RefitterRunner), and `result` (returned from RunAsync), as the
repository's C# coding style guidelines prefer explicit types over var
declarations. Determine the correct return types for each method call and use
those explicit types in place of var.
- Around line 16-37: The CreateOpenApiSpec method is manually creating and
writing a temporary JSON file instead of using the standardized test fixture
helper. Replace the Directory.CreateDirectory and File.WriteAllText logic in the
CreateOpenApiSpec method with a call to SwaggerFileHelper.CreateSwaggerJsonFile,
passing the OpenAPI specification JSON content string to this helper method.
This helper will handle the temporary file creation and path management
according to project coding guidelines for test fixtures.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73df9aa6-20e3-4a62-9841-ac26c1659c8c
📒 Files selected for processing (12)
src/Refitter.Core/Abstractions/IValidator.cssrc/Refitter.Core/RefitterRunner.cssrc/Refitter.Core/RunResult.cssrc/Refitter.Core/RunnerDiagnostic.cssrc/Refitter.Core/Validation/OpenApiValidatorAdapter.cssrc/Refitter.Core/Warning.cssrc/Refitter.MSBuild/RefitterGenerateTask.cssrc/Refitter.SourceGenerator/RefitterSourceGenerator.cssrc/Refitter.Tests/GenerationOrchestratorTests.cssrc/Refitter.Tests/RefitterRunnerTests.cssrc/Refitter/GenerationOrchestrator.cssrc/Refitter/IGenerationReporter.cs
💤 Files with no reviewable changes (2)
- src/Refitter.Tests/GenerationOrchestratorTests.cs
- src/Refitter/IGenerationReporter.cs
…ion in GenerationOrchestrator)
Contributor
Merge conflicts resolved in commit c1f6dc6. The only conflict was in |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1166 +/- ##
==========================================
+ Coverage 94.78% 95.04% +0.25%
==========================================
Files 75 80 +5
Lines 3568 3553 -15
==========================================
- Hits 3382 3377 -5
+ Misses 76 68 -8
+ Partials 110 108 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
This was referenced Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts a shared \RefitterRunner\ class into \Refitter.Core\ that encapsulates the complete generation workflow, then refactors all three distribution forms (CLI, MSBuild, Source Generator) as thin adapters around it.
Changes
Core Library (\Refitter.Core)
CLI (\Refitter)
MSBuild (\Refitter.MSBuild)
Source Generator (\Refitter.SourceGenerator)
Testing
Summary by CodeRabbit