Fix SYSLIB1045 quick fix for C# 14 extension members#124311
Fix SYSLIB1045 quick fix for C# 14 extension members#124311stephentoub merged 3 commits intomainfrom
Conversation
…l modifier The fixer now correctly skips ExtensionBlockDeclarationSyntax nodes when: - Adding the partial modifier to ancestor types (extension blocks can't be partial) - Finding the parent type to place the generated regex property in Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the SYSLIB1045 code fixer where it generated invalid partial extension(...) syntax when applied to Regex calls inside C# 14 extension blocks. Extension blocks inherit from TypeDeclarationSyntax but cannot be marked partial, causing the fixer to incorrectly treat them like regular type declarations.
Changes:
- Added filtering to skip
ExtensionBlockDeclarationSyntaxwhen traversing type ancestors to addpartialmodifiers or place generated regex properties - Ensures generated regex members are placed in the parent class rather than inside the extension block
- Added test coverage for extension member scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs | Added ExtensionBlockDeclarationSyntax filtering in 4 locations: CreateGeneratedRegexProperty, ConvertFieldToGeneratedRegexProperty, ConvertPropertyToGeneratedRegexProperty, and TryPartialize methods |
| src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs | Added CodeFixSupportsExtensionMembers test to verify the fix correctly handles Regex.IsMatch calls inside extension blocks |
Split the extension member test into two: one for static invocation (Regex.IsMatch) and one for constructor (new Regex), covering both code paths through CreateGeneratedRegexProperty. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
|
@copilot please invoke the code-review skill and post the analysis/results as a comment on the PR |
🤖 Copilot Code Review — PR #124388Holistic AssessmentMotivation: The problem is real and well-defined. Approach: The approach is correct — filtering out Summary: ✅ LGTM. The fix is straightforward, correct, and well-tested. All four changed locations are necessary and consistent. The code changes are minimal, focused, and match the existing patterns in the file. Detailed Findings✅ Correctness — All four filter locations are necessary and correctThe four changes form two logical groups:
✅ Test coverage — Adequate for the reachable code pathsTwo tests cover both reachable
The field/property conversion paths (lines 214, 259) are unreachable from extension blocks due to CS9282, so they don't need dedicated test coverage — the defensive filtering there is a sensible safeguard. ✅ Style — Consistent with existing code patternsThe filter pattern 💡 Minor observation — Duplicate filter patternThe expression |
|
/ba-g timeout |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Description
The SYSLIB1045 code fixer generates invalid
partial extension(...)syntax when applied inside C# 14 extension blocks.ExtensionBlockDeclarationSyntaxinherits fromTypeDeclarationSyntax, so the fixer was treating extension blocks like classes when addingpartialmodifiers and placing generated members.Before fix:
After fix:
Changes
TryPartialize: Filter outExtensionBlockDeclarationSyntaxwhen addingpartialto ancestor typesCreateGeneratedRegexProperty: Skip extension blocks when resolving the parent type for member placementConvertFieldToGeneratedRegexProperty/ConvertPropertyToGeneratedRegexProperty: Defensive skip of extension blocks when resolving parent type (fields and properties are not valid inside extension blocks per CS9282, so these paths are unreachable in practice)CodeFixSupportsExtensionMembers_StaticInvocationtest covering the static method invocation path (Regex.IsMatch) inside an extension blockCodeFixSupportsExtensionMembers_Constructortest covering the constructor path (new Regex) inside an extension blockOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.