Add debug logging for source generator when searching for .refitter files #743
Conversation
WalkthroughDocumentation updates require explicit inclusion of .refitter files via MSBuild AdditionalFiles. The source generator now logs discovered .refitter paths and warns if none are found, using a two-phase incremental pipeline: discovery/logging, then code generation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant MSB as MSBuild
participant Gen as Incremental Generator
participant FS as AdditionalFiles (.refitter)
participant Log as Debug Output
MSB->>Gen: Provide AdditionalFiles inputs
Gen->>FS: Discover *.refitter files
FS-->>Gen: List of paths (sorted)
alt No .refitter files found
Gen->>Log: Warn: No .refitter files. Add via <AdditionalFiles Include="..."/>
else One or more files
loop For each discovered path
Gen->>Log: Debug.WriteLine(path)
end
Gen->>Gen: GenerateCode for each .refitter
Gen-->>MSB: Add generated implementation sources
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
|
There was a problem hiding this comment.
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.SourceGenerator/RefitterSourceGenerator.cs (3)
61-71: Avoid unconditional “success” diagnostic; only emit on success.Currently a success info is reported even when errors were raised, which is misleading.
Apply this diff:
- context.ReportDiagnostic( - Diagnostic.Create( - new DiagnosticDescriptor( - "REFITTER001", - "Refitter", - "Refitter generated code successfully", - "Refitter", - DiagnosticSeverity.Info, - true), - Location.None)); + if (!diagnostics.Any(d => d.Severity == DiagnosticSeverity.Error)) + { + context.ReportDiagnostic( + Diagnostic.Create( + new DiagnosticDescriptor( + "REFITTER012", + "Refitter", + "Refitter generation completed successfully.", + "Refitter", + DiagnosticSeverity.Info, + true), + Location.None)); + }
84-93: Don’t emit full .refitter contents; de-duplicate and fix IDs.
- Emitting the entire file JSON as a diagnostic is noisy and risks leaking sensitive paths/config.
- “REFITTER001” is reused for multiple messages; diagnostic IDs should be stable and unique per message.
Apply this diff to drop the content dump and use a distinct ID for the “found file” message (or rely solely on the earlier path logging):
- var diagnostics = new List<Diagnostic> - { - Diagnostic.Create( - new DiagnosticDescriptor( - "REFITTER001", - "Refitter", - $"Found .refitter File: {file.Path}", - "Refitter", - DiagnosticSeverity.Info, - true), - Location.None) - }; + var diagnostics = new List<Diagnostic>(); @@ - diagnostics.Add( - Diagnostic.Create( - new DiagnosticDescriptor( - "REFITTER001", - "Refitter File Contents", - json, - "Refitter", - DiagnosticSeverity.Info, - true), - Location.None)); + // Intentionally avoid dumping file contents to diagnosticsIf you still want a per-file trace, add a concise info diagnostic here with a unique ID (e.g., REFITTER013) that only prints the file name, not contents.
Also applies to: 100-110
201-221: Deserialize failure should be an error, not info.This currently labels an error as Info, which hides real failures.
Apply this diff:
- DiagnosticSeverity.Info, + DiagnosticSeverity.Error,
🧹 Nitpick comments (5)
README.md (1)
167-173: Fix code fence language and suggest wildcard include.Use xml for MSBuild snippets and consider a wildcard so multiple .refitter files are picked up without editing the project file each time.
Apply this diff:
-```markdown +```xml <ItemGroup> - <AdditionalFiles Include="Petstore.refitter" /> + <AdditionalFiles Include="**/*.refitter" /> </ItemGroup></blockquote></details> <details> <summary>src/Refitter.SourceGenerator/RefitterSourceGenerator.cs (4)</summary><blockquote> `22-25`: **Prefer OrdinalIgnoreCase for file-path matching.** Path comparisons should use OrdinalIgnoreCase to avoid culture-related bugs (e.g., Turkish-i). Apply this diff: ```diff - .Where(text => text.Path.EndsWith(".refitter", StringComparison.InvariantCultureIgnoreCase)); + .Where(text => text.Path.EndsWith(".refitter", StringComparison.OrdinalIgnoreCase));
26-31: Sort with OrdinalIgnoreCase.For deterministic, culture-agnostic sorting of paths, use StringComparer.OrdinalIgnoreCase.
Apply this diff:
- .Select((arr, _) => arr.Sort(StringComparer.InvariantCultureIgnoreCase)); + .Select((arr, _) => arr.Sort(StringComparer.OrdinalIgnoreCase));
148-153: Robust filename derivation.Replace string.Replace with Path APIs; handles case differences and avoids “.g.cs” edge case logic.
Apply this diff:
- var filename = settings.OutputFilename ?? Path.GetFileName(file.Path).Replace(".refitter", ".g.cs"); - if (filename == ".g.cs") - { - filename = "Refitter.g.cs"; - } + var baseName = Path.GetFileNameWithoutExtension(file.Path); + var filename = settings.OutputFilename ?? $"{(string.IsNullOrEmpty(baseName) ? "Refitter" : baseName)}.g.cs";
154-165: Guard against null/empty OutputFolder.Ensure a sensible default to avoid Path.Combine throwing when settings.OutputFolder is null.
Apply this diff:
- var folder = Path.Combine(Path.GetDirectoryName(file.Path)!, settings.OutputFolder); + var outputFolder = string.IsNullOrWhiteSpace(settings.OutputFolder) ? "Generated" : settings.OutputFolder; + var folder = Path.Combine(Path.GetDirectoryName(file.Path)!, outputFolder);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)src/Refitter.SourceGenerator/RefitterSourceGenerator.cs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: Follow existing C# coding conventions defined in .editorconfig
Use PascalCase for public members and camelCase for parameters and local variables
Use meaningful variable and method names
Keep methods focused with single responsibility
Files:
src/Refitter.SourceGenerator/RefitterSourceGenerator.cs
src/{Refitter,Refitter.Core,Refitter.SourceGenerator,Refitter.MSBuild}/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Include XML documentation for public APIs
Files:
src/Refitter.SourceGenerator/RefitterSourceGenerator.cs
README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update README.md when adding new CLI options or features
Files:
README.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/{Refitter,Refitter.Core,Refitter.SourceGenerator,Refitter.MSBuild}/**/*.cs : Include XML documentation for public APIs
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/{Refitter.Tests,Refitter.SourceGenerator.Tests}/**/*.cs : All new code must include unit tests following the Refitter.Tests.Examples pattern (xUnit [Fact], assertions, and compile check)
📚 Learning: 2025-08-13T18:27:12.292Z
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/{Refitter,Refitter.Core,Refitter.SourceGenerator,Refitter.MSBuild}/**/*.cs : Include XML documentation for public APIs
Applied to files:
src/Refitter.SourceGenerator/RefitterSourceGenerator.csREADME.md
📚 Learning: 2025-08-13T18:27:12.292Z
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/{Refitter.Tests,Refitter.SourceGenerator.Tests}/**/*.cs : All new code must include unit tests following the Refitter.Tests.Examples pattern (xUnit [Fact], assertions, and compile check)
Applied to files:
src/Refitter.SourceGenerator/RefitterSourceGenerator.csREADME.md
📚 Learning: 2025-08-13T18:27:12.292Z
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/Refitter.Core/RefitInterfaceGenerator.cs : When adding a new CLI option, implement the generation logic in RefitInterfaceGenerator
Applied to files:
src/Refitter.SourceGenerator/RefitterSourceGenerator.cs
🔇 Additional comments (1)
src/Refitter.SourceGenerator/RefitterSourceGenerator.cs (1)
118-125: Optional: re-check OpenApiPath existence after combining and warn.After making the path relative to the .refitter file, consider adding a warning if the file still doesn’t exist to help users catch typos early.
I can add a small diagnostic here (e.g., REFITTER014 Warning) if you confirm this is desirable.
| // add a source output that logs what we found for easier troubleshooting and setup | ||
| context.RegisterSourceOutput(refitterPathList, static (spc, paths) => | ||
| { | ||
| if (paths.Length == 0) | ||
| { | ||
| // log a warning if no .refitter files were found, instructing the user how to add them | ||
| Debug.WriteLine("[Refitter] No .refitter files found. Ensure they are added to your project as `<AdditionalFiles Include=\"Petstore.refitter\" />`"); | ||
| return; | ||
| } | ||
|
|
||
| // log each found .refitter file path | ||
| foreach (var path in paths) | ||
| { | ||
| Debug.WriteLine($"[Refitter] Found .refitter file: {path}"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Debug.WriteLine won’t surface in builds; emit diagnostics instead.
Debug output is typically invisible in VS/MSBuild. Use spc.ReportDiagnostic so users actually see findings.
Apply this diff:
- context.RegisterSourceOutput(refitterPathList, static (spc, paths) =>
+ context.RegisterSourceOutput(refitterPathList, static (spc, paths) =>
{
if (paths.Length == 0)
{
- // log a warning if no .refitter files were found, instructing the user how to add them
- Debug.WriteLine("[Refitter] No .refitter files found. Ensure they are added to your project as `<AdditionalFiles Include=\"Petstore.refitter\" />`");
+ // Emit a visible warning if no .refitter files were found
+ spc.ReportDiagnostic(
+ Diagnostic.Create(
+ new DiagnosticDescriptor(
+ "REFITTER010",
+ "No .refitter files found",
+ "No .refitter files found. Add them via <AdditionalFiles Include=\"**/*.refitter\" /> in your project file.",
+ "Refitter",
+ DiagnosticSeverity.Warning,
+ isEnabledByDefault: true),
+ Location.None));
return;
}
- // log each found .refitter file path
+ // Log each found .refitter file path
foreach (var path in paths)
{
- Debug.WriteLine($"[Refitter] Found .refitter file: {path}");
+ spc.ReportDiagnostic(
+ Diagnostic.Create(
+ new DiagnosticDescriptor(
+ "REFITTER011",
+ "Found .refitter file",
+ $"Found .refitter file: {path}",
+ "Refitter",
+ DiagnosticSeverity.Info,
+ isEnabledByDefault: true),
+ Location.None));
}
});📝 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.
| // add a source output that logs what we found for easier troubleshooting and setup | |
| context.RegisterSourceOutput(refitterPathList, static (spc, paths) => | |
| { | |
| if (paths.Length == 0) | |
| { | |
| // log a warning if no .refitter files were found, instructing the user how to add them | |
| Debug.WriteLine("[Refitter] No .refitter files found. Ensure they are added to your project as `<AdditionalFiles Include=\"Petstore.refitter\" />`"); | |
| return; | |
| } | |
| // log each found .refitter file path | |
| foreach (var path in paths) | |
| { | |
| Debug.WriteLine($"[Refitter] Found .refitter file: {path}"); | |
| } | |
| }); | |
| // add a source output that logs what we found for easier troubleshooting and setup | |
| context.RegisterSourceOutput(refitterPathList, static (spc, paths) => | |
| { | |
| if (paths.Length == 0) | |
| { | |
| // Emit a visible warning if no .refitter files were found | |
| spc.ReportDiagnostic( | |
| Diagnostic.Create( | |
| new DiagnosticDescriptor( | |
| "REFITTER010", | |
| "No .refitter files found", | |
| "No .refitter files found. Add them via <AdditionalFiles Include=\"**/*.refitter\" /> in your project file.", | |
| "Refitter", | |
| DiagnosticSeverity.Warning, | |
| isEnabledByDefault: true), | |
| Location.None)); | |
| return; | |
| } | |
| // Log each found .refitter file path | |
| foreach (var path in paths) | |
| { | |
| spc.ReportDiagnostic( | |
| Diagnostic.Create( | |
| new DiagnosticDescriptor( | |
| "REFITTER011", | |
| "Found .refitter file", | |
| $"Found .refitter file: {path}", | |
| "Refitter", | |
| DiagnosticSeverity.Info, | |
| isEnabledByDefault: true), | |
| Location.None)); | |
| } | |
| }); |
There was a problem hiding this comment.
I have had the opposite experience here
There was a problem hiding this comment.
@codymullins Thank you for clarifying! You're absolutely right - I see from the PR description that you already tried using Diagnostics but couldn't get them to appear in Visual Studio, which is why you went with Debug.WriteLine.
Your practical experience with this specific codebase and toolchain takes precedence over general assumptions. If Debug.WriteLine is working for you and providing the visibility you need in the build output, then that's the right approach for this project.
✏️ Learnings added
Learnt from: codymullins
PR: christianhelle/refitter#743
File: src/Refitter.SourceGenerator/RefitterSourceGenerator.cs:32-47
Timestamp: 2025-09-09T19:25:59.304Z
Learning: In the Refitter project, Debug.WriteLine works better than spc.ReportDiagnostic for source generator logging visibility in Visual Studio build output. The project maintainer found that diagnostics don't appear in Visual Studio, while Debug.WriteLine does surface properly.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/{Refitter,Refitter.Core,Refitter.SourceGenerator,Refitter.MSBuild}/**/*.cs : Include XML documentation for public APIs
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/{Refitter.Tests,Refitter.SourceGenerator.Tests}/**/*.cs : All new code must include unit tests following the Refitter.Tests.Examples pattern (xUnit [Fact], assertions, and compile check)
Learnt from: CR
PR: christianhelle/refitter#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-13T18:27:12.292Z
Learning: Applies to src/Refitter.Core/RefitInterfaceGenerator.cs : When adding a new CLI option, implement the generation logic in RefitInterfaceGenerator
There was a problem hiding this comment.
I'm honestly also surprised that it works to use Debug.WriteLine 🤔
I know that ReportDiagnostic works but I could only see the output from the Errors List Window
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
After trying it out myself again, I can't seem to get anything out of ReportDiagnostic in Visual Studio. This is a bit disappointing as that always used to work
There was a problem hiding this comment.
Well, if you want something even more confusing, I tried this out in Visual Studio Insiders today, and an error reaching the OpenAPI json file causes the entire build to fail, whereas it just warned me inconsistently in VS 2022.
There was a problem hiding this comment.
I know that ReportDiagnostic works but I could only see the output from the Errors List Window
For the life of me, I tried so many different things and it wouldn't show up. I am wondering if perhaps this was fixed in VS Insiders, hence why I saw the error there. I honestly had assumed it was a bug, but it may be the other way around.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #743 +/- ##
==========================================
- Coverage 98.52% 95.47% -3.06%
==========================================
Files 60 60
Lines 2918 2982 +64
==========================================
- Hits 2875 2847 -28
- Misses 4 102 +98
+ Partials 39 33 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@codymullins Thanks for taking the time to implement this ❤️ |
christianhelle
left a comment
There was a problem hiding this comment.
I think we should revert the changes to the README file
README.md
Outdated
| </ItemGroup> | ||
| ``` | ||
|
|
||
| The generator can automatically detect all `.refitter` files inside the project that referenced the `Refitter.SourceGenerator` package and there is no need to include them manually as `AdditionalFiles` |
There was a problem hiding this comment.
This should actually work, and also did in the past.
The Refitter.SourceGenerator project includes a .props file that does the following:
<Project>
<ItemGroup>
<AdditionalFiles Include="**/*.refitter" />
</ItemGroup>
</Project>So by referencing the Refitter.SourceGenerator nuget package, the .props file should be included in the build
I'd like to investigate this a bit before we merge this in
There was a problem hiding this comment.
When I create an .NET 8.0 Console App, add a .refitter file and add a package reference to Refitter.SourceGenerator from Visual Studio, I can see that the .refitter file is automatically included as an additional file
There was a problem hiding this comment.
It seems very unstable when running from Visual Studio, as compared to VS Code or Rider, which seems to forcefully run the source generator upon Build. Visual Studio would run it upon incremental changes to the .refitter file, and seems to only really work once the output file already exists on disk.
I don't remember the behavior to be this way, or maybe my memory of this is wrong
There was a problem hiding this comment.
Fair - I did notice things were presenting differently when I tried via CLI. I had assumed that perhaps the documentation was wrong.
Happy to revert the change and investigate a bit more before any action there.
There was a problem hiding this comment.
I will test again a bit later to see if having the files nested (mine are in Clients/Identity.refitter for example) makes a difference (and if it being a web project does, too?)
| // add a source output that logs what we found for easier troubleshooting and setup | ||
| context.RegisterSourceOutput(refitterPathList, static (spc, paths) => | ||
| { | ||
| if (paths.Length == 0) | ||
| { | ||
| // log a warning if no .refitter files were found, instructing the user how to add them | ||
| Debug.WriteLine("[Refitter] No .refitter files found. Ensure they are added to your project as `<AdditionalFiles Include=\"Petstore.refitter\" />`"); | ||
| return; | ||
| } | ||
|
|
||
| // log each found .refitter file path | ||
| foreach (var path in paths) | ||
| { | ||
| Debug.WriteLine($"[Refitter] Found .refitter file: {path}"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
I'm honestly also surprised that it works to use Debug.WriteLine 🤔
I know that ReportDiagnostic works but I could only see the output from the Errors List Window
|
@all-contributors please add @codymullins for code |
|
I've put up a pull request to add @codymullins! 🎉 |
I was pretty confused why nothing was happening when first trying to get the source generator working. It was mostly user error, though the README has some (seemingly) contradictory instructions.
This pull requests adds clear, explicit logging for which
.refitterfiles are found. I waffled back and forth on whether to only include the debug log if NONE are found, but it seems ultimately helpful to have a log in the build output for what is generated.I tested adding explicit warnings for this using the
Diagnostics, but I could not get it to appear anywhere in Visual Studio.No refitter files:
One refitter file:
Summary by CodeRabbit
New Features
Documentation