[apidiff] Add nullability support to mono-api-info and mono-api-html#25603
Conversation
mono-api-info now reads NullableAttribute and NullableContextAttribute metadata from assemblies and appends '?' to type names for nullable reference types in the XML output (parameters, return types, properties, fields, and events). mono-api-html now: - Strips nullability annotations when matching methods (so nullability- only changes don't cause false removed/added entries) - Detects nullability-only changes and renders them under a separate '(nullability)' subsection header to indicate they are non-breaking - Handles the '?' suffix in type name resolution (GetTypeName) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds nullable reference type awareness to the apidiff tooling. mono-api-info now reads NullableAttribute / NullableContextAttribute metadata and emits a trailing ? on type names for nullable reference types (parameters, return types, properties, fields, events). mono-api-html strips these annotations for member matching to avoid false add/remove pairs and groups nullability-only differences under a separate (nullability) subsection so they are clearly identified as non-breaking.
Changes:
- mono-api-info: new
NullabilityHelperreadsNullableAttribute/NullableContextAttributeand appends?to nullable reference type names. - mono-api-html:
Helper.StripNullability/DiffersOnlyByNullability, plusGetTypeNamehandling of the trailing?. - mono-api-html:
MemberComparer.Modifysplits modifications into regular and(nullability)buckets;MethodComparer.Findmatches return types ignoring nullability.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/api-tools/mono-api-info/mono-api-info.cs | Emits ? suffix on nullable reference type names for fields, properties, events, return types and parameters via new NullabilityHelper. |
| tools/api-tools/mono-api-html/Helpers.cs | GetTypeName recognizes trailing ?; new StripNullability / DiffersOnlyByNullability helpers. |
| tools/api-tools/mono-api-html/MethodComparer.cs | Compare return types after stripping nullability so nullability-only changes don't break method matching. |
| tools/api-tools/mono-api-html/MemberComparer.cs | Splits modified members into normal and (nullability) sections. |
| tools/api-tools/mono-api-html/ApiChange.cs | Flags an ApiChange as nullability-only by recursively comparing source/target attributes and children. |
| static byte? GetNullableContextFromParent (ICustomAttributeProvider? provider) | ||
| { | ||
| if (provider is MethodDefinition method) | ||
| return GetNullableContextFlag (method.DeclaringType); | ||
| if (provider is PropertyDefinition prop) | ||
| return GetNullableContextFlag (prop.DeclaringType); | ||
| if (provider is FieldDefinition field) | ||
| return GetNullableContextFlag (field.DeclaringType); | ||
| if (provider is EventDefinition evt) | ||
| return GetNullableContextFlag (evt.DeclaringType); | ||
| if (provider is TypeDefinition type && type.DeclaringType is not null) | ||
| return GetNullableContextFlag (type.DeclaringType); | ||
| return null; | ||
| } |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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.
Instead of highlighting the entire type name as modified, only the added or removed '?' character is rendered with the change span. For example: NSRunLoopMode[]<span class='added'>?</span> (added nullability) NSRunLoopMode[]<span class='removed-inline'>?</span> (removed nullability) Dictionary<string<span class='removed-inline'>?</span>, string<span class='added'>?</span>> (mixed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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 reverts commit 24d1d58.
|
/review |
|
✅ .NET for Apple Platforms PR Reviewer completed successfully! |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Review Summary
This PR adds nullability annotation support to the API diff tooling — a valuable improvement for tracking C# nullable reference type changes.
⚠️ Issues Found
Performance (3 warnings)
- Character-by-character string allocations in
AppendNullabilityModified(lines 67, 92, 102). These allocations add up when processing large API surfaces. Append chars directly or batch substrings.
Correctness (1 warning)
IsNullabilitySuffixonly checks the first character of HTML entities like&and>, which may not correctly identify nullability suffixes in HTML-encoded contexts.
Code organization (2 suggestions)
- Potential duplication between XElement-level and string-level
DiffersOnlyByNullabilitymethods - Incomplete comment about generic parameter handling in
IsNullableReferenceType
✅ Strengths
- Clean separation: Nullability-only changes are rendered under "(nullability)" subsections, clearly distinguishing them from breaking changes
- Backward compatible: Proper stripping of nullability annotations during method matching prevents false positive removed/added entries
- Comprehensive coverage: Handles fields, properties, events, method return types, and parameters
- Well-structured: The
NullabilityHelperclass provides clear single-purpose methods with good separation of concerns
Verdict
The core logic is sound and the feature works as described. The performance concerns are worth addressing to ensure good behavior on large assemblies, and the HTML entity check should be validated. Otherwise, this is a solid enhancement to the API diff infrastructure.
CI is still running — ensure all checks pass before merge.
Generated by .NET for Apple Platforms PR Reviewer for issue #25603 · sonnet45 1.1M
Comment /review to run again
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Fix DiffersOnlyByNullability to always check children even when parent attributes differ by nullability (prevents misclassifying non-nullability child changes as nullability-only) - Add module-level NullableContextAttribute fallback for types that rely on assembly-wide nullable context - Fix per-character string allocations: add TextChunk.Append(char) overload and use it instead of ToString() per character - Improve IsNullabilitySuffix comment to document HTML entity assumption - Fix misleading comment in IsNullableReferenceType Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
✅ [PR Build #ec909e3] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #ec909e3] 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 |
✅ [PR Build #ec909e3] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #ec909e3] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 193 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
mono-api-info now reads NullableAttribute and NullableContextAttribute
metadata from assemblies and appends '?' to type names for nullable
reference types in the XML output (parameters, return types, properties,
fields, and events).
mono-api-html now:
only changes don't cause false removed/added entries)
'(nullability)' subsection header to indicate they are non-breaking