[UIKit] Enable nullability and clean up UIGraphics.#24655
Conversation
This is file 11 of 30 files with nullability disabled in UIKit. * Enable nullability (#nullable enable). * Add nullable annotations (UIImage?, CGContext?, NSDictionary?, CGPDFInfo?). * Fix null reference return in UIImage.Scale methods caused by nullable GetImageFromCurrentImageContext return type. * Improve XML documentation comments: remove 'To be added.' placeholders, fix formatting, add missing docs for BeginImageContextWithOptions, EndPDFContext, and other members, add 'see cref' references, remove empty <value> and <remarks> elements, fix element ordering (summary before param), correct PDF context method remarks to reference EndPDFContext instead of EndImageContext. Contributes towards #17285.
There was a problem hiding this comment.
Pull request overview
This PR continues the UIKit nullability enablement work by switching UIGraphics to #nullable enable, updating a few related APIs/annotations (including UIImage.Scale), and cleaning up/improving XML documentation so the doc build no longer needs known-failure exceptions.
Changes:
- Enable nullable reference types in
src/UIKit/UIGraphics.csand update several APIs to return/accept nullable values where appropriate. - Fix
UIImage.Scaleto propagate possiblenullresults fromUIGraphics.GetImageFromCurrentImageContext (). - Improve/normalize XML documentation and remove corresponding entries from
Documentation.KnownFailures.txt.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/cecil-tests/Documentation.KnownFailures.txt | Removes known doc-generation failures now that UIGraphics XML docs are improved. |
| src/UIKit/UIImage.cs | Makes Scale overloads return UIImage? to reflect possible null results from UIGraphics image-context extraction. |
| src/UIKit/UIGraphics.cs | Enables nullability, updates return/parameter nullability, and performs significant XML doc cleanup/expansion. |
Comments suppressed due to low confidence (3)
src/UIKit/UIGraphics.cs:185
- With nullable enabled,
SetPDFContextURLshould validate the non-nullableurlparameter before usingurl.Handle. Otherwise anullvalue will result in aNullReferenceException; prefer throwingArgumentNullExceptionearly.
public static void SetPDFContextURL (NSUrl url, CGRect rect)
{
UIGraphicsSetPDFContextURLForRect (url.Handle, rect);
GC.KeepAlive (url);
src/UIKit/UIGraphics.cs:266
- With nullable enabled,
BeginPDFPage (CGRect, NSDictionary)should validate the non-nullablepageInfoparameter before usingpageInfo.Handle. Otherwise anullvalue will result in aNullReferenceException; prefer throwingArgumentNullExceptionearly.
public static void BeginPDFPage (CGRect bounds, NSDictionary pageInfo)
{
UIGraphicsBeginPDFPageWithInfo (bounds, pageInfo.Handle);
GC.KeepAlive (pageInfo);
src/UIKit/UIGraphics.cs:337
- With nullable enabled,
PushContextshould validate the non-nullablectxparameter before dereferencing it. As-is, passingnullwill throw aNullReferenceExceptionwhen accessingctx.Handle; prefer throwingArgumentNullException(for example viaArgumentNullException.ThrowIfNull (ctx)).
public static void PushContext (CGContext ctx)
{
UIGraphicsPushContext (ctx.Handle);
GC.KeepAlive (ctx);
| /// <para>This function can only be invoked from the UI thread.</para> | ||
| /// </remarks> | ||
| public static void BeginPDFContext (NSMutableData data, CGRect bounds, NSDictionary? documentInfo) | ||
| { |
There was a problem hiding this comment.
With nullable enabled, BeginPDFContext (NSMutableData ...) should validate the non-nullable data parameter before accessing data.Handle. Passing null currently results in a NullReferenceException; prefer throwing ArgumentNullException early.
| { | |
| { | |
| System.ArgumentNullException.ThrowIfNull (data); |
| [UnsupportedOSPlatform ("tvos17.0")] | ||
| [UnsupportedOSPlatform ("maccatalyst17.0")] | ||
| public UIImage Scale (CGSize newSize, nfloat scaleFactor) | ||
| public UIImage? Scale (CGSize newSize, nfloat scaleFactor) |
There was a problem hiding this comment.
Scale now returns UIImage?, but the XML documentation still says "The scaled image." without mentioning that the return value can be null (via UIGraphics.GetImageFromCurrentImageContext ()). Please update the <returns> text to document the possible null return.
| [UnsupportedOSPlatform ("tvos17.0")] | ||
| [UnsupportedOSPlatform ("maccatalyst17.0")] | ||
| public UIImage Scale (CGSize newSize) | ||
| public UIImage? Scale (CGSize newSize) |
There was a problem hiding this comment.
Same as the other overload: this Scale method now returns UIImage?, but the XML documentation does not mention that null can be returned. Please update the <returns> docs accordingly.
| /// <remarks> | ||
| /// <para>UIKit keeps a stack of image contexts. This method creates a new image context, makes it the default and places it at the top of the graphic context stack.</para> | ||
| /// <para>To restore the previous graphics context, call the <see cref="EndImageContext" /> method.</para> | ||
| /// <para>You can get the current context by calling the <see cref="GetImageFromCurrentImageContext" /> method.</para> |
There was a problem hiding this comment.
The remarks say "You can get the current context by calling GetImageFromCurrentImageContext", but that API returns an image (not a graphics context). This looks like a documentation bug; please adjust the wording to refer to getting the current image from the image context.
| /// <para>You can get the current context by calling the <see cref="GetImageFromCurrentImageContext" /> method.</para> | |
| /// <para>You can get the current image from the current image context by calling the <see cref="GetImageFromCurrentImageContext" /> method.</para> |
| /// <remarks> | ||
| /// <para>UIKit keeps a stack of image contexts. This method creates a new image context, makes it the default and places it at the top of the graphic context stack.</para> | ||
| /// <para>To restore the previous graphics context, call the <see cref="EndImageContext" /> method.</para> | ||
| /// <para>You can get the current context by calling the <see cref="GetImageFromCurrentImageContext" /> method.</para> |
There was a problem hiding this comment.
Same documentation issue as BeginImageContext: the remarks state that GetImageFromCurrentImageContext returns the current context, but it returns an image. Please correct this wording here as well.
| /// <para>You can get the current context by calling the <see cref="GetImageFromCurrentImageContext" /> method.</para> | |
| /// <para>You can get the image from the current image context by calling the <see cref="GetImageFromCurrentImageContext" /> method.</para> |
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.
✅ [PR Build #094b726] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #094b726] Build passed (Build packages) ✅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 #094b726] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #094b726] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #094b726] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #094b726] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #094b726] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #094b726] 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 #094b726] 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 |
This is file 11 of 30 files with nullability disabled in UIKit.
Contributes towards #17285.