Mark facades containing forwarders referenced via type name strings

src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs

@@ -897,7 +897,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
 						}
 						foreach (var typeNameValue in methodParams[0].UniqueValues ()) {
 							if (typeNameValue is KnownStringValue knownStringValue) {
-								TypeReference foundTypeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents);
+								TypeReference foundTypeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents, out _);
 								TypeDefinition foundType = foundTypeRef?.ResolveToMainTypeDefinition ();
 								if (foundType == null) {
 									// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
@@ -1804,14 +1804,16 @@ void RequireDynamicallyAccessedMembers (ref ReflectionPatternContext reflectionC
 				} else if (uniqueValue is SystemTypeValue systemTypeValue) {
 					MarkTypeForDynamicallyAccessedMembers (ref reflectionContext, systemTypeValue.TypeRepresented, requiredMemberTypes);
 				} else if (uniqueValue is KnownStringValue knownStringValue) {
-					TypeReference typeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents);
+					TypeReference typeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents, out AssemblyDefinition typeAssembly);
 					TypeDefinition foundType = typeRef?.ResolveToMainTypeDefinition ();
 					if (foundType == null) {
 						// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
 						reflectionContext.RecordHandledPattern ();
 					} else {
 						MarkType (ref reflectionContext, typeRef);
 						MarkTypeForDynamicallyAccessedMembers (ref reflectionContext, foundType, requiredMemberTypes);
+						if (typeAssembly.MainModule.GetMatchingExportedType (foundType, out var exportedType))
+							_context.MarkingHelpers.MarkExportedType (exportedType, typeAssembly.MainModule, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, foundType));
 					}
 				} else if (uniqueValue == NullValue.Instance) {
 					// Ignore - probably unreachable path as it would fail at runtime anyway.

src/linker/Linker.Steps/DescriptorMarker.cs

@@ -45,6 +45,9 @@ protected override void ProcessAssembly (AssemblyDefinition assembly, XPathNavig
 			if (GetTypePreserve (nav) == TypePreserve.All) {
 				foreach (var type in assembly.MainModule.Types)
 					MarkAndPreserveAll (type);
+
+				foreach (var exportedType in assembly.MainModule.ExportedTypes)
+					_context.MarkingHelpers.MarkExportedType (exportedType, assembly.MainModule, new DependencyInfo (DependencyKind.ModuleOfExportedType, assembly.MainModule));
 			} else {
 				ProcessTypes (assembly, nav, warnOnUnresolvedTypes);
 				ProcessNamespaces (assembly, nav);

src/linker/Linker.Steps/MarkStep.cs

@@ -331,41 +331,7 @@ private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseTypes,
 
 		void Process ()
 		{
-			while (ProcessPrimaryQueue () || ProcessMarkedPending () || ProcessLazyAttributes () || ProcessLateMarkedAttributes () || MarkFullyPreservedAssemblies ()) {
-
-				// deal with [TypeForwardedTo] pseudo-attributes
-
-				// This marks all exported types that resolve to a marked type. Note that it
-				// may mark unused exported types that happen to resolve to a type which was
-				// marked from a different reference. This seems incorrect.
-				// Note also that we may still remove type forwarder assemblies with marked exports in SweepStep,
-				// if they don't contain other used code.
-				// https://github.com/mono/linker/issues/1740
-				foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) {
-					if (!assembly.MainModule.HasExportedTypes)
-						continue;
-
-					foreach (var exported in assembly.MainModule.ExportedTypes) {
-						bool isForwarder = exported.IsForwarder;
-						var declaringType = exported.DeclaringType;
-						while (!isForwarder && (declaringType != null)) {
-							isForwarder = declaringType.IsForwarder;
-							declaringType = declaringType.DeclaringType;
-						}
-
-						if (!isForwarder)
-							continue;
-						TypeDefinition type = exported.Resolve ();
-						if (type == null)
-							continue;
-						if (!Annotations.IsMarked (type))
-							continue;
-						var di = new DependencyInfo (DependencyKind.ExportedType, type);
-						_context.MarkingHelpers.MarkExportedType (exported, assembly.MainModule, di);
-					}
-				}
-			}
-
+			while (ProcessPrimaryQueue () || ProcessMarkedPending () || ProcessLazyAttributes () || ProcessLateMarkedAttributes () || MarkFullyPreservedAssemblies ()) ;
 			ProcessPendingTypeChecks ();
 		}
 
@@ -838,6 +804,15 @@ void MarkDynamicDependency (DynamicDependency dynamicDependency, IMemberDefiniti
 				}
 			}
 
+			ModuleDefinition module = assembly.MainModule;
+			if (module.HasExportedTypes) {
+				foreach (var exportedType in module.ExportedTypes)
+					if (exportedType.Resolve () == type) {
+						_context.MarkingHelpers.MarkExportedType (exportedType, module, new DependencyInfo (DependencyKind.DynamicDependency, type));
+						break;
+					}
+			}
+
 			IEnumerable<IMemberDefinition> members;
 			if (dynamicDependency.MemberSignature is string memberSignature) {
 				members = DocumentationSignatureParser.GetMembersByDocumentationSignature (type, memberSignature, acceptName: true);
@@ -1325,7 +1300,8 @@ void MarkEntireAssembly (AssemblyDefinition assembly)
 			MarkCustomAttributes (assembly.MainModule, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, assembly.MainModule), null);
 
 			if (assembly.MainModule.HasExportedTypes) {
-				// TODO: This needs more work accross all steps
+				foreach (var exportedType in assembly.MainModule.ExportedTypes)
+					_context.MarkingHelpers.MarkExportedType (exportedType, assembly.MainModule, new DependencyInfo (DependencyKind.ModuleOfExportedType, assembly.MainModule));
 			}
 
 			foreach (TypeDefinition type in assembly.MainModule.Types)
@@ -1830,7 +1806,7 @@ protected virtual void MarkTypeConverterLikeDependency (CustomAttribute attribut
 			TypeDefinition tdef = null;
 			switch (attribute.ConstructorArguments[0].Value) {
 			case string s:
-				tdef = _context.TypeNameResolver.ResolveTypeName (s)?.Resolve ();
+				tdef = _context.TypeNameResolver.ResolveTypeName (s, out _)?.Resolve ();
 				break;
 			case TypeReference type:
 				tdef = type.Resolve ();

src/linker/Linker.Steps/SweepStep.cs

@@ -185,8 +185,7 @@ protected void ProcessAssemblyAction (AssemblyDefinition assembly)
 				//
 				// when main.exe has unused reference to type in lib.dll
 				//
-				if (SweepTypeForwarders (assembly))
-					Annotations.SetAction (assembly, AssemblyAction.Save);
+				Annotations.SetAction (assembly, SweepTypeForwarders (assembly) ?? AssemblyAction.Copy);
 
 				break;
 
@@ -195,8 +194,8 @@ protected void ProcessAssemblyAction (AssemblyDefinition assembly)
 				break;
 
 			case AssemblyAction.Save:
-				SweepTypeForwarders (assembly);
-
+				Annotations.SetAction (assembly, SweepTypeForwarders (assembly) ?? AssemblyAction.Save);
+				
 				//
 				// Save means we need to rewrite the assembly due to removed assembly
 				// references
@@ -296,10 +295,18 @@ void SweepResources (AssemblyDefinition assembly)
 				resources.Remove (resource);
 		}
 
-		bool SweepTypeForwarders (AssemblyDefinition assembly)
+		AssemblyAction? SweepTypeForwarders (AssemblyDefinition assembly)
 		{
-			return assembly.MainModule.HasExportedTypes &&
-				SweepCollectionMetadata (assembly.MainModule.ExportedTypes);
+			if (!assembly.MainModule.HasExportedTypes)
+				return null;
+
+			// If this is a pure facade and none of the exported types were removed,
+			// the assembly will be deleted.
+			var exportedTypes = assembly.MainModule.ExportedTypes;
+			if (SweepCollectionExportedTypes (exportedTypes) && (exportedTypes.Count == 0) && assembly.MainModule.Types.Count <= 1)
+				return AssemblyAction.Delete;
+
+			return AssemblyAction.Save;
 		}
 
 		protected virtual void SweepType (TypeDefinition type)
@@ -529,6 +536,25 @@ protected bool SweepCollectionMetadata<T> (IList<T> list) where T : IMetadataTok
 			return removed;
 		}
 
+		protected bool SweepCollectionExportedTypes (IList<ExportedType> exportedTypes)
+		{
+			bool removed = false;
+			for (int i = 0; i < exportedTypes.Count; i++) {
+				var typeDef = exportedTypes[i].Resolve ();
+				if (Context.KeepTypeForwarderOnlyAssemblies) {
+					if (!ShouldRemove (typeDef))
+						continue;
+				} else if (!ShouldRemove (typeDef) && !ShouldRemove (exportedTypes[i]))
+					continue;
+
+				ElementRemoved (exportedTypes[i]);
+				exportedTypes.RemoveAt (i--);
+				removed = true;
+			}
+
+			return removed;
+		}
+
 		protected virtual bool ShouldRemove<T> (T element) where T : IMetadataTokenProvider
 		{
 			return !Annotations.IsMarked (element);

src/linker/Linker/MarkingHelpers.cs

@@ -1,4 +1,3 @@
-using System;
 using Mono.Cecil;
 
 namespace Mono.Linker
@@ -12,12 +11,12 @@ public MarkingHelpers (LinkContext context)
 			_context = context;
 		}
 
-		public void MarkExportedType (ExportedType type, ModuleDefinition module, in DependencyInfo reason)
+		public void MarkExportedType (ExportedType exportedType, ModuleDefinition module, in DependencyInfo reason)
 		{
-			if (!_context.Annotations.MarkProcessed (type, reason))
+			if (!_context.Annotations.MarkProcessed (exportedType, reason))
 				return;
-			if (_context.KeepTypeForwarderOnlyAssemblies)
-				_context.Annotations.Mark (module, new DependencyInfo (DependencyKind.ModuleOfExportedType, type));
+
+			_context.Annotations.Mark (module, new DependencyInfo (DependencyKind.ModuleOfExportedType, exportedType));
 		}
 	}
 }

src/linker/Linker/ModuleDefinitionExtensions.cs

@@ -11,6 +11,21 @@ public static bool IsCrossgened (this ModuleDefinition module)
 				(module.Attributes & ModuleAttributes.ILLibrary) != 0;
 		}
 
+		public static bool GetMatchingExportedType (this ModuleDefinition module, TypeDefinition typeDefinition, out ExportedType exportedType)
+		{
+			exportedType = null;
+			if (!module.HasExportedTypes || typeDefinition == null)
+				return false;
+
+			foreach (var et in module.ExportedTypes)
+				if (et.Resolve () == typeDefinition) {
+					exportedType = et;
+					return true;
+				}
+
+			return false;
+		}
+
 		public static TypeDefinition ResolveType (this ModuleDefinition module, string typeFullName)
 		{
 			if (typeFullName == null)

src/linker/Linker/TypeNameResolver.cs

@@ -13,8 +13,9 @@ public TypeNameResolver (LinkContext context)
 			_context = context;
 		}
 
-		public TypeReference ResolveTypeName (string typeNameString)
+		public TypeReference ResolveTypeName (string typeNameString, out AssemblyDefinition typeAssembly)
 		{
+			typeAssembly = null;
 			if (string.IsNullOrEmpty (typeNameString))
 				return null;
 
@@ -28,13 +29,20 @@ public TypeReference ResolveTypeName (string typeNameString)
 			}
 
 			if (parsedTypeName is AssemblyQualifiedTypeName assemblyQualifiedTypeName) {
-				return ResolveTypeName (null, assemblyQualifiedTypeName);
+				typeAssembly = _context.TryResolve (assemblyQualifiedTypeName.AssemblyName.Name);
+				if (typeAssembly == null)
+					return null;
+
+
+				return ResolveTypeName (typeAssembly, assemblyQualifiedTypeName.TypeName);
 			}
 
 			foreach (var assemblyDefinition in _context.GetReferencedAssemblies ()) {
 				var foundType = ResolveTypeName (assemblyDefinition, parsedTypeName);
-				if (foundType != null)
+				if (foundType != null) {
+					typeAssembly = assemblyDefinition;
 					return foundType;
+				}
 			}
 
 			return null;
@@ -86,7 +94,8 @@ TypeReference ResolveTypeName (AssemblyDefinition assembly, TypeName typeName)
 				};
 			}
 
-			return assembly.MainModule.ResolveType (typeName.ToString ());
+			TypeDefinition typeDefinition = assembly.MainModule.ResolveType (typeName.ToString ());
+			return typeDefinition;
 		}
 	}
 }

test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveAnExportedType.cs

@@ -10,9 +10,10 @@ namespace Mono.Linker.Tests.Cases.LinkXml
 	// Add another assembly in that uses the forwarder just to make things a little more complex
 	[SetupCompileBefore ("Forwarder.dll", new[] { "Dependencies/CanPreserveAnExportedType_Forwarder.cs" }, references: new[] { "Library.dll" })]
 
-	[RemovedAssembly ("Forwarder.dll")]
 	[KeptMemberInAssembly ("Library.dll", typeof (CanPreserveAnExportedType_Library), "Field1", "Method()", ".ctor()")]
 	[SetupLinkerDescriptorFile ("CanPreserveAnExportedType.xml")]
+
+	[KeptTypeInAssembly ("Forwarder.dll", typeof (CanPreserveAnExportedType_Library))]
 	class CanPreserveAnExportedType
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveExportedTypesUsingRegex.cs

@@ -10,9 +10,10 @@ namespace Mono.Linker.Tests.Cases.LinkXml
 	// Add another assembly in that uses the forwarder just to make things a little more complex
 	[SetupCompileBefore ("Forwarder.dll", new[] { "Dependencies/CanPreserveAnExportedType_Forwarder.cs" }, references: new[] { "Library.dll" })]
 
-	[RemovedAssembly ("Forwarder.dll")]
 	[KeptMemberInAssembly ("Library.dll", typeof (CanPreserveAnExportedType_Library), "Field1", "Method()", ".ctor()")]
 	[SetupLinkerDescriptorFile ("CanPreserveExportedTypesUsingRegex.xml")]
+
+	[KeptTypeInAssembly ("Forwarder.dll", typeof (CanPreserveAnExportedType_Library))]
 	class CanPreserveExportedTypesUsingRegex
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/LinkXml/UsedNonRequiredExportedTypeIsKept.cs

@@ -5,15 +5,17 @@ namespace Mono.Linker.Tests.Cases.LinkXml
 {
 	[SetupLinkerDescriptorFile ("UsedNonRequiredExportedTypeIsKept.xml")]
 
-	[SetupCompileBefore ("lib.dll", new[] { "Dependencies/UsedNonRequiredExportedTypeIsKept_lib.cs" })]
-	[SetupCompileAfter ("libfwd.dll", new[] { "Dependencies/UsedNonRequiredExportedTypeIsKept_lib.cs" })]
-	[SetupCompileAfter ("lib.dll", new[] { "Dependencies/UsedNonRequiredExportedTypeIsKept_fwd.cs" }, references: new[] { "libfwd.dll" })]
+	[SetupCompileBefore ("libfwd.dll", new[] { "Dependencies/UsedNonRequiredExportedTypeIsKept_lib.cs" })]
+	[SetupCompileAfter ("lib.dll", new[] { "Dependencies/UsedNonRequiredExportedTypeIsKept_lib.cs" })]
+	[SetupCompileAfter ("libfwd.dll", new[] { "Dependencies/UsedNonRequiredExportedTypeIsKept_fwd.cs" }, references: new[] { "lib.dll" })]
+	[SetupLinkerAction ("copy", "libfwd")]
 
-	[KeptAssembly ("libfwd.dll")]
-	[KeptMemberInAssembly ("libfwd.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used1), "field", ExpectationAssemblyName = "lib.dll")]
-	[KeptMemberInAssembly ("libfwd.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used2), "Method()", ExpectationAssemblyName = "lib.dll")]
-	[KeptMemberInAssembly ("libfwd.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used3), "Method()", ExpectationAssemblyName = "lib.dll")]
-	[RemovedAssembly ("lib.dll")]
+	[KeptMemberInAssembly ("lib.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used1), "field", ExpectationAssemblyName = "libfwd.dll")]
+	[KeptMemberInAssembly ("lib.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used2), "Method()", ExpectationAssemblyName = "libfwd.dll")]
+	[KeptMemberInAssembly ("lib.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used3), "Method()", ExpectationAssemblyName = "libfwd.dll")]
+	[KeptTypeInAssembly ("libfwd.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used1))]
+	[KeptTypeInAssembly ("libfwd.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used2))]
+	[KeptTypeInAssembly ("libfwd.dll", typeof (UsedNonRequiredExportedTypeIsKept_Used3))]
 	public class UsedNonRequiredExportedTypeIsKept
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/TypeForwarding/UnusedForwarderWithAssemblyCopy.cs

@@ -0,0 +1,28 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+using Mono.Linker.Tests.Cases.TypeForwarding.Dependencies;
+
+namespace Mono.Linker.Tests.Cases.TypeForwarding
+{
+	[SetupLinkerDefaultAction ("link")]
+	[SetupCompileBefore ("Forwarder.dll", new[] { "Dependencies/ReferenceImplementationLibrary.cs" }, defines: new[] { "INCLUDE_REFERENCE_IMPL" })]
+
+	[SetupCompileAfter ("Implementation.dll", new[] { "Dependencies/ImplementationLibrary.cs" })]
+	[SetupCompileAfter ("Forwarder.dll", new[] { "Dependencies/ForwarderLibrary.cs" }, references: new[] { "Implementation.dll" })]
+
+	[SetupLinkerAction ("copy", "Forwarder")]
+
+	[RemovedAssembly ("Forwarder.dll")]
+	[RemovedAssembly ("Implementation.dll")]
+
+	public class UnusedForwarderWithAssemblyCopy
+	{
+		static void Main ()
+		{
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/TypeForwarding/UsedForwarderIsDynamicallyAccessedWithAssemblyCopyUsed.cs

@@ -0,0 +1,40 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Diagnostics.CodeAnalysis;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+using Mono.Linker.Tests.Cases.TypeForwarding.Dependencies;
+
+namespace Mono.Linker.Tests.Cases.TypeForwarding
+{
+
+	[SetupLinkerAction ("link", "test")]
+	[SetupLinkerDefaultAction ("copyused")]
+	[KeepTypeForwarderOnlyAssemblies ("false")]
+
+	[SetupCompileBefore ("Forwarder.dll", new[] { "Dependencies/ReferenceImplementationLibrary.cs" }, defines: new[] { "INCLUDE_REFERENCE_IMPL" })]
+
+	// After compiling the test case we then replace the reference impl with implementation + type forwarder
+	[SetupCompileAfter ("Implementation.dll", new[] { "Dependencies/ImplementationLibrary.cs" })]
+	[SetupCompileAfter ("Forwarder.dll", new[] { "Dependencies/ForwarderLibrary.cs" }, references: new[] { "Implementation.dll" })]
+
+	[KeptAssembly ("Forwarder.dll")]
+	[KeptMemberInAssembly ("Implementation.dll", typeof (ImplementationLibrary), "GetSomeValue()")]
+	[RemovedForwarder ("Forwarder.dll", nameof (ImplementationStruct))]
+	class UsedForwarderIsDynamicallyAccessedWithAssemblyCopyUsed
+	{
+		static void Main ()
+		{
+			PointToTypeInFacade ("Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary, Forwarder");
+		}
+
+		[Kept]
+		static void PointToTypeInFacade (
+			[KeptAttributeAttribute (typeof(DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] string typeName)
+		{
+		}
+	}
+}