-
Notifications
You must be signed in to change notification settings - Fork 554
[Foundation] Fix nullability in NSMutableDictionary. #24428
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
[Foundation] Fix nullability in NSMutableDictionary. #24428
Conversation
* Enable nullability for NSMutableDictionary.cs. * Fix interface implementations to match nullability contracts (IDictionary, IDictionary<NSObject, NSObject>). * Make indexers' return type allow null, but disallow setting null values. * Remove all 'To be added.' XML comments. * Add comprehensive XML documentation for all public methods, properties, and indexers. * Use proper 'see cref' attributes for type references in documentation. * Fix ArgumentNullException to use nameof for parameter names. * Remove an unused constructor. Also add NSMutableDictionary tests for missing key behavior: * Add tests to verify that accessing missing keys returns null. * Test all indexer overloads (string, NSObject, NSString). * Test ObjectForKey method with missing keys. * Test TryGetValue with missing keys returns false and null output. * Test IDictionary interface implementations (indexer and Contains). * Ensure existing keys still work correctly after testing missing keys. These tests document and verify the expected behavior when fetching values for keys that don't exist in the dictionary. This is file 34 of 47 files with nullability disabled in Foundation. Contributes towards #17285.
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 enables nullable reference types for NSMutableDictionary.cs (file 34 of 47) and updates related code that depends on it. The changes ensure proper nullability contracts for dictionary operations, particularly handling the case where missing keys return null, while preventing null value assignments. The PR also adds comprehensive XML documentation and test coverage for missing key scenarios.
Key changes:
- Enable nullability for NSMutableDictionary and update indexers to return nullable NSObject while disallowing null value assignments using [DisallowNull] attribute
- Add comprehensive XML documentation replacing "To be added" placeholders
- Add tests verifying null return behavior for missing keys across all indexer overloads
- Update dependent files across multiple frameworks to handle nullable dictionary returns
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Foundation/NSMutableDictionary.cs | Enables nullability, adds comprehensive XML docs, fixes ArgumentNullException to use nameof, adds [DisallowNull] attributes on indexers |
| src/Foundation/NSDictionary.cs | Adds nullable annotations to indexers in base class, adds [DisallowNull] attributes |
| tests/monotouch-test/Foundation/NSMutableDictionaryTest.cs | Adds 7 new tests verifying null return behavior for missing keys across different indexer overloads and methods |
| tests/cecil-tests/Documentation.KnownFailures.txt | Removes entries for newly documented NSMutableDictionary members |
| src/WebKit/WebNavigationPolicyEventArgs.cs | Updates to handle nullable dictionary indexer returns with null-coalescing operators |
| src/Security/Items.cs | Adds null check after dictionary lookup before constructing SecRecord |
| src/SceneKit/SCNParticleSystem.cs | Adds ArgumentNullException.ThrowIfNull for dictionary value parameter |
| src/Foundation/NSUrlSessionHandler.cs | Updates to use pattern matching for safer dictionary value retrieval |
| src/CoreText/CTTextTab.cs | Adds nullable annotation and null-forgiving operator for dictionary lookup |
| src/CoreText/CTFontDescriptor.cs | Adds nullable casts and null checks for dictionary lookups |
| src/CoreText/CTFont.cs | Adds null checks and default returns for dictionary lookups returning NSNumber |
| src/AudioToolbox/AudioToolbox.cs | Adds nullable annotation for dictionary value property |
| src/AudioToolbox/AudioFile.cs | Updates to check for null dictionary before constructing AudioFileInfoDictionary |
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
✅ [CI Build #b61b97b] Build passed (Build packages) ✅Pipeline on Agent |
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.
✅ [PR Build #b61b97b] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #b61b97b] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #b61b97b] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #b61b97b] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #b61b97b] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #b61b97b] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
💻 [CI Build #b61b97b] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #b61b97b] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 124 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Also add NSMutableDictionary tests for missing key behavior:
These tests document and verify the expected behavior when fetching
values for keys that don't exist in the dictionary.
This is file 34 of 47 files with nullability disabled in Foundation.
Contributes towards #17285.