Skip to content

[sharpie] Emit correct nullability for block/delegate parameters#25837

Merged
rolfbjarne merged 17 commits into
mainfrom
dev/rolf/sharpie-block-nullability
Jun 30, 2026
Merged

[sharpie] Emit correct nullability for block/delegate parameters#25837
rolfbjarne merged 17 commits into
mainfrom
dev/rolf/sharpie-block-nullability

Conversation

@rolfbjarne

@rolfbjarne rolfbjarne commented Jun 26, 2026

Copy link
Copy Markdown
Member

When ObjC declares a block with nullable parameters (e.g., void (^)(BOOL success, NSError * _Nullable error)), sharpie previously produced Action<bool, NSError>, losing the nullability information. This PR teaches sharpie to produce Action<bool, NSError?> instead.

Approach

The fix operates at two levels in the sharpie pipeline:

  • TypeBinder.VisitFunctionType -- After resolving each block parameter type, checks for CX_AttrKind_TypeNullable in the AttributedType annotations (which clang already attaches). If found, wraps the resolved type with ComposedType.HasNullableSpecifier = true to produce the ? suffix in type arguments. This handles the inline Action<>/Func<> case.

  • CustomDelegateMassager + BindingGenerator -- When converting inline delegates to named delegate declarations, unwraps the nullable ComposedType back to its base type while preserving annotations. This allows the existing NullabilityMassager to add [NullAllowed] to the delegate parameter (the correct representation for binding definition delegates).

xtro-sharpie validation

Extends NullabilityCheck to validate block parameter nullability:

  • Detects BlockPointerType parameters and inspects the underlying FunctionProtoType
  • For each block parameter, compares native nullability against the managed NullableAttribute byte array positions
  • Reports !missing-null-allowed! / !extra-null-allowed! for mismatches

This addresses the existing TODO in NullabilityCheck: "check the generics (don't think we have such cases yet)".

🤖 Pull request created by Copilot

rolfbjarne and others added 7 commits June 26, 2026 17:44
@rolfbjarne
[sharpie] Emit nullable type arguments for block parameters
d21740a 
When ObjC declares a block with nullable parameters (e.g.,
void (^)(BOOL, NSError * _Nullable)), sharpie now produces
Action<bool, NSError?> instead of Action<bool, NSError>.

This also applies to the return type of Func<> delegates.

The logic checks for CX_AttrKind_TypeNullable annotations on
resolved types and wraps them with ComposedType.HasNullableSpecifier
to produce the '?' suffix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[sharpie] Unwrap nullable types when creating delegate declarations
221e9cd 
When CustomDelegateMassager or BindingGenerator convert inline
Action<>/Func<> types to named delegate declarations, unwrap
nullable ComposedTypes (T?) back to their base type. This preserves
the annotations so NullabilityMassager can add [NullAllowed] to
the delegate parameter instead, which is the correct representation
for binding definition delegate parameters.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[xtro-sharpie] Validate nullability of block parameter types
4310a64 
Extend the NullabilityCheck to inspect block/delegate parameters
for nullability mismatches between native and managed code.

When a method parameter is a block type (e.g., void (^)(BOOL, NSError *)),
the check now validates nullability of each block parameter against
the NullableAttribute byte array on the managed generic type arguments.

Reports !missing-null-allowed! and !extra-null-allowed! for block
parameter nullability mismatches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[sharpie] Emit nullable type arguments for block parameters
6e20d0c 
When ObjC declares a block with nullable parameters (e.g.,
void (^)(BOOL, NSError * _Nullable)), sharpie now produces
Action<bool, NSError?> instead of Action<bool, NSError>.

This also applies to the return type of Func<> delegates.

The logic checks for CX_AttrKind_TypeNullable annotations on
resolved types and wraps them with ComposedType.HasNullableSpecifier
to produce the '?' suffix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[sharpie] Unwrap nullable types when creating delegate declarations
7359629 
When CustomDelegateMassager or BindingGenerator convert inline
Action<>/Func<> types to named delegate declarations, unwrap
nullable ComposedTypes (T?) back to their base type. This preserves
the annotations so NullabilityMassager can add [NullAllowed] to
the delegate parameter instead, which is the correct representation
for binding definition delegate parameters.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[xtro-sharpie] Validate nullability of block parameter types
284aeaf 
Extend the NullabilityCheck to inspect block/delegate parameters
for nullability mismatches between native and managed code.

When a method parameter is a block type (e.g., void (^)(BOOL, NSError *)),
the check now validates nullability of each block parameter against
the NullableAttribute byte array on the managed generic type arguments.

Reports !missing-null-allowed! and !extra-null-allowed! for block
parameter nullability mismatches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
Merge commit '4310a642f780743e5a47a00fe951376ee4b273d1' into dev/rolf…
898c0bb 
…/sharpie-block-nullability
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
rolfbjarne and others added 3 commits June 29, 2026 16:09
@rolfbjarne
[sharpie] Update expected test output for block parameter nullability
d57ce70 
Update the Nullability test expected output files to reflect the new
nullable type arguments in block parameters (Action<NSObject?> instead
of Action<NSObject>).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[xtro-sharpie] Add block parameter nullability entries to .todo files
9afc2d5 
Classify all 2003 block parameter nullability mismatches as known
issues. Also change the error message for block parameter nullability
to use '?' instead of '[NullAllowed]' since the fix is adding a
nullable type argument (e.g. NSError?) rather than an attribute.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
Merge remote-tracking branch 'origin/main' into dev/rolf/sharpie-bloc…
3b67650 
…k-nullability
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@rolfbjarne rolfbjarne marked this pull request as ready for review June 30, 2026 06:08
Copilot AI review requested due to automatic review settings June 30, 2026 06:08
Copilot AI reviewed Jun 30, 2026
View reviewed changes

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 improves sharpie’s nullability fidelity for Objective-C blocks by preserving _Nullable annotations on block/delegate parameter types, emitting Action<..., T?>/Func<..., T?> for inline delegates, and extending xtro-sharpie validation to detect nullability mismatches inside block parameters.

Changes:

  • Update TypeBinder.VisitFunctionType to wrap block parameter/return generic arguments as nullable (T?) when clang’s TypeNullable attribute is present.
  • Preserve nullability annotations when converting inline delegates into named delegate declarations so NullabilityMassager can emit [NullAllowed] on delegate parameters.
  • Extend xtro-sharpie NullabilityCheck to validate nullability for block parameter inner parameters, and update api-annotations ignore baselines accordingly.

Reviewed changes

Copilot reviewed 180 out of 180 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/sharpie/Sharpie.Bind/TypeBinder.cs Detects TypeNullable on block parameter/return types and emits nullable AstType (?) for inline Action<>/Func<> generic arguments.
tools/sharpie/Sharpie.Bind/Massagers/CustomDelegateMassager.cs Unwraps T? back to T (while preserving annotations) when materializing named delegates so [NullAllowed] can be applied to delegate parameters.
tools/sharpie/Sharpie.Bind/BindingGenerator.cs Applies the same nullable-unwrapping logic when generating delegate declarations from block pointer typedefs.
tests/xtro-sharpie/xtro-sharpie/NullabilityCheck.cs Adds validation that compares native block-parameter nullability vs managed NullableAttribute positions for Action<>/Func<> type arguments.
tests/xtro-sharpie/api-annotations-dotnet/*.ignore (multiple files) Baseline updates for newly-detected block-parameter nullability discrepancies across platforms/frameworks.
tests/sharpie/Tests/Nullability.macosx.cs Updates expected output to Action<NSObject?> for a block whose parameter is declared _Nullable.
tests/sharpie/Tests/Nullability.iphoneos.cs Same as macOS test: expects Action<NSObject?> for nullable block parameter.

Comment thread tests/xtro-sharpie/xtro-sharpie/NullabilityCheck.cs
Comment thread tests/xtro-sharpie/api-annotations-dotnet/common-UserNotifications.ignore
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@rolfbjarne
[xtro-sharpie] Remove unused variable managedTypeArgResolved
f76d673 
Fixes CS0219 warning that would be promoted to an error by
TreatWarningsAsErrors.

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

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
rolfbjarne and others added 5 commits June 30, 2026 11:44
@rolfbjarne
[xtro-sharpie] Move common block nullability entries to common-*.ignore
ba073b3 
Entries that are identical across all four platforms (iOS, macOS, tvOS,
MacCatalyst) are moved from the platform-specific .ignore files into
the corresponding common-*.ignore file to reduce duplication.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[xtro-sharpie] Remove leftover .tmp files
6d71de0 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[xtro-sharpie] Remove empty platform-specific .ignore files
dda6b72 
Delete platform-specific .ignore files that have no remaining entries
after moving common entries to common-*.ignore.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
[xtro-sharpie] Remove duplicate comment lines in common-*.ignore
ce3b3b7 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne
Merge remote-tracking branch 'origin/main' into dev/rolf/sharpie-bloc…
2dce399 
…k-nullability
@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

✅ [PR Build #2dce399] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 2dce399110027b52f4174add5d56fb65806a5311 [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

✅ [PR Build #2dce399] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 2dce399110027b52f4174add5d56fb65806a5311 [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jun 30, 2026

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: 2dce399110027b52f4174add5d56fb65806a5311 [PR build]

@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

✅ [PR Build #2dce399] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 2dce399110027b52f4174add5d56fb65806a5311 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

Sign in to view
@rolfbjarne rolfbjarne mentioned this pull request Jun 30, 2026
Open
@rolfbjarne rolfbjarne enabled auto-merge (squash) June 30, 2026 15:03
@vs-mobiletools-engineering-service2

vs-mobiletools-engineering-service2 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

🚀 [CI Build #2dce399] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 199 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 4 tests passed. Html Report (VSDrops) Download
✅ linker (iOS): All 15 tests passed. Html Report (VSDrops) Download
✅ linker (MacCatalyst): All 15 tests passed. Html Report (VSDrops) Download
✅ linker (macOS): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (tvOS): All 15 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 18 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 17 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 18 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 18 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 Monterey (12): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Ventura (13): All 5 tests passed. Html Report (VSDrops) Download
✅ 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: 2dce399110027b52f4174add5d56fb65806a5311 [PR build]

@rolfbjarne rolfbjarne merged commit 8cbe6f9 into main Jun 30, 2026
57 checks passed
@rolfbjarne rolfbjarne deleted the dev/rolf/sharpie-block-nullability branch June 30, 2026 15:49
jonathanpeppers pushed a commit that referenced this pull request Jun 30, 2026
@rolfbjarne @jonathanpeppers
[sharpie] Emit correct nullability for block/delegate parameters (#25837
2c98b3f 
)

When ObjC declares a block with nullable parameters (e.g., `void (^)(BOOL success, NSError * _Nullable error)`), sharpie previously produced `Action<bool, NSError>`, losing the nullability information. This PR teaches sharpie to produce `Action<bool, NSError?>` instead.

## Approach

The fix operates at two levels in the sharpie pipeline:

- **TypeBinder.VisitFunctionType** -- After resolving each block parameter type, checks for `CX_AttrKind_TypeNullable` in the `AttributedType` annotations (which clang already attaches). If found, wraps the resolved type with `ComposedType.HasNullableSpecifier = true` to produce the `?` suffix in type arguments. This handles the inline `Action<>`/`Func<>` case.

- **CustomDelegateMassager + BindingGenerator** -- When converting inline delegates to named delegate declarations, unwraps the nullable `ComposedType` back to its base type while preserving annotations. This allows the existing `NullabilityMassager` to add `[NullAllowed]` to the delegate parameter (the correct representation for binding definition delegates).

## xtro-sharpie validation

Extends `NullabilityCheck` to validate block parameter nullability:

- Detects `BlockPointerType` parameters and inspects the underlying `FunctionProtoType`
- For each block parameter, compares native nullability against the managed `NullableAttribute` byte array positions
- Reports `!missing-null-allowed!` / `!extra-null-allowed!` for mismatches

This addresses the existing TODO in `NullabilityCheck`: "check the generics (don't think we have such cases yet)".

🤖 Pull request created by Copilot

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Rolf Bjarne Kvinge <rokvin@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@dalexsoto dalexsoto dalexsoto approved these changes

Assignees

No one assigned

Labels

copilot

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rolfbjarne @vs-mobiletools-engineering-service2 @dalexsoto