Skip to content

Allow input and setting files on command line#848

Closed
lilinus wants to merge 3 commits into
christianhelle:mainfrom
lilinus:oas-and-setting-files-fix
Closed

Allow input and setting files on command line#848
lilinus wants to merge 3 commits into
christianhelle:mainfrom
lilinus:oas-and-setting-files-fix

Conversation

@lilinus

@lilinus lilinus commented Dec 18, 2025

Copy link
Copy Markdown

Fixes #847

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation to consistently detect missing inputs and return a clear "no input" error.
    • Detect and report conflicting input sources early with a concise "two inputs" error to prevent ambiguous configurations.
    • Clarified and centralized error messages so input-related issues are handled predictably and in the correct order.
    • Ensured conflict checks run before path validation so the chosen input path is propagated correctly.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Reworks input validation in SettingsValidator.cs: adds a unified no-input/no-settings check, a distinct two-input check, renames helper error methods, changes the order of OpenApiPath propagation, and ensures the two-input conflict is evaluated before OpenApiPath validation. No public signatures changed.

Changes

Cohort / File(s) Summary
Validation logic
src/Refitter/SettingsValidator.cs
Consolidated empty/input checks into GetValidationErrorForNoInputNoSettings; introduced GetValidationErrorForTwoInputFiles (renamed from previous helper) with updated messaging; removed direct assignment in ValidateFilePath; added guard in ValidateFileAndOutputSettings to detect and return the two-input error when both OpenApiPath sources exist; assign settings.OpenApiPath = refitGeneratorSettings.OpenApiPath only after the two-input check.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify CLI-provided OpenApiPath combined with a settings file that lacks openApiPath is accepted (issue #847).
  • Confirm the two-input guard triggers only when both sources provide non-empty OpenApiPath values.
  • Review ordering of settings.OpenApiPath assignment versus validation to ensure no unintended propagation.
  • Check updated helper names/messages and replace any remaining references to old helper names.

Poem

🐇 I hopped through checks both wide and narrow,
I nudged the paths and caught the sparrow,
One "none" declared, one clash well-spotted,
I moved the assign and left no dotted,
I nibble code carrots, cheerful and thorough. 🥕

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 describes the main change: allowing both input and setting files to be specified on the command line, which is the core objective of fixing issue #847.
Linked Issues check ✅ Passed The code changes directly address issue #847 by allowing input file and settings file to be specified together and improving error messaging to remove the confusing message.
Out of Scope Changes check ✅ Passed All changes in SettingsValidator.cs are directly related to fixing the validation logic for handling combined input and settings files, with no out-of-scope modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a7e82c and c21a44b.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (2)
src/Refitter/SettingsValidator.cs (2)

10-14: LGTM: No-input validation is correct.

The check correctly ensures at least one input source (CLI or settings file) is provided before proceeding.


37-43: Assignment moved to ValidateFileAndOutputSettings - verify downstream logic.

The removal of settings.OpenApiPath assignment here is intentional, as it's been moved to line 55. Ensure the downstream validation logic handles all scenarios correctly.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (3)
src/Refitter/SettingsValidator.cs (3)

33-33: Critical: Unconditional assignment destroys CLI-provided OpenAPI path.

This line unconditionally overwrites settings.OpenApiPath with the value from the settings file, even if the user provided an OpenAPI file via CLI. This breaks the primary use case from issue #847 where a user provides ./openapi.json on the command line and a settings file for other configuration.

Example failure:

refitter ./openapi.json --settings-file ./openapi.refitter --output ./GeneratedCode.cs

If openapi.refitter doesn't define openApiPath, line 33 sets settings.OpenApiPath to null/empty, and line 48-51 returns "openApiPath in settings file is required"—even though the user provided it via CLI.

🔎 Apply this diff to preserve CLI-provided values:
-        settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+        if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
+        {
+            settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+        }

21-27: Minor: Fix typo in error message.

Line 25 contains a grammatical error: "or use specify" should be "or specify".

🔎 Apply this diff to fix the typo:
     return ValidationResult.Error(
         "You should either specify an input URL/file directly " +
-        "or use specify it in 'openApiPath' from the settings file, " +
+        "or specify it in 'openApiPath' from the settings file, " +
         "not both");

48-51: Fix the logic for handling CLI and settings file OpenApiPath precedence.

Line 33 unconditionally overwrites settings.OpenApiPath with the settings file value, destroying CLI arguments. When a user provides a CLI argument and the settings file lacks openApiPath, line 33 wipes out the CLI value, causing the check at lines 48-50 to incorrectly report that no path was specified in the CLI even though one was provided.

The validation at lines 42-45 cannot detect the double-specification scenario because settings.OpenApiPath has already been destroyed. Preserve the CLI value if provided, using it only as a fallback when the settings file path is absent.

🧹 Nitpick comments (1)
src/Refitter/SettingsValidator.cs (1)

8-19: Verify validation logic with comprehensive tests.

The changes to validation logic require thorough testing across multiple input scenarios:

  1. CLI OpenApiPath only
  2. Settings file with openApiPath only
  3. CLI OpenApiPath + settings file without openApiPath (issue #847 use case)
  4. CLI OpenApiPath + settings file with openApiPath (should error)
  5. Neither provided (should error)

Based on learnings, CLI changes should be tested with real OpenAPI specifications to ensure generation works correctly.

Do you want me to generate comprehensive unit tests covering all these validation scenarios? I can create test cases that verify the fix for issue #847 and prevent regression.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cf9119 and 7be1046.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (1)
src/Refitter/SettingsValidator.cs (1)

10-14: Approve the empty-inputs check, but note incomplete validation.

The check correctly handles the case where neither CLI nor settings file provides an input source. However, the validation doesn't prevent the problematic scenario where the user provides OpenApiPath via CLI AND the settings file also contains openApiPath—this should ideally be caught before line 33's assignment.

Comment thread src/Refitter/SettingsValidator.cs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
src/Refitter/SettingsValidator.cs (1)

49-52: Related issue: Check uses wrong source for OpenApiPath validation.

After applying the fix at line 40 (conditional assignment), this check will still incorrectly fail when CLI provides OpenApiPath but the settings file doesn't. The check should validate the effective/merged settings.OpenApiPath rather than refitGeneratorSettings.OpenApiPath.

🔎 Apply this diff to check the effective path:
-    if (string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
+    if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
     {
         return GetValidationErrorForOpenApiPath();
     }
♻️ Duplicate comments (1)
src/Refitter/SettingsValidator.cs (1)

40-40: Critical: Unconditional assignment overwrites CLI-provided OpenApiPath.

When a user provides an input file on the CLI and a settings file that does NOT contain openApiPath, this line unconditionally overwrites the CLI value with null/empty from the settings file. This causes the validation at line 49 to fail with an incorrect error.

Example failure:

refitter ./openapi.json --settings-file ./settings.refitter
# If settings.refitter has no openApiPath → CLI value is lost → error

The assignment should only occur when the CLI didn't provide a path.

🔎 Apply this diff to preserve CLI-provided OpenApiPath:
-    settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+    if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
+    {
+        settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+    }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7be1046 and 9ba15f5.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (2)
src/Refitter/SettingsValidator.cs (2)

10-14: LGTM!

The guard correctly validates that at least one input source is provided (either CLI path or settings file). This is the appropriate first-level validation.


21-27: LGTM!

The renamed method GetValidationErrorForNoOrTwoInputFiles better describes its purpose. The error message clearly communicates the constraint.

@christianhelle christianhelle self-assigned this Dec 18, 2025
@christianhelle christianhelle added the enhancement New feature, bug fix, or request label Dec 18, 2025

@christianhelle christianhelle left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff!

@christianhelle

Copy link
Copy Markdown
Owner

@all-contributors please add @lilinus for code

@allcontributors

Copy link
Copy Markdown
Contributor

@christianhelle

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

@codecov

codecov Bot commented Dec 18, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.76%. Comparing base (1dda6f4) to head (057bb32).
⚠️ Report is 80 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #848   +/-   ##
=======================================
  Coverage   92.76%   92.76%           
=======================================
  Files          23       23           
  Lines        1948     1948           
=======================================
  Hits         1807     1807           
  Misses         48       48           
  Partials       93       93           
Flag Coverage Δ
unittests 92.76% <ø> (ø)

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.

@lilinus
lilinus force-pushed the oas-and-setting-files-fix branch from 9ba15f5 to d4d75ac Compare December 18, 2025 15:35
@lilinus

lilinus commented Dec 18, 2025

Copy link
Copy Markdown
Author

@christianhelle I noticed there were some errors in my code, I have pushed a fix now.

@lilinus
lilinus force-pushed the oas-and-setting-files-fix branch from d4d75ac to 65a47b9 Compare December 18, 2025 15:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
src/Refitter/SettingsValidator.cs (1)

38-55: Critical: CLI-provided OpenApiPath is overwritten before dual-input validation.

The logic flaw from the previous review remains. Line 42 unconditionally overwrites settings.OpenApiPath with the settings file value BEFORE the check at lines 51-55. This breaks the intended fix for issue #847:

Scenario: refitter ./openapi.json --settings-file ./settings.refitter

  1. settings.OpenApiPath = "./openapi.json" (from CLI)
  2. Line 42 overwrites it with refitGeneratorSettings.OpenApiPath
  3. Lines 51-55 compare identical values (both from settings file)

Result: If settings file has openApiPath → false positive error. If it doesn't → CLI path is lost and line 57-60 fails.

🔎 Apply this diff to preserve CLI value and fix the validation:
 private static ValidationResult ValidateFilePath(Settings settings)
 {
+    var cliOpenApiPath = settings.OpenApiPath;
     var json = File.ReadAllText(settings.SettingsFilePath!);
     var refitGeneratorSettings = Serializer.Deserialize<RefitGeneratorSettings>(json);
-    settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+
+    if (string.IsNullOrWhiteSpace(cliOpenApiPath))
+    {
+        settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+    }
 
-    return ValidateFileAndOutputSettings(settings, refitGeneratorSettings);
+    return ValidateFileAndOutputSettings(settings, refitGeneratorSettings, cliOpenApiPath);
 }
 
 private static ValidationResult ValidateFileAndOutputSettings(
     Settings settings,
-    RefitGeneratorSettings refitGeneratorSettings)
+    RefitGeneratorSettings refitGeneratorSettings,
+    string? cliOpenApiPath = null)
 {
-    if (!string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
+    if (!string.IsNullOrWhiteSpace(cliOpenApiPath) &&
         !string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
     {
         return GetValidationErrorForTwoInputFiles();
     }
🧹 Nitpick comments (1)
src/Refitter/SettingsValidator.cs (1)

22-29: LGTM with minor nit.

Clear error message. Line 29 introduces an unnecessary blank line before GetValidationErrorForTwoInputFiles.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ba15f5 and d4d75ac.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (1)
src/Refitter/SettingsValidator.cs (1)

10-14: LGTM!

Clean consolidation of the no-input validation into a single, clear check.

Comment thread src/Refitter/SettingsValidator.cs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (2)
src/Refitter/SettingsValidator.cs (2)

22-36: Fix grammar in error messages.

Both messages contain "or use specify it" which should be "or specify it".

🔎 Apply this diff to fix the grammar:
     private static ValidationResult GetValidationErrorForNoInputNoSettings()
     {
         return ValidationResult.Error(
             "You should specify an input URL/file directly " +
-            "or use specify it in 'openApiPath' from the settings file");
+            "or specify it in 'openApiPath' from the settings file");
     }


     private static ValidationResult GetValidationErrorForTwoInputFiles()
     {
         return ValidationResult.Error(
             "You should either specify an input URL/file directly " +
-            "or use specify it in 'openApiPath' from the settings file, " +
+            "or specify it in 'openApiPath' from the settings file, " +
             "not both");
     }

82-88: Fix grammar: same issue in output path error message.

Line 86 also contains "or use specify it".

🔎 Apply this diff:
     private static ValidationResult GetValidationErrorForOutputPath()
     {
         return ValidationResult.Error(
             "You should either specify an output path directly from --output " +
-            "or use specify it in 'outputFolder' and 'outputFilename' from the settings file, " +
+            "or specify it in 'outputFolder' and 'outputFilename' from the settings file, " +
             "not both");
     }
♻️ Duplicate comments (1)
src/Refitter/SettingsValidator.cs (1)

20-20: Critical: Extra closing brace causes compilation error.

This stray } was previously identified and should have been removed. It closes the class prematurely, causing all subsequent methods to be outside the class scope.

🔎 Apply this diff to remove the extra brace:
         return !string.IsNullOrWhiteSpace(settings.SettingsFilePath)
             ? ValidateFilePath(settings)
             : ValidateOperationNameAndUrl(settings);
     }
-    }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4d75ac and 65a47b9.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (1)
src/Refitter/SettingsValidator.cs (1)

10-14: LGTM!

The early validation correctly catches the case when no input source is provided at all.

Comment thread src/Refitter/SettingsValidator.cs Outdated
Comment on lines +50 to +61
if (!string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
!string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
{
return GetValidationErrorForTwoInputFiles();
}

if (string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
{
return GetValidationErrorForOpenApiPath();
}

settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Logic prevents CLI input file with settings file that lacks openApiPath.

When a user runs refitter ./openapi.json --settings-file ./openapi.refitter where the settings file does not contain openApiPath (using it only for other options), the current logic fails:

  1. Line 50-51: settings.OpenApiPath is non-empty, refitGeneratorSettings.OpenApiPath is empty → condition FALSE
  2. Line 56: refitGeneratorSettings.OpenApiPath is empty → returns error

This defeats the PR objective. The check at line 56 should only trigger when neither source provides the path. Additionally, line 61 unconditionally overwrites the CLI value.

🔎 Apply this diff to fix the validation logic:
     private static ValidationResult ValidateFileAndOutputSettings(
         Settings settings,
         RefitGeneratorSettings refitGeneratorSettings)
     {
         if (!string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
             !string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
         {
             return GetValidationErrorForTwoInputFiles();
         }

-        if (string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
+        if (string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
+            string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
         {
             return GetValidationErrorForOpenApiPath();
         }

-        settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+        if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
+        {
+            settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+        }

         if (!string.IsNullOrWhiteSpace(settings.OutputPath) &&
🤖 Prompt for AI Agents
In src/Refitter/SettingsValidator.cs around lines 50 to 61, the logic
incorrectly returns an error when the CLI provides OpenApiPath but the settings
file does not, and then unconditionally overwrites the CLI value; change the
second validation to only error when both sources are empty (use && instead of
the current single-source check), and only assign settings.OpenApiPath from
refitGeneratorSettings.OpenApiPath when settings.OpenApiPath is empty so the CLI
value is not overwritten.

@lilinus
lilinus force-pushed the oas-and-setting-files-fix branch from 65a47b9 to 5a7e82c Compare December 18, 2025 16:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (2)
src/Refitter/SettingsValidator.cs (2)

30-36: Fix grammatical error in user-facing message.

Line 34 contains "or use specify it" which should be "or specify it".

🔎 Apply this diff to correct the grammar:
         return ValidationResult.Error(
             "You should either specify an input URL/file directly " +
-            "or use specify it in 'openApiPath' from the settings file, " +
+            "or specify it in 'openApiPath' from the settings file, " +
             "not both");

58-61: Critical: Validation checks source instead of merged value.

After the assignment at line 56, the validation at line 58 should check settings.OpenApiPath (the merged result) rather than refitGeneratorSettings.OpenApiPath (the settings file source). This forces the settings file to always provide openApiPath even when the CLI provides it.

Combined with the issue at line 56, this prevents the PR's intended behavior. After fixing line 56 to conditional assignment, this line should validate the final merged value.

🔎 Apply this diff (after fixing line 56) to validate the merged value:
-        if (string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
+        if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
         {
             return GetValidationErrorForOpenApiPath();
         }
♻️ Duplicate comments (2)
src/Refitter/SettingsValidator.cs (2)

50-56: Critical: Unconditional assignment destroys CLI input, breaking the PR objective.

Line 56 unconditionally overwrites settings.OpenApiPath with the settings file value, which destroys any CLI-provided input. This defeats the entire purpose of PR #848.

Example failure (the exact scenario from issue #847):

refitter ./openapi.json --settings-file ./openapi.refitter

If the settings file does NOT contain openApiPath (using it only for other options):

  1. Line 50-51: Check passes because refitGeneratorSettings.OpenApiPath is empty
  2. Line 56: Overwrites settings.OpenApiPath = null → CLI value ./openapi.json is destroyed
  3. Line 58: Detects empty refitGeneratorSettings.OpenApiPath → returns error

The user cannot use CLI input with a settings file unless the settings file also specifies openApiPath, which contradicts the PR objective.

🔎 Apply this diff to preserve the CLI value:
         if (!string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
             !string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
         {
             return GetValidationErrorForTwoInputFiles();
         }
 
-        settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+        if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
+        {
+            settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+        }

20-20: Critical: Extra closing brace causes compilation failure.

Line 20 contains a stray } that prematurely closes the SettingsValidator class. All methods defined after this line (lines 22-119) would be outside the class scope, causing compilation to fail.

This was flagged in a previous review comment as addressed in commit 65a47b9, but the issue persists in the current code.

🔎 Apply this diff to remove the stray brace:
         return !string.IsNullOrWhiteSpace(settings.SettingsFilePath)
             ? ValidateFilePath(settings)
             : ValidateOperationNameAndUrl(settings);
     }
-    }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65a47b9 and 5a7e82c.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (2)
src/Refitter/SettingsValidator.cs (2)

10-14: LGTM! Clean consolidation of initial validation.

The combined check correctly ensures at least one input source is provided before proceeding to deeper validation.


38-44: LGTM! Settings file loading is correct.

The deserialization correctly loads the settings file and delegates validation to the next stage.

Comment on lines +22 to 27
private static ValidationResult GetValidationErrorForNoInputNoSettings()
{
return !string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
!string.IsNullOrWhiteSpace(settings.SettingsFilePath);
return ValidationResult.Error(
"You should specify an input URL/file directly " +
"or use specify it in 'openApiPath' from the settings file");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix grammatical error in user-facing message.

Line 25 contains "or use specify it" which should be "or specify it".

🔎 Apply this diff to correct the grammar:
         return ValidationResult.Error(
             "You should specify an input URL/file directly " +
-            "or use specify it in 'openApiPath' from the settings file");
+            "or specify it in 'openApiPath' from the settings file");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static ValidationResult GetValidationErrorForNoInputNoSettings()
{
return !string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
!string.IsNullOrWhiteSpace(settings.SettingsFilePath);
return ValidationResult.Error(
"You should specify an input URL/file directly " +
"or use specify it in 'openApiPath' from the settings file");
}
private static ValidationResult GetValidationErrorForNoInputNoSettings()
{
return ValidationResult.Error(
"You should specify an input URL/file directly " +
"or specify it in 'openApiPath' from the settings file");
}
🤖 Prompt for AI Agents
In src/Refitter/SettingsValidator.cs around lines 22 to 27, the user-facing
error message contains a grammatical mistake ("or use specify it"); update the
returned string to replace "or use specify it in 'openApiPath' from the settings
file" with "or specify it in 'openApiPath' in the settings file" (or equivalent
correct phrasing) so the message reads clearly and grammatically correct.

@christianhelle

Copy link
Copy Markdown
Owner

@lilinus latest change doesn't build. There's an extra } in SettingsValidator.cs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
src/Refitter/SettingsValidator.cs (1)

32-33: Fix grammatical error in error message.

The message contains "or use specify it" which is grammatically incorrect.

🔎 Apply this diff to correct the grammar:
-            "or use specify it in 'openApiPath' from the settings file, " +
+            "or specify it in 'openApiPath' from the settings file, " +
♻️ Duplicate comments (2)
src/Refitter/SettingsValidator.cs (2)

49-60: Critical: Unconditional assignment breaks CLI input + settings file combination.

Line 55 unconditionally overwrites the CLI-provided OpenApiPath with the value from the settings file. This defeats the PR objective of allowing refitter ./openapi.json --settings-file ./openapi.refitter when the settings file doesn't contain openApiPath.

Example failure:

refitter ./openapi.json --settings-file ./openapi.refitter

If openapi.refitter contains no openApiPath:

  1. Line 49-50: CLI has value, settings file doesn't → condition FALSE
  2. Line 55: settings.OpenApiPath = null → CLI value lost!
  3. Line 57: Now empty → ERROR

The assignment should only occur when the CLI didn't provide a value, and the validation should check if both sources are empty.

🔎 Apply this diff to fix the validation logic:
     if (!string.IsNullOrWhiteSpace(settings.OpenApiPath) &&
         !string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
     {
         return GetValidationErrorForTwoInputFiles();
     }

-    settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
-
-    if (string.IsNullOrWhiteSpace(refitGeneratorSettings.OpenApiPath))
+    if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
+    {
+        settings.OpenApiPath = refitGeneratorSettings.OpenApiPath;
+    }
+
+    if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
     {
         return GetValidationErrorForOpenApiPath();
     }

24-25: Fix grammatical error in user-facing message.

Line 25 contains "or use specify it" which should be "or specify it".

🔎 Apply this diff to correct the grammar:
-            "or use specify it in 'openApiPath' from the settings file");
+            "or specify it in 'openApiPath' from the settings file");
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a7e82c and c21a44b.

📒 Files selected for processing (1)
  • src/Refitter/SettingsValidator.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

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

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

Files:

  • src/Refitter/SettingsValidator.cs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/Settings/RefitGeneratorSettings.cs : Add corresponding property to `src/Refitter.Core/Settings/RefitGeneratorSettings.cs` for each new CLI option

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test CLI changes with real OpenAPI specifications to ensure generation works correctly

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Tests/**/*.cs : Test with both OpenAPI 2.0 and 3.0 specifications when working with OpenAPI specifications

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/GenerateCommand.cs : Update `CreateRefitGeneratorSettings()` method in `src/Refitter/GenerateCommand.cs` when adding new CLI options

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter/Settings.cs : Add new CLI options as properties in `src/Refitter/Settings.cs` with `[CommandOption("--option-name")]` attribute

Applied to files:

  • src/Refitter/SettingsValidator.cs
📚 Learning: 2025-12-04T23:11:05.599Z
Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-04T23:11:05.599Z
Learning: Applies to src/Refitter.Core/**/*.cs : Implement new CLI option logic in `src/Refitter.Core/RefitInterfaceGenerator.cs` or related generator classes

Applied to files:

  • src/Refitter/SettingsValidator.cs
🧬 Code graph analysis (1)
src/Refitter/SettingsValidator.cs (1)
src/Refitter/GenerateCommand.cs (1)
  • ValidationResult (15-25)
🔇 Additional comments (2)
src/Refitter/SettingsValidator.cs (2)

10-14: LGTM: No-input validation is correct.

The check correctly ensures at least one input source (CLI or settings file) is provided before proceeding.


37-43: Assignment moved to ValidateFileAndOutputSettings - verify downstream logic.

The removal of settings.OpenApiPath assignment here is intentional, as it's been moved to line 55. Ensure the downstream validation logic handles all scenarios correctly.

@christianhelle

Copy link
Copy Markdown
Owner

@lilinus I checked out this branch locally to try it out

Given the following .refitter file:

{
  "namespace": "Petstore"
}

and the following command

refitter ./openapi.json --settings-file .\petstore.refitter

I get the an error message: Error: The 'openApiPath' in settings file is required when URL or file path to OpenAPI Specification file is not specified in command line argument

I guess there's more work do be done to properly support this.

This sort of falls back to the original idea of either using a .refitter settings file, or using CLI arguments. Combining CLI arguments with a .refitter settings file means we need to handle merging the options available from CLI arguments with the .refitter settings file, and showing specialized error messages per conflicting option

@lilinus

lilinus commented Dec 19, 2025

Copy link
Copy Markdown
Author

Ah thanks for such quick feedback. Sorry, I did the changes in a bit of a rush yesterday.
I see there is more work to do, I can close this and produce a PR with more general functionality, if that is something you think is suitable for this tool?

Thanks for an awesome tool btw.

@sonarqubecloud

Copy link
Copy Markdown

christianhelle
christianhelle previously approved these changes Jan 13, 2026
@christianhelle
christianhelle dismissed their stale review January 13, 2026 08:40

clicked approved by accident from VS Code

@lilinus lilinus closed this Jan 23, 2026
@lilinus

lilinus commented Jan 28, 2026

Copy link
Copy Markdown
Author

I closed this becuase I couldn't find any way to distinguish between missing and default value on the refit generator settings, without duplicating alot of code.

Sorry!

@christianhelle

Copy link
Copy Markdown
Owner

I closed this becuase I couldn't find any way to distinguish between missing and default value on the refit generator settings, without duplicating alot of code.

Sorry!

No problem ❤️

I probably hit the same wall before, but just couldn't remember it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature, bug fix, or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failing CLI example with confusing error message

2 participants