[Foundation] Improve NSArray.FromNativeObjects slightly.#24509
[Foundation] Improve NSArray.FromNativeObjects slightly.#24509rolfbjarne merged 3 commits intomainfrom
Conversation
* Enable nullability. * Reuse existing code to create the native array. * Add tests. * Add xml docs. * Update nullability in some of the consumers of NSArray.FromNativeObjects whenever it makes sense. Contributes towards #17285.
There was a problem hiding this comment.
Pull request overview
This PR improves the NSArray.FromNativeObjects method by enabling nullable reference types, adding XML documentation, improving performance by reusing existing code to create native arrays, and updating nullability annotations in consumer methods. The changes also add comprehensive tests to verify null value handling in dictionary and ordered set operations.
Changes:
- Enabled nullable reference types for
NSArray.FromNativeObjectsmethods and added comprehensive XML documentation - Refactored
NSArray.FromNativeObjectsto useFromIntPtrsinstead of manual marshaling, eliminating memory allocation/deallocation overhead - Updated nullability in
NSDictionary,NSDictionary<TKey,TValue>, andNSMutableOrderedSet<TKey>methods to accept nullable arrays - Added comprehensive tests for null value handling in dictionaries and ordered sets
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Foundation/NSArray.cs | Enabled nullable reference types, added XML docs, refactored to use FromIntPtrs, added negative count validation |
| src/Foundation/NSDictionary.cs | Updated method signatures to accept NSObject?[] and improved XML documentation |
| src/Foundation/NSDictionary_2.cs | Updated generic method signatures to accept nullable arrays and refactored to call count-based overload |
| src/Foundation/NSMutableOrderedSet_1.cs | Updated method signatures to accept nullable arrays and improved XML documentation |
| tests/monotouch-test/Foundation/NSDictionaryTest.cs | Added tests for null value handling in non-generic dictionaries |
| tests/monotouch-test/Foundation/NSDictionary2Test.cs | Added tests for null value handling in generic dictionaries |
| tests/monotouch-test/Foundation/NSMutableOrderedSet1Test.cs | Added tests for null value handling in ordered set operations |
Comments suppressed due to low confidence (1)
src/Foundation/NSDictionary.cs:191
- The error message is not descriptive. It should explain what is wrong with the count parameter. Consider using a message like "count must be between 1 and the array length" for clarity.
if (count < 1 || objects.Length < count || keys.Length < count)
throw new ArgumentException ("count");
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #559acab] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #559acab] Build passed (Detect API changes) ✅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 |
✅ [CI Build #559acab] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #559acab] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #559acab] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #559acab] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #559acab] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #559acab] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #559acab] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 117 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
whenever it makes sense.
Contributes towards #17285.