Skip to content

Commit 9822cf0

Browse files
[TrimmableTypeMap][Core B] Apply review refactors in AssemblyIndex
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5d2bb7b commit 9822cf0

File tree

1 file changed

+105
-97
lines changed

1 file changed

+105
-97
lines changed

src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs

Lines changed: 105 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ public static AssemblyIndex Create (string filePath)
6767

6868
void Build ()
6969
{
70-
FindJniNameProviderAttributes ();
71-
7270
foreach (var typeHandle in Reader.TypeDefinitions) {
7371
var typeDef = Reader.GetTypeDefinition (typeHandle);
7472

@@ -81,53 +79,16 @@ void Build ()
8179

8280
var (registerInfo, attrInfo) = ParseAttributes (typeDef);
8381

84-
if (attrInfo != null) {
82+
if (attrInfo is not null) {
8583
AttributesByType [typeHandle] = attrInfo;
8684
}
8785

88-
if (registerInfo != null) {
86+
if (registerInfo is not null) {
8987
RegisterInfoByType [typeHandle] = registerInfo;
9088
}
9189
}
9290
}
9391

94-
/// <summary>
95-
/// Finds all types in this assembly that implement <c>Java.Interop.IJniNameProviderAttribute</c>.
96-
/// </summary>
97-
void FindJniNameProviderAttributes ()
98-
{
99-
foreach (var typeHandle in Reader.TypeDefinitions) {
100-
var typeDef = Reader.GetTypeDefinition (typeHandle);
101-
if (ImplementsIJniNameProviderAttribute (typeDef)) {
102-
var name = Reader.GetString (typeDef.Name);
103-
JniNameProviderAttributes.Add (name);
104-
}
105-
}
106-
}
107-
108-
bool ImplementsIJniNameProviderAttribute (TypeDefinition typeDef)
109-
{
110-
foreach (var implHandle in typeDef.GetInterfaceImplementations ()) {
111-
var impl = Reader.GetInterfaceImplementation (implHandle);
112-
if (impl.Interface.Kind == HandleKind.TypeReference) {
113-
var typeRef = Reader.GetTypeReference ((TypeReferenceHandle)impl.Interface);
114-
var name = Reader.GetString (typeRef.Name);
115-
var ns = Reader.GetString (typeRef.Namespace);
116-
if (name == "IJniNameProviderAttribute" && ns == "Java.Interop") {
117-
return true;
118-
}
119-
} else if (impl.Interface.Kind == HandleKind.TypeDefinition) {
120-
var ifaceTypeDef = Reader.GetTypeDefinition ((TypeDefinitionHandle)impl.Interface);
121-
var name = Reader.GetString (ifaceTypeDef.Name);
122-
var ns = Reader.GetString (ifaceTypeDef.Namespace);
123-
if (name == "IJniNameProviderAttribute" && ns == "Java.Interop") {
124-
return true;
125-
}
126-
}
127-
}
128-
return false;
129-
}
130-
13192
/// <summary>
13293
/// Sets the merged set of JNI name provider attributes from all loaded assemblies
13394
/// and re-classifies any attributes that weren't recognized in the initial pass.
@@ -138,9 +99,10 @@ public void ReclassifyAttributes (HashSet<string> mergedJniNameProviders)
13899

139100
foreach (var typeHandle in Reader.TypeDefinitions) {
140101
var typeDef = Reader.GetTypeDefinition (typeHandle);
102+
AttributesByType.TryGetValue (typeHandle, out var existing);
141103

142104
// Skip types that already have component attribute info
143-
if (AttributesByType.TryGetValue (typeHandle, out var existing) && existing.HasComponentAttribute) {
105+
if (existing is not null) {
144106
continue;
145107
}
146108

@@ -149,16 +111,16 @@ public void ReclassifyAttributes (HashSet<string> mergedJniNameProviders)
149111
var ca = Reader.GetCustomAttribute (caHandle);
150112
var attrName = GetCustomAttributeName (ca, Reader);
151113

152-
if (attrName == null || attrName == "RegisterAttribute" || attrName == "ExportAttribute") {
114+
if (attrName is null || attrName == "RegisterAttribute" || attrName == "ExportAttribute") {
153115
continue;
154116
}
155117

156118
if (mergedJniNameProviders.Contains (attrName) && !IsKnownComponentAttribute (attrName)) {
157119
var componentName = TryGetNameProperty (ca);
158-
if (componentName != null) {
159-
var attrInfo = existing ?? new TypeAttributeInfo ();
160-
attrInfo.HasComponentAttribute = true;
161-
attrInfo.ComponentAttributeJniName = componentName.Replace ('.', '/');
120+
if (componentName is not null) {
121+
var attrInfo = new JniNameProviderAttributeInfo (attrName) {
122+
JniName = componentName.Replace ('.', '/'),
123+
};
162124
AttributesByType [typeHandle] = attrInfo;
163125
}
164126
}
@@ -174,14 +136,14 @@ internal static string GetFullName (TypeDefinition typeDef, MetadataReader reade
174136
if (typeDef.IsNested) {
175137
var declaringType = reader.GetTypeDefinition (typeDef.GetDeclaringType ());
176138
var parentName = GetFullName (declaringType, reader);
177-
return parentName + "+" + name;
139+
return $"{parentName}+{name}";
178140
}
179141

180142
if (ns.Length == 0) {
181143
return name;
182144
}
183145

184-
return ns + "." + name;
146+
return $"{ns}.{name}";
185147
}
186148

187149
(RegisterInfo? register, TypeAttributeInfo? attrs) ParseAttributes (TypeDefinition typeDef)
@@ -193,24 +155,23 @@ internal static string GetFullName (TypeDefinition typeDef, MetadataReader reade
193155
var ca = Reader.GetCustomAttribute (caHandle);
194156
var attrName = GetCustomAttributeName (ca, Reader);
195157

196-
if (attrName == null) {
158+
if (attrName is null) {
197159
continue;
198160
}
199161

200162
if (attrName == "RegisterAttribute") {
201163
registerInfo = ParseRegisterAttribute (ca, customAttributeTypeProvider);
202164
} else if (attrName == "ExportAttribute") {
203-
// [Export] methods are detected per-method in CollectMarshalMethods
165+
// [Export] methods are not handled yet and supporting them will be implemented later
204166
} else if (IsJniNameProviderAttribute (attrName)) {
205-
attrInfo ??= new TypeAttributeInfo ();
206-
attrInfo.HasComponentAttribute = true;
167+
attrInfo ??= CreateTypeAttributeInfo (attrName);
207168
var componentName = TryGetNameProperty (ca);
208-
if (componentName != null) {
209-
attrInfo.ComponentAttributeJniName = componentName.Replace ('.', '/');
169+
if (componentName is not null) {
170+
attrInfo.JniName = componentName.Replace ('.', '/');
210171
}
211-
if (attrName == "ApplicationAttribute") {
212-
attrInfo.ApplicationBackupAgent = TryGetTypeProperty (ca, "BackupAgent");
213-
attrInfo.ApplicationManageSpaceActivity = TryGetTypeProperty (ca, "ManageSpaceActivity");
172+
if (attrInfo is ApplicationAttributeInfo applicationAttributeInfo) {
173+
applicationAttributeInfo.BackupAgent = TryGetTypeProperty (ca, "BackupAgent");
174+
applicationAttributeInfo.ManageSpaceActivity = TryGetTypeProperty (ca, "ManageSpaceActivity");
214175
}
215176
}
216177
}
@@ -230,7 +191,7 @@ bool IsJniNameProviderAttribute (string attrName)
230191
return true;
231192
}
232193

233-
if (allJniNameProviderAttributes != null && allJniNameProviderAttributes.Contains (attrName)) {
194+
if (allJniNameProviderAttributes is not null && allJniNameProviderAttributes.Contains (attrName)) {
234195
return true;
235196
}
236197

@@ -239,6 +200,19 @@ bool IsJniNameProviderAttribute (string attrName)
239200
return IsKnownComponentAttribute (attrName);
240201
}
241202

203+
static TypeAttributeInfo CreateTypeAttributeInfo (string attrName)
204+
{
205+
return attrName switch {
206+
"ActivityAttribute" => new ActivityAttributeInfo (),
207+
"ServiceAttribute" => new ServiceAttributeInfo (),
208+
"BroadcastReceiverAttribute" => new BroadcastReceiverAttributeInfo (),
209+
"ContentProviderAttribute" => new ContentProviderAttributeInfo (),
210+
"ApplicationAttribute" => new ApplicationAttributeInfo (),
211+
"InstrumentationAttribute" => new InstrumentationAttributeInfo (),
212+
_ => new JniNameProviderAttributeInfo (attrName),
213+
};
214+
}
215+
242216
static bool IsKnownComponentAttribute (string attrName)
243217
{
244218
return attrName == "ActivityAttribute"
@@ -350,7 +324,7 @@ public void Dispose ()
350324
}
351325

352326
/// <summary>
353-
/// Parsed [Register] or [Export] attribute data for a type or method.
327+
/// Parsed [Register] attribute data for a type or method.
354328
/// </summary>
355329
sealed class RegisterInfo
356330
{
@@ -359,55 +333,89 @@ sealed class RegisterInfo
359333
public string? Connector { get; }
360334
public bool DoNotGenerateAcw { get; }
361335

362-
/// <summary>
363-
/// For [Export] methods: Java exception type names the method declares it can throw.
364-
/// </summary>
365-
public IReadOnlyList<string>? ThrownNames { get; }
366-
367-
/// <summary>
368-
/// For [Export] methods: super constructor arguments string.
369-
/// </summary>
370-
public string? SuperArgumentsString { get; }
371-
372-
public RegisterInfo (string jniName, string? signature, string? connector, bool doNotGenerateAcw,
373-
IReadOnlyList<string>? thrownNames = null, string? superArgumentsString = null)
336+
public RegisterInfo (string jniName, string? signature, string? connector, bool doNotGenerateAcw)
374337
{
375338
JniName = jniName;
376339
Signature = signature;
377340
Connector = connector;
378341
DoNotGenerateAcw = doNotGenerateAcw;
379-
ThrownNames = thrownNames;
380-
SuperArgumentsString = superArgumentsString;
381342
}
382343
}
383344

384345
/// <summary>
385-
/// Aggregated attribute information for a type, beyond [Register].
346+
/// Parsed [Export] attribute data for a method.
386347
/// </summary>
387-
sealed class TypeAttributeInfo
348+
sealed class ExportInfo
388349
{
389-
/// <summary>
390-
/// Type has [Activity], [Service], [BroadcastReceiver], [ContentProvider],
391-
/// [Application], or [Instrumentation].
392-
/// </summary>
393-
public bool HasComponentAttribute { get; set; }
350+
public IReadOnlyList<string>? ThrownNames { get; }
351+
public string? SuperArgumentsString { get; }
394352

395-
/// <summary>
396-
/// The JNI name from the Name property of a component attribute
397-
/// (e.g., [Activity(Name = "my.app.MainActivity")] → "my/app/MainActivity").
398-
/// Null if no Name was specified on the component attribute.
399-
/// </summary>
400-
public string? ComponentAttributeJniName { get; set; }
353+
public ExportInfo (IReadOnlyList<string>? thrownNames, string? superArgumentsString)
354+
{
355+
ThrownNames = thrownNames;
356+
SuperArgumentsString = superArgumentsString;
357+
}
358+
}
401359

402-
/// <summary>
403-
/// If the type has [Application(BackupAgent = typeof(X))],
404-
/// this is the full name of X.
405-
/// </summary>
406-
public string? ApplicationBackupAgent { get; set; }
360+
abstract class TypeAttributeInfo
361+
{
362+
protected TypeAttributeInfo (string attributeName)
363+
{
364+
AttributeName = attributeName;
365+
}
407366

408-
/// <summary>
409-
/// If the type has [Application(ManageSpaceActivity = typeof(X))],
410-
/// this is the full name of X.
411-
/// </summary>
412-
public string? ApplicationManageSpaceActivity { get; set; }
367+
public string AttributeName { get; }
368+
public string? JniName { get; set; }
369+
}
370+
371+
sealed class ActivityAttributeInfo : TypeAttributeInfo
372+
{
373+
public ActivityAttributeInfo () : base ("ActivityAttribute")
374+
{
375+
}
376+
}
377+
378+
sealed class ServiceAttributeInfo : TypeAttributeInfo
379+
{
380+
public ServiceAttributeInfo () : base ("ServiceAttribute")
381+
{
382+
}
383+
}
384+
385+
sealed class BroadcastReceiverAttributeInfo : TypeAttributeInfo
386+
{
387+
public BroadcastReceiverAttributeInfo () : base ("BroadcastReceiverAttribute")
388+
{
389+
}
390+
}
391+
392+
sealed class ContentProviderAttributeInfo : TypeAttributeInfo
393+
{
394+
public ContentProviderAttributeInfo () : base ("ContentProviderAttribute")
395+
{
396+
}
397+
}
398+
399+
sealed class InstrumentationAttributeInfo : TypeAttributeInfo
400+
{
401+
public InstrumentationAttributeInfo () : base ("InstrumentationAttribute")
402+
{
403+
}
404+
}
405+
406+
sealed class JniNameProviderAttributeInfo : TypeAttributeInfo
407+
{
408+
public JniNameProviderAttributeInfo (string attributeName) : base (attributeName)
409+
{
410+
}
411+
}
412+
413+
sealed class ApplicationAttributeInfo : TypeAttributeInfo
414+
{
415+
public ApplicationAttributeInfo () : base ("ApplicationAttribute")
416+
{
417+
}
418+
419+
public string? BackupAgent { get; set; }
420+
public string? ManageSpaceActivity { get; set; }
413421
}

0 commit comments

Comments
 (0)