Harden .refitter settings deserialization and output path handling

[christianhelle]

This fixes #998

Summary by CodeRabbit

• New Features

  • Settings files now default to generating code in a ./Generated folder when no output folder is specified; output paths are correctly rooted relative to the settings file and CLI/default behaviors were clarified.

• Tests

  • Added comprehensive tests covering settings-file defaults, output path resolution, naming preservation, and generation/compilation of produced code.

• Chores

  • Extended ignore rules for generated artifacts and enhanced build script cleanup and regression checks; added a test configuration for output-folder scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors output-path resolution and settings-default application so .refitter outputFolder/outputFilename are rooted under the settings file directory (defaulting outputFolder to ./Generated when absent) and adds tests and MSBuild test infra to validate the behavior.

Changes

Cohort / File(s)	Summary
Core CLI behavior src/Refitter/GenerateCommand.cs	Initialize settings once, apply settings-file defaults (ApplySettingsFileDefaults), default OutputFolder to "./Generated" when missing, and compute single-file output paths rooted under the settings file directory.
MSBuild task output discovery src/Refitter.MSBuild/RefitterGenerateTask.cs	Refactored single-file mode to compute an effectiveOutputFolder (defaults to "./Generated" when blank) and build a full outputPath rooted relative to the .refitter file directory.
Unit tests (GenerateCommand) src/Refitter.Tests/GenerateCommandTests.cs	Added tests invoking non-public GetOutputPath and ApplySettingsFileDefaults to assert filename fallback, default folder application, and rooted output-path behavior for single- and multi-file scenarios.
Integration/regression tests src/Refitter.Tests/Examples/SettingsFileOutputPathTests.cs	New test class (3 tests) validating settings deserialization, default application, output-path rooting, preservation of custom naming, and that generated output compiles; includes reflection helpers and temp-dir helpers.
MSBuild test harness & config test/MSBuild/build.ps1, test/MSBuild/petstore-with-outputfolder.refitter	Added .refitter test config with outputFolder/outputFilename; enhanced PowerShell script cleanup, added hard-failure checks for missing/stray generated files and content-based smoke checks for interface names.
Repo config & minor formatting .gitignore, src/Refitter.Tests/Examples/PropertyNamingPolicyTests.cs, src/Refitter.Tests/Examples/TrimUnusedSchemaAliasWithLeadingDigitPropertyTests.cs	Added ignore patterns for generated test C# artifacts; restored file-termination newlines in two test files.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant CLI as CLI / User
    participant GenerateCMD as GenerateCommand
    participant SettingsFile as .refitter File
    participant Generator as RefitGenerator
    participant FS as FileSystem

    CLI->>GenerateCMD: invoke (with/without settings file)
    alt settings file present
        GenerateCMD->>SettingsFile: read & deserialize
        GenerateCMD->>GenerateCMD: ApplySettingsFileDefaults() (default OutputFolder="./Generated", OutputFilename fallback)
        GenerateCMD->>Generator: create generator with settings
    else no settings file
        GenerateCMD->>GenerateCMD: build settings from CLI args (use CLI output path directly)
        GenerateCMD->>Generator: create generator with CLI settings
    end
    GenerateCMD->>Generator: Generate() -> generated code
    Generator->>FS: write output to computed path (rooted under settings file dir when used)
    FS-->>CLI: files created

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Fix recursive schema stack overflows #971: Modifies Refitter.MSBuild/RefitterGenerateTask.cs—related to single-file output-path and generated-file discovery changes.
• Fix MSBuild task so that the generated code is included in the compilation #745: Overlaps with GetExpectedGeneratedFiles / outputFolder defaulting behavior in the MSBuild task.
• Create .refitter settings file as part of output #859: Related refactor of GenerateCommand settings-file deserialization and defaults application.

Poem

🐰 I hopped through folders, sniffed each file,
Found settings lost and fixed their aisle,
Defaulted Generated when none was shown,
Rooted outputs back to their home,
Now interfaces spring from tidy soil 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.57% 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 title accurately summarizes the main changes: hardening deserialization and output path handling for .refitter settings, which directly addresses issue #998.
Linked Issues check	✅ Passed	The PR successfully addresses issue #998 requirements: it fixes output path resolution to respect outputFolder and filename from .refitter files, handles default output folder fallback to './Generated', and restores proper multi-file generation behavior.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing issue #998: fixing .refitter deserialization, correcting output path handling, adding comprehensive regression tests, and updating MSBuild build verification script.

_{✏️ 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 fix-issue-998

---

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.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Refitter/GenerateCommand.cs (1)

649-711: ⚠️ Potential issue | 🟠 Major

Let an explicit CLI --output win over OutputFolder.

Once ApplySettingsFileDefaults() backfills OutputFolder, both GetOutputPath overloads keep prefixing that folder even when settings.OutputPath was explicitly provided. That turns --output Client.cs into <settingsDir>/Generated/Client.cs, and in multi-file mode the explicit override is ignored entirely. Based on learnings: Map new CLI settings in CreateRefitGeneratorSettings() method in GenerateCommand.cs.

🤖 Prompt for AI Agents

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

In `@src/Refitter/GenerateCommand.cs` around lines 649 - 711, The bug is that
ApplySettingsFileDefaults backfills OutputFolder which then overrides an
explicit CLI --output; to fix, update CreateRefitGeneratorSettings() so
CLI-provided settings.OutputPath (when not null/empty and not
Settings.DefaultOutputPath) takes precedence and does NOT get shadowed by
refitGeneratorSettings.OutputFolder—i.e. when mapping CLI into
RefitGeneratorSettings, avoid setting or overwriting
refitGeneratorSettings.OutputFolder if settings.OutputPath was explicitly
provided (instead let GetOutputPath overloads use settings.OutputPath), and only
map or default OutputFolder when CLI did not specify an output path; touch
CreateRefitGeneratorSettings, ApplySettingsFileDefaults, and the two
GetOutputPath overloads to ensure this precedence is preserved.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In @.gitignore:
- Line 288: The .gitignore currently only ignores
test/Generated/GeneratedCode/*.generated.cs but not the MSBuild regression
outputs; update the .gitignore to also ignore the MSBuild-generated directories
by adding patterns that match files under test/MSBuild/Generated/ and
test/MSBuild/GeneratedOutput/ (e.g., the generated C# files like *.generated.cs
and any temporary output files) so running test/MSBuild/build.ps1 no longer
leaves generated C# files in git status.

In `@src/Refitter.Tests/GenerateCommandTests.cs`:
- Around line 151-291: The tests (e.g.,
GetOutputPath_Should_Root_Relative_To_SettingsFile_When_OutputFolder_Is_Default,
GetOutputPath_Should_Use_SettingsFileName_When_OutputFilename_Is_Missing, and
ApplySettingsFileDefaults_* cases) hardcode Windows paths and backslashes which
break on Unix; update them to build platform-independent paths by creating a
settingsFilePath using Path.Combine/Path.GetFullPath (or Path.GetTempPath +
Path.DirectorySeparatorChar) and assert expected results using
Path.Combine/Path.GetFullPath/Path.GetFileNameWithoutExtension on the values
returned by GenerateCommand.GetOutputPath and after calling
GenerateCommand.ApplySettingsFileDefaults, and compare normalized paths rather
than raw @"C:\..." strings while still referencing Settings.DefaultOutputPath,
RefitGeneratorSettings.OutputFolder/OutputFilename to preserve intent.

In `@src/Refitter/GenerateCommand.cs`:
- Around line 42-58: The code that reads and deserializes the settings file
(File.ReadAllTextAsync(settings.SettingsFilePath),
Serializer.Deserialize<RefitGeneratorSettings>(...), and
ApplySettingsFileDefaults(...)) must be moved inside the main try block so any
IO or deserialization exceptions are caught and handled by the existing
catch/Analytics.LogError flow; update the GenerateCommand method to only build
refitGeneratorSettings from CreateRefitGeneratorSettings(settings) when no
settings file is provided, but if settings.SettingsFilePath is set perform
ReadAllTextAsync, Serializer.Deserialize<RefitGeneratorSettings>, optional
override of OpenApiPath, and ApplySettingsFileDefaults inside the try that
surrounds the overall generation logic (keeping reference to
refitGeneratorSettings variable unchanged) so failures surface in the normal
error panel and logging.

In `@test/MSBuild/build.ps1`:
- Around line 57-77: The script currently only warns when the expected interface
names are missing, so change the two checks that look for "interface
ISwaggerPetstore" and "interface ISwaggerPetstoreWithFolder" to fail the build
instead: replace the Write-Host warning branches for those two tests with error
output and an exit 1 (same style as the existing missing-file check) so the
pipeline fails if naming.interfaceName is ignored.

---

Outside diff comments:
In `@src/Refitter/GenerateCommand.cs`:
- Around line 649-711: The bug is that ApplySettingsFileDefaults backfills
OutputFolder which then overrides an explicit CLI --output; to fix, update
CreateRefitGeneratorSettings() so CLI-provided settings.OutputPath (when not
null/empty and not Settings.DefaultOutputPath) takes precedence and does NOT get
shadowed by refitGeneratorSettings.OutputFolder—i.e. when mapping CLI into
RefitGeneratorSettings, avoid setting or overwriting
refitGeneratorSettings.OutputFolder if settings.OutputPath was explicitly
provided (instead let GetOutputPath overloads use settings.OutputPath), and only
map or default OutputFolder when CLI did not specify an output path; touch
CreateRefitGeneratorSettings, ApplySettingsFileDefaults, and the two
GetOutputPath overloads to ensure this precedence is preserved.

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a7bb2397-88fa-456c-b63d-d80f3d6d8a4f

📥 Commits

Reviewing files that changed from the base of the PR and between a6eecf9 and 7840b90.

📒 Files selected for processing (9)

• .gitignore
• src/Refitter.MSBuild/RefitterGenerateTask.cs
• src/Refitter.Tests/Examples/PropertyNamingPolicyTests.cs
• src/Refitter.Tests/Examples/SettingsFileOutputPathTests.cs
• src/Refitter.Tests/Examples/TrimUnusedSchemaAliasWithLeadingDigitPropertyTests.cs
• src/Refitter.Tests/GenerateCommandTests.cs
• src/Refitter/GenerateCommand.cs
• test/MSBuild/build.ps1
• test/MSBuild/petstore-with-outputfolder.refitter

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 4 file(s) based on 4 unresolved review comments.

A stacked PR containing fixes has been created.

• Stacked PR: #1001
• Files modified:

• .gitignore
• src/Refitter.Tests/GenerateCommandTests.cs
• src/Refitter/GenerateCommand.cs
• test/MSBuild/build.ps1

Time taken: 3m 26s

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.18%. Comparing base (f1b2aed) to head (a8252d8).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1000   +/-   ##
=======================================
  Coverage   93.18%   93.18%           
=======================================
  Files          27       27           
  Lines        1804     1804           
=======================================
  Hits         1681     1681           
  Misses         49       49           
  Partials       74       74           

Flag	Coverage Δ
unittests	93.18% <ø> (ø)

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.

[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

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@src/Refitter/GenerateCommand.cs`:
- Around line 696-700: When deriving the default OutputFilename in
GenerateCommand.cs, handle the special-case where
Path.GetFileNameWithoutExtension(settingsFilePath) returns an empty string for
dotfiles (e.g., ".refitter"); if refitterFileName is null/empty (or the original
filename equals ".refitter"), set refitterFileName = "Output" before assigning
refitGeneratorSettings.OutputFilename = $"{refitterFileName}.cs" so the fallback
becomes "Output.cs" instead of ".cs". Use the existing symbols settingsFilePath,
refitterFileName and refitGeneratorSettings.OutputFilename to locate and update
the logic.

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 438c10e2-2a05-4665-b3db-c109d53567e3

📥 Commits

Reviewing files that changed from the base of the PR and between 7840b90 and a8252d8.

📒 Files selected for processing (4)

• .gitignore
• src/Refitter.Tests/GenerateCommandTests.cs
• src/Refitter/GenerateCommand.cs
• test/MSBuild/build.ps1

✅ Files skipped from review due to trivial changes (1)

• src/Refitter.Tests/GenerateCommandTests.cs

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

• .gitignore

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

A stacked PR containing fixes has been created.

• Stacked PR: #1002
• Files modified:

• src/Refitter/GenerateCommand.cs

Time taken: 3m 12s