Skip to content

[xtro-sharpie] Fix ProtocolAttribute native name lookup - #26498

Merged
rolfbjarne merged 6 commits into
mainfrom
dev/rolf/upgraded-potato
Sep 4, 2026
Merged

[xtro-sharpie] Fix ProtocolAttribute native name lookup#26498
rolfbjarne merged 6 commits into
mainfrom
dev/rolf/upgraded-potato

Conversation

@rolfbjarne

@rolfbjarne rolfbjarne commented Aug 28, 2026

Copy link
Copy Markdown
Member

Fixes the xtro-sharpie nullability check for members of protocols whose native name is supplied through ProtocolAttribute.Name.

Helpers.GetName(TypeDefinition) previously only inspected constructor arguments and fell back to the managed type name. For many types, that prevented matching native declarations and caused a number of diagnostics to go unreported.

🤖 Pull request created by Copilot

Helpers.GetName(TypeDefinition) only checked ProtocolAttribute's
constructor arguments for the native protocol name, but [Protocol]'s
Name is set via a named property, not a constructor argument (e.g.
[Protocol (Name = "NFCISO15693Tag")]). This caused the lookup to
fall back to the managed type name (NFCIso15693Tag), which doesn't
match the native declaration's casing (NFCISO15693Tag), so
NullabilityCheck's GetMethod lookup silently failed and no
!missing-null-allowed! diagnostic was ever produced for members of
such protocols.

This is why NFCISO15693Tag.stayQuietWithCompletionHandler:'s missing
nullable annotation on its block parameter was never caught, even
though the sibling ConnectToTag/ConnectTo methods (bound with
non-property-based native names) were correctly flagged.

Fix GetName to also check the attribute's properties for 'Name', and
add the now-detected StayQuiet diagnostic to the CoreNFC .ignore files.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
rolfbjarne requested a review from dalexsoto as a code owner August 28, 2026 18:42
Copilot AI lite review requested due to automatic review settings August 28, 2026 18:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes xtro-sharpie’s native name lookup for protocol types when the Objective-C protocol name is provided via ProtocolAttribute.Name, so the nullability analysis can correctly match native declarations and emit diagnostics for protocol members (e.g., CoreNFC StayQuiet).

Changes:

  • Update Helpers.GetName (TypeDefinition?) to also read ProtocolAttribute’s named Name property when no constructor argument is present.
  • Record newly-detected CoreNFC block-parameter nullability diagnostics in both iOS and Mac Catalyst CoreNFC ignore lists.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/xtro-sharpie/xtro-sharpie/Helpers.cs Extends protocol native-name resolution to consider ProtocolAttribute.Name.
tests/xtro-sharpie/api-annotations-dotnet/MacCatalyst-CoreNFC.ignore Adds the newly detected INFCIso15693Tag.StayQuiet nullability diagnostic.
tests/xtro-sharpie/api-annotations-dotnet/iOS-CoreNFC.ignore Adds the newly detected INFCIso15693Tag.StayQuiet nullability diagnostic.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/xtro-sharpie/xtro-sharpie/Helpers.cs
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

dalexsoto
dalexsoto previously approved these changes Aug 31, 2026
@vs-mobiletools-engineering-service2

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)

✅ API diff vs stable

NET (empty diffs)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 27ed9c2254d9f00e81b583c0233d88dcd3cbd6dd [PR build]

@rolfbjarne
rolfbjarne enabled auto-merge (squash) September 3, 2026 05:03
@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

🚀 [CI Build #27ed9c2] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 264 tests passed 🎉

Tests counts

✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 7 tests passed. Html Report (VSDrops) Download
✅ linker (iOS): All 31 tests passed. Html Report (VSDrops) Download
✅ linker (MacCatalyst): All 31 tests passed. Html Report (VSDrops) Download
✅ linker (macOS): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (tvOS): All 31 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 25 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 25 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 20 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 25 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

Linux build succeeded

Pipeline on Agent
Hash: 27ed9c2254d9f00e81b583c0233d88dcd3cbd6dd [PR build]

@rolfbjarne
rolfbjarne merged commit 8dd9ff3 into main Sep 4, 2026
54 checks passed
@rolfbjarne
rolfbjarne deleted the dev/rolf/upgraded-potato branch September 4, 2026 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copilot ready-to-review This PR is ready to review/merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants