[Foundation] Fix nullability in NSUrlCredential.#24277
Conversation
This is file 15 of 47 files with nullability disabled in Foundation. Changes: - Enabled nullable reference types - Made SecIdentity property return SecIdentity? (nullable) since it can return null when Identity handle is IntPtr.Zero - Replaced "To be added." XML documentation with proper documentation for all members - Added see cref attributes for better cross-referencing - Removed unnecessary whitespace in XML comments - Replaced manual null checks with ArgumentNullException.ThrowIfNull for cleaner code - Added returns documentation for all methods that were missing it Contributes towards #17285.
There was a problem hiding this comment.
Pull Request Overview
This PR enables nullable reference types for the NSUrlCredential class in Foundation, contributing to the larger effort tracked in issue #17285. The changes improve code safety and documentation quality.
Key changes:
- Enabled nullable reference types by adding
#nullable enable - Updated the
SecIdentityproperty return type to be nullable (SecIdentity?) to accurately reflect that it can return null - Replaced manual null checks with
ArgumentNullException.ThrowIfNullfor cleaner, more modern code
| { | ||
| if (identity is null) | ||
| throw new ArgumentNullException ("identity"); | ||
| ArgumentNullException.ThrowIfNull (identity); |
There was a problem hiding this comment.
The certificates parameter should have a null check added for consistency. The static method FromIdentityCertificatesPersistance checks both identity and certificates parameters (lines 35-36), but this constructor only checks identity. Since nullable reference types are now enabled and certificates is declared as non-nullable (SecCertificate []), a null check should be added here:
ArgumentNullException.ThrowIfNull (identity);
ArgumentNullException.ThrowIfNull (certificates);| ArgumentNullException.ThrowIfNull (identity); | |
| ArgumentNullException.ThrowIfNull (identity); | |
| ArgumentNullException.ThrowIfNull (certificates); |
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.
…ullability-15-nsurlcredential
✅ [CI Build #f384725] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #f384725] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #f384725] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #f384725] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #f384725] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #f384725] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #f384725] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) 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 |
💻 [CI Build #f384725] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
🚀 [CI Build #f384725] 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 |
This is file 15 of 47 files with nullability disabled in Foundation.
Changes:
Contributes towards #17285.