Uh oh!
There was an error while loading. Please reload this page.
[TrimmableTypeMap] Assembly-level manifest attribute scanning - #11035
Conversation
Add scanning for assembly-level attributes ([Application], [Activity], [Service], [BroadcastReceiver], [ContentProvider], permissions, etc.) and wire them into the manifest generator via Generate(XDocument?) overload. Replace ManifestModel.cs with individual Scanner/ record types for better composability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds assembly-level Android manifest attribute scanning to the TrimmableTypeMap scanner pipeline and wires the collected data into manifest generation (plus adjusts native callback naming expectations in tests).
Changes:
- Introduce new scanner model record types to represent assembly-level manifest concepts (permissions, uses-* entries, app-level metadata/properties).
- Extend
AssemblyIndex/JavaPeerScannerto parse and aggregate assembly-level manifest attributes and attach component attribute info to peers. - Refactor
ManifestGeneratorto support an optional pre-loadedXDocumenttemplate and update tests for the refined native callback naming.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/OverrideDetectionTests.cs | Updates expected native callback name derived from connector signature. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/JcwJavaSourceGeneratorTests.cs | Updates expected generated Java to call/declare the refined native callback name. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesPermissionInfo.cs | New record for <uses-permission> data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesLibraryInfo.cs | New record for <uses-library> data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesFeatureInfo.cs | New record for <uses-feature> data (name vs GL ES variant). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesConfigurationInfo.cs | New record for <uses-configuration> data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PropertyInfo.cs | New record for <property> entries under <application>. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionTreeInfo.cs | New record for <permission-tree> data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionInfo.cs | New record for <permission> data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionGroupInfo.cs | New record for <permission-group> data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/MetaDataInfo.cs | New record for <meta-data> items. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/IntentFilterInfo.cs | New record for intent filter data captured from [IntentFilter]. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/ComponentInfo.cs | New public model for component attribute data used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyManifestInfo.cs | New aggregate container for assembly-level manifest attributes across scanned assemblies. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs | Adds ScanAssemblyAttributes() and parsing for assembly-level attributes; expands type-attribute parsing to capture properties/intent-filters/metadata. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Adds assembly-manifest scanning entry point, component mapping, and improved native callback name derivation from connector strings. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Updates doc comment for ComponentAttribute. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestModel.cs | Deletes legacy shared manifest model (replaced by scanner record types). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs | Adds Generate(XDocument?, ...) overload and refactors template loading/default manifest creation. |
Uh oh!
There was an error while loading. Please reload this page.
… types Add assembly-level manifest attribute scanning to the TrimmableTypeMap scanner: - AssemblyIndex.ScanAssemblyAttributes(): parses 9 assembly-level attribute types (UsesPermission, UsesLibrary, UsesFeature, UsesConfiguration, Permission, PermissionGroup, PermissionTree, MetaData, Property) - JavaPeerScanner.ScanAssemblyManifestInfo(): aggregates attributes across assemblies - JavaPeerScanner.ToComponentInfo()/HasPublicParameterlessCtor(): attaches component info to scanned peers - ManifestGenerator.Generate(XDocument?): new overload accepting pre-loaded template - ManifestGenerator.CreateDefaultManifest(): extracted from LoadOrCreateManifest - Replace ManifestModel.cs (105-line flat class) with 12 focused record types - Update JavaPeerInfo.ComponentAttribute doc comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e enable - Fix ParseAttributes: collect IntentFilter/MetaData in local lists and attach after the loop to avoid creating attrInfo with wrong AttributeName when [IntentFilter] appears before [Activity] - Remove redundant #nullable enable from 13 files (project has <Nullable>enable) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e name UsesFeatureAttribute.Name has a private setter and is set via the (string name) constructor. The parser only read NamedArguments, so the feature name was always null when set via the constructor, causing <uses-feature> entries to be silently skipped. Read FixedArguments[0] first (matching ParseUsesPermissionAttribute and ParseUsesLibraryAttribute), then allow NamedArguments to override. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add stub attributes (UsesFeatureAttribute with private-setter Name, UsesPermissionAttribute, UsesLibraryAttribute, MetaDataAttribute, IntentFilterAttribute) and assembly-level usages in TestFixtures. Expose ScanAssemblyManifestInfo() via FixtureTestBase and add 6 tests in AssemblyAttributeScanningTests covering constructor-arg parsing for UsesFeature, UsesPermission, UsesLibrary, MetaData, and the GLESVersion named-arg-only variant. The UsesFeature_ConstructorArg and UsesFeature_ConstructorArgWithNamedProperty tests are regression tests for the ParseUsesFeatureAttribute fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove leading blank lines from 13 files (leftover from #nullable enable removal) - Remove unnecessary XML doc comments on internal record types - Remove unused HasPublicDefaultConstructor property and HasPublicParameterlessCtor method - Enhance ParseNameAndProperties to also read FixedArguments[0] for constructor-arg names - Refactor individual Parse*Attribute methods into static Create*Info methods that accept pre-parsed (name, props) from ParseNameAndProperties, eliminating duplicated DecodeAttribute calls and NamedArguments iteration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…overload - Move ParseNameAndProperties(ca) call before the switch in ScanAssemblyAttributes so it runs once instead of being duplicated in every case - Inline trivial CreatePermissionInfo/CreatePermissionGroupInfo/CreatePermissionTreeInfo one-liners directly at the call site - Remove the file-path Generate() overload from ManifestGenerator; IO belongs in the caller - Fix leading blank line in ManifestGeneratorTests.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1239c1b to
e93ba6bCompareUsesLibraryAttribute(string, bool) puts Required in FixedArguments[1]. ParseNameAndProperties now stores additional bool ctor args in props so CreateUsesLibraryInfo can read them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e93ba6b to
fd71439Compare
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ✅ LGTM
Found 0 errors, 0 warnings, 1 suggestion.
CI: dotnet-android all 3 legs green (Linux ✅, Mac ✅, Windows ✅). Xamarin.Android-PR pending (rerun queued).
👍 Positive callouts
- Clean shared
ParseNameAndPropertiesentry point withKnownAssemblyAttributesguard — avoids decoding non-manifest attributes FixedArguments[0]read for constructor-arg names (UsesFeature, UsesPermission, UsesLibrary, MetaData, SupportsGLTexture, Property)FixedArguments[1]read for UsesLibrary 2-arg ctor(string, bool)— prevents silentRequiredlossHashSet.Add()pattern for all dedup sets — prevents cross-assembly duplicates- 8 scanner tests covering constructor-arg parsing, named-arg-only variants, and 2-arg ctor regression
- Visibility split is correct:
ComponentInfo/IntentFilterInfo/MetaDataInfopublic (exposed viaJavaPeerInfo), all others internal - No
null!, no#region, nostring.Empty, noArray.Empty<T>(), no leading blank lines
💡 Documentation — PR description is out of date (still mentions removed HasPublicParameterlessCtor/GetNativeCallbackName, doesn't mention bug fixes or tests). Consider updating it before review.
Review generated by android-reviewer from review guidelines.
jonathanpeppers
left a comment
There was a problem hiding this comment.
Overall looks good. I'm rerunning some of the lanes that failed.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Add scanning for assembly-level manifest attributes and wire them into the manifest generator. Replace the shared
ManifestModel.cswith individual Scanner record types and aComponentInfomodel for type-level component attributes.Depends on #10991
Changes
Scanner
AssemblyIndex.cs: AddScanAssemblyAttributes()with sharedParseNameAndProperties()entry point and individualCreate*Infostatic methods for each attribute type. Capture component attribute named properties, intent filters, and metadata on type attributes.JavaPeerScanner.cs: AddScanAssemblyManifestInfo(),ToComponentInfo(),ParseConnectorDeclaringType(). WireComponentAttributeinto peer creation.AssemblyManifestInfo,ComponentInfo,IntentFilterInfo,MetaDataInfo,PermissionInfo,PermissionGroupInfo,PermissionTreeInfo,PropertyInfo,UsesConfigurationInfo,UsesFeatureInfo,UsesLibraryInfo,UsesPermissionInfoManifestModel.cs— replaced by the above recordsGenerator
ManifestGenerator.cs: Remove file-pathGenerate()overload — IO belongs in the caller. Keep only theXDocument?overload.Bug fixes
ParseUsesFeatureAttribute: ReadFixedArguments[0]for the feature name (constructor arg with private setter)UsesLibrary2-arg ctor: StoreFixedArguments[1](bool) in props soRequired=falseis not silently lostRefactoring
ParseNameAndProperties(ca)before the switch inScanAssemblyAttributesCreatePermissionInfo/CreatePermissionGroupInfo/CreatePermissionTreeInfoHasPublicDefaultConstructorproperty andHasPublicParameterlessCtormethod#nullable enableremoval)Tests
[assembly: ...]declarations inTestFixtures/AssemblyAttributes.csScanAssemblyManifestInfo()viaFixtureTestBaseAssemblyAttributeScanningTestswith 8 scanner tests covering constructor-arg parsing, named-arg-only variants, and the 2-arg ctor