Better processing of internal inheritance, generic interfaces, explicit interface methods

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+### Fixed
+- [#38: Internal types in public API break `omit-non-api-members=true`](https://github.com/JetBrains/Refasmer/issues/38).
+- Fix regression since v2.0.0: public compiler-generated types are no longer ommitted from the refasmed assemblies.
+
 ## [2.0.0] - 2024-11-20
 ### Changed
 - **(Breaking change!)** A new mandatory parameter `--omit-non-api-types` (pass either `true` or `false`).
@@ -185,4 +190,4 @@ Release the initial version in form of a .NET executable and a NuGet package.
 [1.0.32]: https://github.com/JetBrains/Refasmer/compare/1.0.31...1.0.32
 [1.0.33]: https://github.com/JetBrains/Refasmer/compare/1.0.32...1.0.33
 [2.0.0]: https://github.com/JetBrains/Refasmer/compare/1.0.33...v2.0.0
-[Unreleased]: https://github.com/JetBrains/Refasmer/compare/1.0.33...HEAD
+[Unreleased]: https://github.com/JetBrains/Refasmer/compare/v2.0.0...HEAD

src/Directory.Build.props

@@ -1,6 +1,6 @@
 <Project>
     <PropertyGroup Label="Packaging">
-        <Version>2.0</Version>
+        <Version>2.0.1-pre05</Version>
         <Copyright>Copyright © JetBrains 2024</Copyright>
 
         <PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>

src/Refasmer/Filters/PartialTypeFilterBase.cs

@@ -1,3 +1,4 @@
+using System.Reflection;
 using System.Reflection.Metadata;
 
 namespace JetBrains.Refasmer.Filters;
@@ -12,6 +13,7 @@ public abstract class PartialTypeFilterBase(bool omitNonApiMembers) : IImportFil
 
     public virtual bool AllowImport(TypeDefinition type, MetadataReader reader)
     {
+        if (type.Attributes.HasFlag(TypeAttributes.Public)) return true;
         var isCompilerGenerated = AttributeCache.HasAttribute(reader, type, FullNames.CompilerGenerated);
         return !isCompilerGenerated;
     }

src/Refasmer/Importer/ImportLogic.cs

@@ -87,17 +87,13 @@ private TypeDefinitionHandle ImportTypeDefinitionSkeleton(TypeDefinitionHandle s
         if (!forcePreservePrivateFields)
             PostProcessSkippedValueTypeFields(skippedInstanceFields!, importedInstanceFields!);
 
-        var implementations = src.GetMethodImplementations()
-            .Select(_reader.GetMethodImplementation)
-            .Where(mi => AllowImportType(_reader.GetMethodClass(mi.MethodDeclaration)))
-            .Select(mi => (MethodDefinitionHandle)mi.MethodBody)
-            .ToImmutableHashSet();
+        var implementations = GetAllowedInterfaceMethodImplementations(src);
 
         foreach (var srcMethodHandle in src.GetMethods())
         {
             var srcMethod = _reader.GetMethodDefinition(srcMethodHandle);
 
-            if (!implementations.Contains(srcMethodHandle) && Filter?.AllowImport(srcMethod, _reader) == false)
+            if (!AllowImportMethod(implementations, srcMethodHandle, srcMethod))
             {
                 Trace?.Invoke($"Not imported {_reader.ToString(srcMethod)}");
                 continue;
@@ -406,6 +402,19 @@ public bool IsReferenceAssembly() =>
             .Select(_reader.GetFullname)
             .Any(name => name == FullNames.ReferenceAssembly);
 
+    private ImmutableHashSet<MethodDefinitionHandle> GetAllowedInterfaceMethodImplementations(TypeDefinition type) =>
+        type.GetMethodImplementations()
+            .Select(_reader.GetMethodImplementation)
+            .Where(mi => AllowImportType(_reader.GetMethodClass(mi.MethodDeclaration)))
+            .Select(mi => (MethodDefinitionHandle)mi.MethodBody)
+            .ToImmutableHashSet();
+
+    private bool AllowImportMethod(
+        IImmutableSet<MethodDefinitionHandle> implementations,
+        MethodDefinitionHandle methodHandle,
+        MethodDefinition method) =>
+        !implementations.Contains(methodHandle) && (Filter == null || Filter.AllowImport(method, _reader));
+
     public ReservedBlob<GuidHandle> Import()
     {
         if (_reader.IsAssembly)
@@ -616,17 +625,41 @@ private IEnumerable<TypeDefinitionHandle> CalculateInternalTypesToPreserve(
             var candidateTypes = new List<TypeDefinitionHandle>();
             var type = _reader.GetTypeDefinition(importedTypeHandle);
             var collector = new UsedTypeCollector(candidateTypes);
+
+            var parentTypes = new List<EntityHandle>();
+            if (!type.BaseType.IsNil) parentTypes.Add(type.BaseType);
+            foreach (var interfaceImplHandle in type.GetInterfaceImplementations())
+            {
+                var interfaceImpl = _reader.GetInterfaceImplementation(interfaceImplHandle);
+                parentTypes.Add(interfaceImpl.Interface);
+            }
+
+            foreach (var parentTypeHandle in parentTypes)
+            {
+                switch (parentTypeHandle.Kind)
+                {
+                    case HandleKind.TypeDefinition:
+                        candidateTypes.Add((TypeDefinitionHandle)parentTypeHandle);
+                        break;
+                    case HandleKind.TypeSpecification:
+                        var specification = _reader.GetTypeSpecification((TypeSpecificationHandle)parentTypeHandle);
+                        AcceptTypeSignature(specification.Signature, collector);
+                        break;
+                }
+            }
+
             foreach (var fieldHandle in type.GetFields())
             {
                 var field = _reader.GetFieldDefinition(fieldHandle);
                 if (Filter == null || Filter.AllowImport(field, _reader))
                     AcceptFieldSignature(field, collector);
             }
 
+            var methodImplementations = GetAllowedInterfaceMethodImplementations(type);
             foreach (var methodHandle in type.GetMethods())
             {
                 var method = _reader.GetMethodDefinition(methodHandle);
-                if (Filter == null || Filter.AllowImport(method, _reader))
+                if (AllowImportMethod(methodImplementations, methodHandle, method))
                     AcceptMethodSignature(method, collector);
             }
 

tests/Refasmer.Tests/IntegrationTestBase.cs

@@ -5,7 +5,7 @@
 
 namespace JetBrains.Refasmer.Tests;
 
-public abstract class IntegrationTestBase : IDisposable
+public abstract class IntegrationTestBase
 {
     protected static async Task<string> BuildTestAssembly()
     {
@@ -72,21 +72,25 @@ protected static string RefasmTestAssembly(string assemblyPath, bool omitNonApiM
     }
 
     protected static Task VerifyTypeContent(string assemblyPath, string typeName) =>
-        VerifyTypeContents(assemblyPath, [typeName], [typeName]);
+        VerifyTypeContents(assemblyPath, [typeName], parameters: [typeName]);
 
-    protected static Task VerifyTypeContents(string assemblyPath, string[] typeNames, object[]? parameters = null)
+    protected static Task VerifyTypeContents(string assemblyPath, string[] typeNames, bool assertTypeExists = true, object[]? parameters = null)
     {
         var assembly = AssemblyDefinition.ReadAssembly(assemblyPath);
         var printout = new StringBuilder();
         foreach (var typeName in typeNames)
         {
             var type = assembly.MainModule.GetType(typeName);
-            Assert.That(
-                type,
-                Is.Not.Null,
-                $"Type \"{typeName}\" is not found in assembly \"{assemblyPath}\".");
+            if (assertTypeExists)
+            {
+                Assert.That(
+                    type,
+                    Is.Not.Null,
+                    $"Type \"{typeName}\" is not found in assembly \"{assemblyPath}\".");
+            }
 
-            Printer.PrintType(type, printout);
+            if (type != null)
+                Printer.PrintType(type, printout);
         }
 
         var verifySettings = new VerifySettings();
@@ -103,7 +107,8 @@ private static void MarkTypesInternal(string inputAssemblyPath, string outputAss
         var assemblyDefinition = AssemblyDefinition.ReadAssembly(inputAssemblyPath);
         foreach (var type in assemblyDefinition.MainModule.Types)
         {
-            if (type.IsPublic && type.Name.EndsWith(typeNameSuffix))
+            var friendlyTypeName = type.Name.Split('`')[0]; // strip generic suffix
+            if (type.IsPublic && friendlyTypeName.EndsWith(typeNameSuffix))
             {
                 type.IsPublic = false;
                 type.IsNotPublic = true;
@@ -147,7 +152,8 @@ public void Dispose()
 
     protected static Outputs CollectConsoleOutput() => new();
 
-    public void Dispose()
+    [TearDown]
+    public void TearDown()
     {
         Program.ResetArguments();
     }

tests/Refasmer.Tests/IntegrationTests.cs

@@ -34,13 +34,36 @@ public async Task CheckRefasmedTypeOmitNonApi(string typeName)
         await VerifyTypeContent(resultAssembly, typeName);
     }
 
-    [Test]
-    public async Task InternalTypeInPublicApi()
+    [TestCase(true)]
+    [TestCase(false)]
+    public async Task CheckCompilerGeneratedClasses(bool omitNonApi)
+    {
+        var assemblyPath = await BuildTestAssemblyWithInternalTypeInPublicApi();
+        var resultAssembly = RefasmTestAssembly(assemblyPath, omitNonApiMembers: omitNonApi);
+        await VerifyTypeContents(
+            resultAssembly,
+            ["RefasmerTestAssembly.CompilerGeneratedPublicClass", "RefasmerTestAssembly.CompilerGeneratedInternalClass"],
+            assertTypeExists: false,
+            parameters: [omitNonApi]);
+    }
+
+    [TestCase("PublicClassWithInternalTypeInApi", "Class1ToBeMarkedInternal")]
+    [TestCase("PublicClassDerivingFromInternal", "Class2ToBeMarkedInternal")]
+    [TestCase("PublicClassImplementingInternal", "IInterface1ToBeMarkedInternal")]
+    [TestCase("PublicClassWithInternalInterfaceImpl", "Class3ToBeMarkedInternal,IInterface2ToBeMarkedInternal`1")]
+    [TestCase("PublicClassWithInternalTypeInExplicitImpl", "IInterface3")]
+    public async Task InternalTypeInPublicApi(string mainClassName, string auxiliaryClassNames)
     {
         var assemblyPath = await BuildTestAssemblyWithInternalTypeInPublicApi();
         var resultAssembly = RefasmTestAssembly(assemblyPath, omitNonApiMembers: true);
+
+        var fullMainClassName = $"RefasmerTestAssembly.{mainClassName}";
+        var fullAuxiliaryClassNames = auxiliaryClassNames.Split(',').Select(x => $"RefasmerTestAssembly.{x}");
+
         await VerifyTypeContents(
             resultAssembly,
-            ["RefasmerTestAssembly.PublicClassWithInternalTypeInApi", "RefasmerTestAssembly.ClassToBeMarkedInternal"]);
+            [fullMainClassName, ..fullAuxiliaryClassNames],
+            assertTypeExists: false,
+            parameters: [mainClassName]);
     }
 }

tests/Refasmer.Tests/Printer.cs

@@ -8,7 +8,23 @@ public static class Printer
     public static void PrintType(TypeDefinition type, StringBuilder printout, string indent = "")
     {
         var access = GetAccessString(type);
-        printout.AppendLine($"{indent}{access} type: {type.FullName}");
+        var typeKind = GetTypeKindString(type);
+        printout.AppendLine($"{indent}{access} {typeKind}: {type.FullName}");
+
+        var baseType = type.BaseType;
+        if (baseType != null && baseType.FullName != "System.Object" && baseType.FullName != "System.ValueType")
+        {
+            printout.AppendLine($"{indent}  - base type: {baseType.FullName}");
+        }
+
+        if (type.HasInterfaces)
+        {
+            foreach (var @interface in type.Interfaces)
+            {
+                printout.AppendLine($"{indent}  - interface impl: {@interface.InterfaceType.FullName}");
+            }
+        }
+
         if (type.HasFields)
         {
             printout.AppendLine($"{indent}fields:");
@@ -34,10 +50,15 @@ public static void PrintType(TypeDefinition type, StringBuilder printout, string
                     }
                 }
 
-                printout.AppendLine($"{indent}): {method.ReturnType}:");
-                foreach (var instruction in method.Body.Instructions)
+                printout.AppendLine($"): {method.ReturnType}:");
+                if (method.IsAbstract)
+                    printout.AppendLine($"{indent}  - <abstract>");
+                else
                 {
-                    printout.AppendLine($"{indent}  - {instruction}");
+                    foreach (var instruction in method.Body.Instructions)
+                    {
+                        printout.AppendLine($"{indent}  - {instruction}");
+                    }
                 }
             }
         }
@@ -62,4 +83,14 @@ private static string GetAccessString(TypeDefinition type)
         if (type.IsNestedFamilyAndAssembly) return "private protected";
         return "internal";
     }
+
+    private static string GetTypeKindString(TypeDefinition typeDefinition)
+    {
+        var result = new StringBuilder();
+        if (typeDefinition.IsInterface) result.Append("interface");
+        else if (typeDefinition.IsAbstract) result.Append("abstract ");
+        if (typeDefinition.IsValueType) result.Append("struct");
+        else if (typeDefinition.IsClass) result.Append("class");
+        return result.ToString();
+    }
 }

tests/Refasmer.Tests/data/IntegrationTests.CheckCompilerGeneratedClasses_omitNonApi=False.verified.txt

@@ -0,0 +1,3 @@
+public type: RefasmerTestAssembly.CompilerGeneratedPublicClass
+methods:
+- .ctor(): System.Void:

tests/Refasmer.Tests/data/IntegrationTests.CheckCompilerGeneratedClasses_omitNonApi=True.verified.txt

@@ -0,0 +1,3 @@
+public type: RefasmerTestAssembly.CompilerGeneratedPublicClass
+methods:
+- .ctor(): System.Void:

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.BlittableGraph.verified.txt

@@ -1,3 +1,3 @@
-public type: RefasmerTestAssembly.BlittableGraph
+public struct: RefasmerTestAssembly.BlittableGraph
 fields:
 - <SyntheticNonEmptyStructMarker>: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.BlittableStructWithPrivateFields.verified.txt

@@ -1,3 +1,3 @@
-public type: RefasmerTestAssembly.BlittableStructWithPrivateFields
+public struct: RefasmerTestAssembly.BlittableStructWithPrivateFields
 fields:
 - <SyntheticNonEmptyStructMarker>: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.EmptyStructWithStaticMember.verified.txt

@@ -1 +1 @@
-public type: RefasmerTestAssembly.EmptyStructWithStaticMember
+public struct: RefasmerTestAssembly.EmptyStructWithStaticMember

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.NonBlittableGraph.verified.txt

@@ -1,3 +1,3 @@
-public type: RefasmerTestAssembly.NonBlittableGraph
+public struct: RefasmerTestAssembly.NonBlittableGraph
 fields:
 - <SyntheticNonEmptyStructMarker>: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.NonBlittableStructWithPrivateFields.verified.txt

@@ -1,3 +1,3 @@
-public type: RefasmerTestAssembly.NonBlittableStructWithPrivateFields
+public struct: RefasmerTestAssembly.NonBlittableStructWithPrivateFields
 fields:
 - <SyntheticNonEmptyStructMarker>: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.NonEmptyStructWithStaticMember.verified.txt

@@ -1,3 +1,3 @@
-public type: RefasmerTestAssembly.NonEmptyStructWithStaticMember
+public struct: RefasmerTestAssembly.NonEmptyStructWithStaticMember
 fields:
 - <SyntheticNonEmptyStructMarker>: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.PublicClassWithPrivateFields.verified.txt

@@ -1,4 +1,4 @@
-public type: RefasmerTestAssembly.PublicClassWithPrivateFields
+public class: RefasmerTestAssembly.PublicClassWithPrivateFields
 fields:
 - PublicInt: System.Int32
 methods:

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.PublicStructWithPrivateFields.verified.txt

@@ -1,3 +1,3 @@
-public type: RefasmerTestAssembly.PublicStructWithPrivateFields
+public struct: RefasmerTestAssembly.PublicStructWithPrivateFields
 fields:
 - PublicInt: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.StructWithNestedPrivateTypes.verified.txt

@@ -1,7 +1,7 @@
-public type: RefasmerTestAssembly.StructWithNestedPrivateTypes
+public struct: RefasmerTestAssembly.StructWithNestedPrivateTypes
 fields:
 - <SyntheticNonEmptyStructMarker>: System.Int32
 types:
-  internal type: RefasmerTestAssembly.StructWithNestedPrivateTypes/UnusedPublicStruct
+  internal struct: RefasmerTestAssembly.StructWithNestedPrivateTypes/UnusedPublicStruct
   fields:
   - <SyntheticNonEmptyStructMarker>: System.Int32

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedTypeOmitNonApi_typeName=RefasmerTestAssembly.UnsafeClassWithFunctionPointer.verified.txt

@@ -1,4 +1,4 @@
-public type: RefasmerTestAssembly.UnsafeClassWithFunctionPointer
+public class: RefasmerTestAssembly.UnsafeClassWithFunctionPointer
 methods:
 - MethodWithFunctionPointer(method System.Void *() functionPointer): System.Void:
 - .ctor(): System.Void:

tests/Refasmer.Tests/data/IntegrationTests.CheckRefasmedType_typeName=RefasmerTestAssembly.NonBlittableGraph.verified.txt

@@ -1,11 +1,11 @@
-public type: RefasmerTestAssembly.NonBlittableGraph
+public struct: RefasmerTestAssembly.NonBlittableGraph
 fields:
 - x: RefasmerTestAssembly.NonBlittableGraph/DubiouslyBlittableType
 types:
-  private type: RefasmerTestAssembly.NonBlittableGraph/DubiouslyBlittableType
+  private struct: RefasmerTestAssembly.NonBlittableGraph/DubiouslyBlittableType
   fields:
   - x: RefasmerTestAssembly.NonBlittableGraph/DubiouslyBlittableType/NonBlittableType
   types:
-    private type: RefasmerTestAssembly.NonBlittableGraph/DubiouslyBlittableType/NonBlittableType
+    private struct: RefasmerTestAssembly.NonBlittableGraph/DubiouslyBlittableType/NonBlittableType
     fields:
     - x: System.String

tests/Refasmer.Tests/data/IntegrationTests.InternalTypeInPublicApi_mainClassName=PublicClassDerivingFromInternal.verified.txt

@@ -0,0 +1,4 @@
+public type: RefasmerTestAssembly.PublicClassDerivingFromInternal
+methods:
+- .ctor(): System.Void:
+internal type: RefasmerTestAssembly.Class2ToBeMarkedInternal

tests/Refasmer.Tests/data/IntegrationTests.InternalTypeInPublicApi_mainClassName=PublicClassImplementingInternal.verified.txt

@@ -0,0 +1,5 @@
+public class: RefasmerTestAssembly.PublicClassImplementingInternal
+  - interface impl: RefasmerTestAssembly.IInterface1ToBeMarkedInternal
+methods:
+- .ctor(): System.Void:
+internal interface: RefasmerTestAssembly.IInterface1ToBeMarkedInternal

tests/Refasmer.Tests/data/IntegrationTests.InternalTypeInPublicApi_mainClassName=PublicClassWithInternalInterfaceImpl.verified.txt

@@ -0,0 +1,6 @@
+public class: RefasmerTestAssembly.PublicClassWithInternalInterfaceImpl
+  - interface impl: RefasmerTestAssembly.IInterface2ToBeMarkedInternal`1<RefasmerTestAssembly.Class3ToBeMarkedInternal>
+methods:
+- .ctor(): System.Void:
+internal class: RefasmerTestAssembly.Class3ToBeMarkedInternal
+internal interface: RefasmerTestAssembly.IInterface2ToBeMarkedInternal`1

tests/Refasmer.Tests/data/IntegrationTests.InternalTypeInPublicApi_mainClassName=PublicClassWithInternalTypeInApi.verified.txt

@@ -0,0 +1,5 @@
+public type: RefasmerTestAssembly.PublicClassWithInternalTypeInApi
+methods:
+- Accept(RefasmerTestAssembly.Class1ToBeMarkedInternal argument): System.Void:
+- .ctor(): System.Void:
+internal type: RefasmerTestAssembly.Class1ToBeMarkedInternal

tests/Refasmer.Tests/data/IntegrationTests.InternalTypeInPublicApi_mainClassName=PublicClassWithInternalTypeInExplicitImpl.verified.txt

@@ -0,0 +1,5 @@
+public class: RefasmerTestAssembly.PublicClassWithInternalTypeInExplicitImpl
+  - interface impl: RefasmerTestAssembly.IInterface3
+methods:
+- .ctor(): System.Void:
+internal interface: RefasmerTestAssembly.IInterface3