feat: support custom formatting in interpolated string handlers#241
feat: support custom formatting in interpolated string handlers#241
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds format-aware overloads for Write, WriteLine, and AppendFormatted across primitive, nullable, object, and generic types; updates existing overloads to delegate to the new format-enabled variants. Tests are consolidated and expanded to validate formatting; minor build/test project formatting and package updates applied. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@tests/InterpolationCodeWriter.Tests/SourceTextWriterTest.WriteLine.cs`:
- Around line 5-13: This test file is using nullable annotations but lacks a
file-level opt-in; add the nullable context directive by placing "#nullable
enable" at the top of the file so the nullable checks apply for the tests
(affects the code around the WriteLineValueAppendsStringRepresentation test and
any usages of SourceTextWriter and the MemberData methods like
WriteLinePrimitiveCases/WriteLineNullableValueCases).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Directory.Build.propsSourceGenerators.slnxmodules/polyfill.propssrc/InterpolationCodeWriter.CSharp.Sources/build/Raiqub.Generators.InterpolationCodeWriter.CSharp.Sources.targetssrc/InterpolationCodeWriter/SourceTextWriter.Interpolation.cssrc/InterpolationCodeWriter/SourceTextWriter.Write.cssrc/InterpolationCodeWriter/SourceTextWriter.WriteLine.cstests/Directory.Packages.propstests/InterpolationCodeWriter.CSharp.Tests/CodeWriterDispatcherGenericTest.cstests/InterpolationCodeWriter.CSharp.Tests/CodeWriterDispatcherTest.cstests/InterpolationCodeWriter.CSharp.Tests/Fakes/SourceProductionContextRunner.cstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.Interpolation.cstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.Write.cstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.WriteFormat.cstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.WriteLine.cstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.WriteLineFormat.cs
💤 Files with no reviewable changes (1)
- Directory.Build.props
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/InterpolationCodeWriter.CSharp.Tests/InterpolationCodeWriter.CSharp.Tests.csproj (1)
14-27: LGTM! Coverage tooling additions are correctly configured.The addition of
coverlet.msbuildalongsidecoverlet.collectorprovides comprehensive code coverage support. Package references correctly use central package management without explicit versions.Minor nit (optional): The element ordering within
coverlet.msbuild(lines 19-20) is reversed compared tocoverlet.collector(lines 15-16). Consider aligning for consistency:✨ Optional consistency fix
<PackageReference Include="coverlet.msbuild"> - <PrivateAssets>all</PrivateAssets> <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> + <PrivateAssets>all</PrivateAssets> </PackageReference>,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/InterpolationCodeWriter.CSharp.Tests/InterpolationCodeWriter.CSharp.Tests.csproj` around lines 14 - 27, The two PackageReference blocks should be consistent: in the coverlet.msbuild PackageReference (the element with Include="coverlet.msbuild") swap the order of the child elements so they match coverlet.collector (place the IncludeAssets element before the PrivateAssets element); update the coverlet.msbuild PackageReference to have IncludeAssets then PrivateAssets to maintain consistent element ordering across both package references.tests/InterpolationCodeWriter.Tests/InterpolationCodeWriter.Tests.csproj (1)
13-20: Both coverlet.collector and coverlet.msbuild are included.Having both coverage packages provides flexibility for different collection mechanisms (data collector vs. MSBuild integration), but they serve overlapping purposes. If your CI/CD pipeline only uses one approach, consider keeping only the relevant package to reduce dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/InterpolationCodeWriter.Tests/InterpolationCodeWriter.Tests.csproj` around lines 13 - 20, The project currently references both coverlet.collector and coverlet.msbuild; decide which coverage mechanism your CI uses and remove the unused package reference to avoid redundant dependencies—either keep coverlet.collector (data collector approach) or keep coverlet.msbuild (MSBuild integration) and delete the other PackageReference entry for coverlet.collector or coverlet.msbuild in the project file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tests/InterpolationCodeWriter.CSharp.Tests/InterpolationCodeWriter.CSharp.Tests.csproj`:
- Around line 14-27: The two PackageReference blocks should be consistent: in
the coverlet.msbuild PackageReference (the element with
Include="coverlet.msbuild") swap the order of the child elements so they match
coverlet.collector (place the IncludeAssets element before the PrivateAssets
element); update the coverlet.msbuild PackageReference to have IncludeAssets
then PrivateAssets to maintain consistent element ordering across both package
references.
In `@tests/InterpolationCodeWriter.Tests/InterpolationCodeWriter.Tests.csproj`:
- Around line 13-20: The project currently references both coverlet.collector
and coverlet.msbuild; decide which coverage mechanism your CI uses and remove
the unused package reference to avoid redundant dependencies—either keep
coverlet.collector (data collector approach) or keep coverlet.msbuild (MSBuild
integration) and delete the other PackageReference entry for coverlet.collector
or coverlet.msbuild in the project file.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CLAUDE.mdtests/Directory.Packages.propstests/InterpolationCodeWriter.CSharp.Tests/InterpolationCodeWriter.CSharp.Tests.csprojtests/InterpolationCodeWriter.CSharp.Tests/packages.lock.jsontests/InterpolationCodeWriter.Tests/InterpolationCodeWriter.Tests.csprojtests/InterpolationCodeWriter.Tests/packages.lock.json
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Directory.Packages.props
2 participants
Pull Request
The issue or feature being addressed
Adds support for custom format strings in interpolated string handlers of
SourceTextWriter, enabling callers to pass format specifiers (e.g.,{value:X2}) directly via string interpolation.Details on the issue fix or feature implementation
AppendFormatted<T>(T value, string? format)overload to theInterpolatedStringHandlerinSourceTextWriter.Interpolation.csto handle format strings in interpolated expressionsWriteandWriteLinemethods inSourceTextWriter.Write.csandSourceTextWriter.WriteLine.csto support format parametersSourceTextWriterTest.WriteFormat.csandSourceTextWriterTest.WriteLineFormat.cscovering the new formatting behaviourSourceTextWriterTest.Write.csandSourceTextWriterTest.WriteLine.csto align with the refactored method signaturesCodeWriterDispatcherGenericTest.cs,CodeWriterDispatcherTest.cs) andSourceProductionContextRunner.csConfirm the following
Summary by CodeRabbit
New Features
Tests
Style
Documentation