-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix CS8603 error for nullable reference types in SourceGen binding paths #32480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: simonrozsival <[email protected]>
When binding path contains conditional access to a nullable object followed by a reference type property (e.g., source.NullableObject?.StringProperty where StringProperty is string?), the property type now correctly includes nullable annotation. This prevents CS8603 "Possible null reference return" errors in generated binding code.
The fix ensures reference types with conditional access in the path are marked as nullable in the TypedBinding signature, while preserving the existing behavior for value types (which use ?? default fallback).
Fixes issue where XAML bindings like <Entry Text="{Binding SomeNullableObject.SomeStringProperty}"/> would generate code with CS8603 errors when SomeNullableObject is nullable and SomeStringProperty is a nullable reference type.
Co-authored-by: simonrozsival <[email protected]>
src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs
Show resolved
Hide resolved
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs
Outdated
Show resolved
Hide resolved
simonrozsival
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot PTAL
- Changed StringProperty from nullable (string?) to non-nullable (string) in test cases - the nullability comes from NullableObject being nullable, not the property itself - Removed WriteLine debugging statements from tests - Converted CorrectlyDetectsNullableReferenceTypes test to a snapshot test with full expected output - Split combined assertions into separate asserts for better test clarity - Removed unnecessary blank lines from expected output formatting - Updated setter expectations to include null check when property is non-nullable Co-authored-by: simonrozsival <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes CS8603 errors in SourceGen-generated code when binding to nullable reference types through a nullable path (e.g., source.SomeNullableObject?.SomeStringProperty).
Key Changes:
- Fixed bug in
CompiledBindingMarkup.cswhere nullable annotation was being overwritten - Added comprehensive test coverage for nullable reference types in binding paths
- Tests verify both the generated code structure and absence of CS8603 errors
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Controls/src/SourceGen/CompiledBindingMarkup.cs |
Fixed nullable annotation logic by moving assignment before annotation |
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs |
Added snapshot test and error verification test for nullable reference types |
src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs |
Added integration tests for SetBinding and Create with nullable paths |
|
Is this part of the latest .NET release today? |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [Microsoft.Maui.Controls](https://github.com/dotnet/maui) | nuget | patch | `10.0.11` -> `10.0.20` | --- ### Release Notes <details> <summary>dotnet/maui (Microsoft.Maui.Controls)</summary> ### [`v10.0.20`](https://github.com/dotnet/maui/releases/tag/10.0.20): SR2 [Compare Source](dotnet/maui@10.0.11...10.0.20) ##### What's Changed **Note:** This is a service release (SR2) tracking ongoing release/10.0.1xx-sr2 branch development. The version number 10.0.20 represents the second service release for .NET MAUI 10. .NET MAUI 10.0.20 development introduces significant improvements across all platforms with focus on quality, performance, and developer experience. Current development includes enhancements to XAML Source Generator, iOS/macOS fixes, AppThemeBinding improvements, and improved developer tooling. ##### AI - PR Reviewer Agent by [@​PureWeen](https://github.com/PureWeen) in dotnet/maui#32340 - \[AI] The Issue-Resolver agent by [@​kubaflo](https://github.com/kubaflo) in dotnet/maui#32804 - \[AI] Fixed links for agents by [@​kubaflo](https://github.com/kubaflo) in dotnet/maui#32818 - Add checkpoint/resume system for PR review agent by [@​kubaflo](https://github.com/kubaflo) in dotnet/maui#32774 - Improved pr-reviewer AI agent and modified the usage instructions in README by [@​kubaflo](https://github.com/kubaflo) in dotnet/maui#32718 - Agents scripts by [@​PureWeen](https://github.com/PureWeen) in dotnet/maui#32819 - Simplify agent instructions and consolidate sandbox testing workflow by [@​Copilot](https://github.com/Copilot) in dotnet/maui#32950 ##### Brushes - Fix SolidColorBrush.Equals to compare Color values instead of references by [@​StephaneDelcroix](https://github.com/StephaneDelcroix) in dotnet/maui#32956 <details> <summary>🔧 Fixes</summary> - [Using DynamicResource with OnPlatform<Color> causes infinite loop](dotnet/maui#27281) </details> ##### Data Binding - Fix CS8603 error for nullable reference types in SourceGen binding paths by [@​Copilot](https://github.com/Copilot) in dotnet/maui#32480 <details> <summary>🔧 Fixes</summary> - [error CS8603: Possible null reference return in generated code using SourceGen](dotnet/maui#32478) </details> ##### Docs - Add README-AI.md file by [@​PureWeen](https://github.com/PureWeen) in dotnet/maui#32502 - Improve AI agent documentation and testing instructions by [@​Copilot](https://github.com/Copilot) in https://github.com/dotnet/maui/p...
Fix Implemented and Tested - Review Feedback Addressed
Issue Analysis:
source.SomeNullableObject.SomeStringPropertywhereSomeNullableObjectis nullable andSomeStringPropertyis a reference type, the generated code produces CS8603 errorCompiledBindingMarkup.csin theTryParsePathmethod (lines 410-422)previousPartTypeFix Applied:
propertyType = previousPartTypeassignmentReview Feedback Addressed:
StringPropertyfromstring?tostringin test cases (nullability comes from nullable object, not the property)Testing:
Files modified:
src/Controls/src/SourceGen/CompiledBindingMarkup.cs- Fixed nullable annotation logicsrc/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs- Updated test cases per review feedbacksrc/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs- Updated test cases per review feedbackOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.