[RGen] Fix how we detect 'this' parameters. #23360
Conversation
Fix the method that parses a parameter to correctly mark it as a 'this' parameter and update the parameter formater to correctly render the parameter modifiers.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes how the RGen tool detects 'this' parameters in extension methods and ensures they are correctly rendered with proper modifiers in generated code.
- Corrects parameter parsing to detect 'this' parameters from syntax rather than symbol metadata
- Updates parameter formatter to properly handle 'this' modifier rendering
- Adds test coverage for extension method parameter formatting
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/rgen/Microsoft.Macios.Generator/DataModel/Parameter.Generator.cs |
Changes detection logic to use syntax-based parsing for 'this' parameters |
src/rgen/Microsoft.Macios.Generator/Formatters/ParameterFormatter.cs |
Updates parameter formatter to correctly render 'this' modifier |
tests/rgen/Microsoft.Macios.Generator.Tests/Formatters/MethodFormatterTests.cs |
Adds test case for extension method parameter formatting |
| SyntaxTokenList modifiers; | ||
| if (parameter.IsThis) { | ||
| modifiers = TokenList (Token (SyntaxKind.ThisKeyword)); | ||
| } else { | ||
| // modifiers come from two situations, we have the params keyword or not. We cannot have params + a ref modifier | ||
| // so we build them based on that. If you call WithModifiers twice, you will me stepping on the previous collection | ||
| // it won't be a merge | ||
| modifiers = parameter.IsParams | ||
| ? TokenList (Token (SyntaxKind.ParamsKeyword)) | ||
| : parameter.ReferenceKind.ToTokens (); | ||
| } |
There was a problem hiding this comment.
[nitpick] The modifiers variable is declared but not initialized. Consider declaring it inline within each branch to make the logic clearer and avoid potential uninitialized usage.
Copilot uses AI. Check for mistakes.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
src/rgen/Microsoft.Macios.Generator/Formatters/ParameterFormatter.cs
Outdated
Show resolved
Hide resolved
…ter.cs Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
✅ [CI Build #bea86a5] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #bea86a5] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commit.NET ( No breaking changes )✅ API diff vs stable.NET ( No breaking changes )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #bea86a5] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #bea86a5] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #bea86a5] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #bea86a5] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #bea86a5] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
🚀 [CI Build #bea86a5] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 115 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Fix the method that parses a parameter to correctly mark it as a 'this' parameter and update the parameter formater to correctly render the parameter modifiers.