[Trimming] Use resource designer Type in ResourceIdManager#11439
Open
simonrozsival wants to merge 26 commits into
Open
[Trimming] Use resource designer Type in ResourceIdManager#11439simonrozsival wants to merge 26 commits into
simonrozsival wants to merge 26 commits into
Conversation
Emit ResourceDesignerAttribute with typeof(Resource) for app resource designers so ResourceIdManager can use the attribute-provided Type instead of Assembly.GetType(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates how the runtime locates the application Resource designer type by introducing a Type-based ResourceDesignerAttribute API and emitting typeof(Resource) in generated designer code, allowing ResourceIdManager to avoid string-based reflection.
Changes:
- Added
ResourceDesignerAttribute(Type resourceType)plusResourceTypeproperty, and updated PublicAPI baselines. - Updated resource designer generation to emit
typeof(global::...Resource)for application designers (while keeping string form for libraries). - Updated
ResourceIdManagerto use the newResourceTypeproperty.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Utilities/ResourceDesignerImportGenerator.cs | Updates comments/documentation for how library designer attributes are discovered. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Expected/GenerateDesignerFileWithLibraryReferenceExpected.cs | Updates expected generated designer output to use typeof(Resource) for application designers. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Expected/GenerateDesignerFileWithElevenStyleableAttributesExpected.cs | Updates expected generated designer output to use typeof(Resource) for application designers. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Expected/GenerateDesignerFileExpected.cs | Updates expected generated designer output to use typeof(Resource) for application designers. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesigner.cs | Emits typeof(Resource) for application assemblies and a string type name for library assemblies. |
| src/Mono.Android/PublicAPI/API-37/PublicAPI.Unshipped.txt | Records new ResourceDesignerAttribute(Type) and ResourceType API as unshipped. |
| src/Mono.Android/PublicAPI/API-36/PublicAPI.Unshipped.txt | Records new ResourceDesignerAttribute(Type) and ResourceType API as unshipped. |
| src/Mono.Android/PublicAPI/API-36.1/PublicAPI.Unshipped.txt | Records new ResourceDesignerAttribute(Type) and ResourceType API as unshipped. |
| src/Mono.Android/PublicAPI/API-35/PublicAPI.Unshipped.txt | Records new ResourceDesignerAttribute(Type) and ResourceType API as unshipped. |
| src/Mono.Android/Android.Runtime/ResourceIdManager.cs | Switches application resource designer lookup from Assembly.GetType(FullName) to ResourceDesignerAttribute.ResourceType. |
| src/Mono.Android/Android.Runtime/ResourceDesignerAttribute.cs | Adds a Type-based constructor and ResourceType property while retaining the string-based constructor. |
simonrozsival
changed the title
Mark the legacy string-based ResourceDesignerAttribute constructor as requiring dynamic code, and emit the Type-based constructor from generated resource designers so internal code avoids the dynamic-code path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the legacy string constructor behavior in ResourceDesignerAttribute, but route it through a dynamic-code-marked constructor and a localized Assembly.GetType fallback. The Type constructor remains the safe path used by generated resource designers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When ResourceDesignerAttribute is constructed with a Type, ignore assemblies other than the resource type's declaring assembly instead of asserting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The legacy string-based ResourceDesignerAttribute path reaches Assembly.GetType(string), which is annotated RequiresUnreferencedCode. Mark the compatibility constructor accordingly while keeping the Type constructor as the safe generated path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Only return from ResourceIdManager when ResourceDesignerAttribute resolves a non-null resource type, allowing mismatched Type-based attributes to be skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ResourceDesignerAttribute may now be generated with typeof(Resource). When importing resource designer attributes from metadata, use Type.FullName for Type fixed arguments while preserving legacy string arguments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Decode ResourceDesignerAttribute type arguments with a local provider so DummyCustomAttributeProvider keeps its existing behavior while resource designer imports still handle typeof(Resource). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the local ResourceDesignerAttributeTypeProvider and read the serialized fixed attribute argument directly so both string and typeof(Resource) constructors are handled without changing the shared dummy provider. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document why reading the first fixed argument as a serialized string works for both string and System.Type ResourceDesignerAttribute constructors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a build-task test proving ResourceDesignerImportGenerator imports library resources when ResourceDesignerAttribute is emitted with typeof(Resource), and remove the experimental provider strategy from ResourceDesignerAttribute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
added
the
copilot
Member
|
/review |
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM — Clean trimming improvement
Summary: This PR adds a Type-based constructor to ResourceDesignerAttribute so the resource designer can pass typeof(Resource) instead of a string name, enabling the trimmer to statically track the type reference and eliminating Assembly.GetType() at runtime. The old string-based constructor is preserved with [RequiresUnreferencedCode] for backward compatibility.
What I verified:
- ✅ The strategy pattern (
IResourceTypeProvider) cleanly separates the two constructor paths without code duplication - ✅
DynamicallyAccessedMembersannotations are correctly propagated through the type hierarchy - ✅ The raw blob parsing in
ResourceDesignerImportGeneratorcorrectly handles both old (string) and new (typeof) custom attribute encodings, since both are SerString-encoded in ECMA-335 - ✅
LinkDesignerBase.FindResourceDesigner(Cecil-based, not modified) continues to work because Cecil'sTypeReference.ToString()returns the namespace-qualified name, matching the old string format - ✅
TypeResourceTypeProvider.GetResourceTypeFromAssemblyhas a sensible assembly equality check preventing cross-assembly type mismatches - ✅ New test covers the import generator decoding path for the typeof-based attribute
- ✅ PublicAPI files updated for all API levels
- ✅ CI is green
Issue counts: 0 ❌ errors · 1
Generated by Android PR Reviewer for issue #11439 · ● 3.2M
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Decode System.Type custom attribute arguments through DummyCustomAttributeProvider so ResourceDesignerImportGenerator can use the shared GetCustomAttributeArguments() helper for both typeof(Resource) and string ResourceDesignerAttribute constructors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename the private metadata decoder marker to systemTypeSentinel so it is clear the value is only an identity token for DecodeValue() type checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit c42f89d.
This reverts commit 99877b0.
Document that legacy ResourceDesignerAttribute string values may be namespace-qualified and suppress the expected IL2026 in the AndroidX NativeAOT code-behind test until transitive binding packages rebuild with the Type constructor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
✅ LGTM — Clean, well-structured trimming improvement
Summary: This PR makes ResourceDesignerAttribute trim-safe by generating assembly-qualified names ("Foo.Resource, Foo") instead of bare type names, enabling Type.GetType() to resolve types without trim-unsafe Assembly.GetType() calls. The fallback path for legacy non-AQN bindings is properly preserved with appropriate trim warning suppressions.
Strengths:
- Clean separation: type resolution logic moved into the attribute itself, simplifying
ResourceIdManager - Backward-compatible: old non-AQN format still works via the fallback path
GetTypeFullNameFromAssemblyQualifiedNameis simple, correct, and reused in bothResourceDesignerImportGeneratorandLinkDesignerBase- Good test coverage: expected files updated, new test verifies AQN round-trip through the import generator
- Well-documented temporary
IL2122suppression with clear plan for removal
Minor suggestions only (3 💡): inconsistent brace style, cache field effectiveness note, and edge-case test idea. No blocking issues found.
| Severity | Count |
|---|---|
| ❌ Error | 0 |
| 0 | |
| 💡 Suggestion | 3 |
Generated by Android PR Reviewer for issue #11439 · ● 11.7M
Refactored FullName property to use auto-implemented property syntax and updated GetResourceTypeFromAssembly method logic.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
added
the
ready-to-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ResourceDesignerAttributeconstructor and use it for newly generated resource designer attributesXamarin.Kotlin.Resource, not assembly-qualified names that can satisfy DAM preservationResourceIdManagerto resolve resource designer types through the attribute provider instead of directly callingAssembly.GetType()typeof(Resource)or stringResourceDesignerAttributemetadata continue to import IDsIL2026in the AndroidX code-behind test until transitive binding packages rebuild with the Type constructorPart of #10794.
Binding-library follow-up
Xamarin.AndroidX.AppCompat 1.6.1.6pullsXamarin.Kotlin.StdLib 1.9.21.1.net6.0-android31.0andnet7.0-android33.0Kotlin assemblies contain[assembly: ResourceDesigner("Xamarin.Kotlin.Resource", IsApplication = false)].[DynamicallyAccessedMembers]on the string parameter cannot make the legacy constructor trim-safe.ResourceDesignerAttribute(typeof(...)).IL2026suppression fromCodeBehindTests.Validation
RUNNINGONCI=true ./dotnet-local.sh test bin/TestDebug/net10.0/Xamarin.Android.Build.Tests.dll --filter Name~SuccessfulAndroidXAppXamarin.Kotlin.StdLib 1.9.21.1with ILSpy to confirm the legacy string value isXamarin.Kotlin.Resource