[CoreNFC] Add missing nullability on NSError parameters in callbacks. - #26531
[CoreNFC] Add missing nullability on NSError parameters in callbacks.#26531rolfbjarne wants to merge 6 commits into
Conversation
Count both the array and its element type when walking NullableAttribute positions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fdf7050-96ec-4575-91b8-12824be5644a
Non-generic value types do not consume positions in NullableAttribute byte arrays. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fdf7050-96ec-4575-91b8-12824be5644a
There was a problem hiding this comment.
🟡 Changes recommended
xtro-sharpie’s CountNullablePositions still miscounts generic value types due to branch ordering, which can produce incorrect nullability diagnostics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates CoreNFC bindings to correctly surface nullable NSError parameters in completion callbacks (per Apple docs), and adjusts xtro-sharpie’s nullability accounting so it can correctly interpret nullable positions for arrays/value types when validating generated APIs.
Changes:
- Enable nullable context in
src/corenfc.csand annotateNSError(and related callback parameters) as nullable where native APIs can passnil. - Fix xtro-sharpie’s nullable-position counting for array types and value types.
- Refresh xtro-sharpie ignore baselines by removing entries/files that are no longer expected after the analysis fix and binding updates.
File summaries
| File | Description |
|---|---|
| tests/xtro-sharpie/xtro-sharpie/NullabilityCheck.cs | Updates nullable-position accounting (arrays/value types) used to validate block parameter nullability. |
| tests/xtro-sharpie/api-annotations-dotnet/macOS-HealthKit.ignore | Removes a now-unneeded ignore entry after nullability-position accounting changes. |
| tests/xtro-sharpie/api-annotations-dotnet/MacCatalyst-HealthKit.ignore | Removes a now-unneeded ignore entry after nullability-position accounting changes. |
| tests/xtro-sharpie/api-annotations-dotnet/iOS-HealthKit.ignore | Removes a now-unneeded ignore entry after nullability-position accounting changes. |
| tests/xtro-sharpie/api-annotations-dotnet/MacCatalyst-CoreNFC.ignore | Removes the ignore file now that CoreNFC callback nullability is corrected and/or diagnostics no longer apply. |
| tests/xtro-sharpie/api-annotations-dotnet/iOS-CoreNFC.ignore | Removes now-unneeded CoreNFC nullability ignore entries. |
| tests/xtro-sharpie/api-annotations-dotnet/MacCatalyst-AppClip.ignore | Removes the ignore file now that diagnostics no longer apply. |
| tests/xtro-sharpie/api-annotations-dotnet/iOS-AppClip.ignore | Removes the ignore file now that diagnostics no longer apply. |
| tests/xtro-sharpie/api-annotations-dotnet/iOS-ARKit.ignore | Removes the ignore file now that diagnostics no longer apply. |
| src/corenfc.cs | Enables nullable context and fixes nullable NSError (and related) parameters in completion callbacks, including NFCQueryNdefStatusCompletionHandler.error. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Move the IsValueType check before GenericInstanceType handling in CountNullablePositions so generic value types (such as Nullable<T>) contribute zero nullable metadata positions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
rolfbjarne
added
ready-to-review
✅ 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 #107084c] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 0 tests failed, 261 tests passed. Failures❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Also a few nullability fixes in xtro to properly detect nullable types.
Fixes #21540.