Fix generator bug with derived reference in base class (#236)

microsoft · Mar 19, 2024 · 08da4e9 · 08da4e9
1 parent 0a613e9
commit 08da4e9
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 37 deletions.
diff --git a/src/NodeApi.Generator/SymbolExtensions.cs b/src/NodeApi.Generator/SymbolExtensions.cs
@@ -254,56 +254,65 @@ private static Type BuildSymbolicObjectType(
         Type[]? genericTypeParameters,
         bool buildType)
     {
+        TypeBuilder typeBuilder;
         Type? baseType = typeSymbol.BaseType?.AsType(genericTypeParameters, buildType);
 
         // A base type might have had a reference to this type and therefore already defined it.
         if (SymbolicTypes.TryGetValue(typeFullName, out Type? thisType))
         {
-            return thisType;
-        }
-
-        TypeBuilder typeBuilder = ModuleBuilder.DefineType(
-            name: typeFullName,
-            GetTypeAttributes(typeSymbol.TypeKind),
-            parent: baseType);
+            if (thisType is not TypeBuilder)
+            {
+                // The type is already fully built.
+                return thisType;
+            }
 
-        if (typeSymbol.TypeParameters.Length > 0)
-        {
-            genericTypeParameters ??= [];
-            genericTypeParameters = typeBuilder.DefineGenericParameters(
-                typeSymbol.TypeParameters.Select((p) => p.Name).ToArray());
+            typeBuilder = (TypeBuilder)thisType;
         }
+        else
+        {
+            typeBuilder = ModuleBuilder.DefineType(
+                name: typeFullName,
+                GetTypeAttributes(typeSymbol.TypeKind),
+                parent: baseType);
 
-        // Add the type builder to the map while building it, to support circular references.
-        SymbolicTypes.Add(typeFullName, typeBuilder);
+            if (typeSymbol.TypeParameters.Length > 0)
+            {
+                genericTypeParameters ??= [];
+                genericTypeParameters = typeBuilder.DefineGenericParameters(
+                    typeSymbol.TypeParameters.Select((p) => p.Name).ToArray());
+            }
 
-        BuildSymbolicTypeMembers(typeSymbol, typeBuilder, genericTypeParameters);
+            // Add the type builder to the map while building it, to support circular references.
+            SymbolicTypes.Add(typeFullName, typeBuilder);
 
-        // Preserve JS attributes, which might be referenced by the marshaller.
-        foreach (AttributeData attribute in typeSymbol.GetAttributes())
-        {
-            if (attribute.AttributeClass!.ContainingNamespace.ToString()!.StartsWith(
-                    typeof(JSExportAttribute).Namespace!))
+            BuildSymbolicTypeMembers(typeSymbol, typeBuilder, genericTypeParameters);
+
+            // Preserve JS attributes, which might be referenced by the marshaller.
+            foreach (AttributeData attribute in typeSymbol.GetAttributes())
             {
-                Type attributeType = attribute.AttributeClass.AsType();
-                ConstructorInfo constructor = attributeType.GetConstructor(
-                    attribute.ConstructorArguments.Select((a) => a.Type!.AsType()).ToArray()) ??
-                    throw new MissingMemberException(
-                        $"Constructor not found for attribute: {attributeType.Name}");
-                CustomAttributeBuilder attributeBuilder = new(
-                    constructor,
-                    attribute.ConstructorArguments.Select((a) => a.Value).ToArray(),
-                    attribute.NamedArguments.Select((a) =>
-                        GetAttributeProperty(attributeType, a.Key)).ToArray(),
-                    attribute.NamedArguments.Select((a) => a.Value.Value).ToArray());
-                typeBuilder.SetCustomAttribute(attributeBuilder);
+                if (attribute.AttributeClass!.ContainingNamespace.ToString()!.StartsWith(
+                        typeof(JSExportAttribute).Namespace!))
+                {
+                    Type attributeType = attribute.AttributeClass.AsType();
+                    ConstructorInfo constructor = attributeType.GetConstructor(
+                        attribute.ConstructorArguments.Select((a) => a.Type!.AsType()).ToArray()) ??
+                        throw new MissingMemberException(
+                            $"Constructor not found for attribute: {attributeType.Name}");
+                    CustomAttributeBuilder attributeBuilder = new(
+                        constructor,
+                        attribute.ConstructorArguments.Select((a) => a.Value).ToArray(),
+                        attribute.NamedArguments.Select((a) =>
+                            GetAttributeProperty(attributeType, a.Key)).ToArray(),
+                        attribute.NamedArguments.Select((a) => a.Value.Value).ToArray());
+                    typeBuilder.SetCustomAttribute(attributeBuilder);
+                }
             }
-        }
 
-        static PropertyInfo GetAttributeProperty(Type type, string name)
-            => type.GetProperty(name, BindingFlags.Public | BindingFlags.Instance) ??
-                throw new MissingMemberException(
-                    $"Property {name} not found on attribute {type.Name}.");
+            static PropertyInfo GetAttributeProperty(Type type, string name)
+                => type.GetProperty(name, BindingFlags.Public | BindingFlags.Instance) ??
+                    throw new MissingMemberException(
+                        $"Property {name} not found on attribute {type.Name}.");
+        }
 
         if (!buildType)
         {

diff --git a/test/TestCases/napi-dotnet/ComplexTypes.cs b/test/TestCases/napi-dotnet/ComplexTypes.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+#pragma warning disable IDE0060 // Unused parameters
 #pragma warning disable IDE0301 // Collection initialization can be simplified
 
 using System;
@@ -179,3 +180,17 @@ public enum TestEnum
     One,
     Two,
 }
+
+// Ensure module generation handles circular references between a base class and derived class.
+public class BaseClass
+{
+    protected BaseClass(int x) { }
+
+    public DerivedClass? Derived { get; set; }
+}
+
+[JSExport]
+public class DerivedClass : BaseClass
+{
+    public DerivedClass(int x) : base(x) { }
+}