Merge pull request #572 from stakx/dp/refactor/members-collector

Share `CompositeTypeContributor`'s type element collections with `MembersCollector`

Author: stakx

src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs

@@ -37,19 +37,15 @@ public ClassProxyTargetContributor(Type targetType, INamingScope namingScope)
 			this.targetType = targetType;
 		}
 
-		protected override IEnumerable<MembersCollector> CollectElementsToProxyInternal(IProxyGenerationHook hook)
+		protected override IEnumerable<MembersCollector> GetCollectors()
 		{
-			Debug.Assert(hook != null, "hook != null");
-
 			var targetItem = new ClassMembersCollector(targetType) { Logger = Logger };
-			targetItem.CollectMembersToProxy(hook);
 			yield return targetItem;
 
 			foreach (var @interface in interfaces)
 			{
 				var item = new InterfaceMembersOnClassCollector(@interface, true,
 					targetType.GetInterfaceMap(@interface)) { Logger = Logger };
-				item.CollectMembersToProxy(hook);
 				yield return item;
 			}
 		}

src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs

@@ -35,19 +35,15 @@ public ClassProxyWithTargetTargetContributor(Type targetType, INamingScope namin
 			this.targetType = targetType;
 		}
 
-		protected override IEnumerable<MembersCollector> CollectElementsToProxyInternal(IProxyGenerationHook hook)
+		protected override IEnumerable<MembersCollector> GetCollectors()
 		{
-			Debug.Assert(hook != null, "hook != null");
-
 			var targetItem = new WrappedClassMembersCollector(targetType) { Logger = Logger };
-			targetItem.CollectMembersToProxy(hook);
 			yield return targetItem;
 
 			foreach (var @interface in interfaces)
 			{
 				var item = new InterfaceMembersOnClassCollector(@interface, true,
 					targetType.GetInterfaceMap(@interface)) { Logger = Logger };
-				item.CollectMembersToProxy(hook);
 				yield return item;
 			}
 		}

src/Castle.Core/DynamicProxy/Contributors/CompositeTypeContributor.cs

@@ -31,9 +31,9 @@ internal abstract class CompositeTypeContributor : ITypeContributor
 		protected readonly ICollection<Type> interfaces = new HashSet<Type>();
 
 		private ILogger logger = NullLogger.Instance;
-		private readonly ICollection<MetaProperty> properties = new TypeElementCollection<MetaProperty>();
-		private readonly ICollection<MetaEvent> events = new TypeElementCollection<MetaEvent>();
-		private readonly ICollection<MetaMethod> methods = new TypeElementCollection<MetaMethod>();
+		private readonly List<MetaProperty> properties = new List<MetaProperty>();
+		private readonly List<MetaEvent> events = new List<MetaEvent>();
+		private readonly List<MetaMethod> methods = new List<MetaMethod>();
 
 		protected CompositeTypeContributor(INamingScope namingScope)
 		{
@@ -48,27 +48,18 @@ public ILogger Logger
 
 		public void CollectElementsToProxy(IProxyGenerationHook hook, MetaType model)
 		{
-			foreach (var collector in CollectElementsToProxyInternal(hook))
+			Debug.Assert(hook != null);
+			Debug.Assert(model != null);
+
+			var sink = new MembersCollectorSink(model, this);
+
+			foreach (var collector in GetCollectors())
 			{
-				foreach (var method in collector.Methods)
-				{
-					model.AddMethod(method);
-					methods.Add(method);
-				}
-				foreach (var @event in collector.Events)
-				{
-					model.AddEvent(@event);
-					events.Add(@event);
-				}
-				foreach (var property in collector.Properties)
-				{
-					model.AddProperty(property);
-					properties.Add(property);
-				}
+				collector.CollectMembersToProxy(hook, sink);
 			}
 		}
 
-		protected abstract IEnumerable<MembersCollector> CollectElementsToProxyInternal(IProxyGenerationHook hook);
+		protected abstract IEnumerable<MembersCollector> GetCollectors();
 
 		public virtual void Generate(ClassEmitter @class)
 		{
@@ -145,5 +136,49 @@ private void ImplementMethod(MetaMethod method, ClassEmitter @class,
 				}
 			}
 		}
+
+		private sealed class MembersCollectorSink : IMembersCollectorSink
+		{
+			private readonly MetaType model;
+			private readonly CompositeTypeContributor contributor;
+
+			public MembersCollectorSink(MetaType model, CompositeTypeContributor contributor)
+			{
+				this.model = model;
+				this.contributor = contributor;
+			}
+
+			// You may have noticed that most contributors do not query `MetaType` at all,
+			// but only their own collections. So perhaps you are wondering why collected
+			// type elements are added to `model` at all, and not just to `contributor`?
+			//
+			// TL;DR: This prevents member name collisions in the generated proxy type.
+			//
+			// `MetaType` uses `TypeElementCollection`s internally, which switches members
+			// to explicit implementation whenever a name collision with a previously added
+			// member occurs.
+			//
+			// It would be pointless to do this at the level of the individual contributor,
+			// because name collisions could still occur across several contributors. This
+			// is why they all share the same `MetaType` instance.
+
+			public void Add(MetaEvent @event)
+			{
+				model.AddEvent(@event);
+				contributor.events.Add(@event);
+			}
+
+			public void Add(MetaMethod method)
+			{
+				model.AddMethod(method);
+				contributor.methods.Add(method);
+			}
+
+			public void Add(MetaProperty property)
+			{
+				model.AddProperty(property);
+				contributor.properties.Add(property);
+			}
+		}
 	}
 }

src/Castle.Core/DynamicProxy/Contributors/IMembersCollectorSink.cs

@@ -0,0 +1,25 @@
+// Copyright 2004-2021 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Castle.DynamicProxy.Contributors
+{
+	using Castle.DynamicProxy.Generators;
+
+	internal interface IMembersCollectorSink
+	{
+		void Add(MetaEvent @event);
+		void Add(MetaMethod method);
+		void Add(MetaProperty property);
+	}
+}

src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyTargetContributor.cs

@@ -34,15 +34,12 @@ public InterfaceProxyTargetContributor(Type proxyTargetType, bool canChangeTarge
 			this.canChangeTarget = canChangeTarget;
 		}
 
-		protected override IEnumerable<MembersCollector> CollectElementsToProxyInternal(IProxyGenerationHook hook)
+		protected override IEnumerable<MembersCollector> GetCollectors()
 		{
-			Debug.Assert(hook != null, "hook != null");
-
 			foreach (var @interface in interfaces)
 			{
 				var item = GetCollectorForInterface(@interface);
 				item.Logger = Logger;
-				item.CollectMembersToProxy(hook);
 				yield return item;
 			}
 		}

src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs

@@ -32,13 +32,11 @@ public InterfaceProxyWithoutTargetContributor(INamingScope namingScope, GetTarge
 			getTargetExpression = getTarget;
 		}
 
-		protected override IEnumerable<MembersCollector> CollectElementsToProxyInternal(IProxyGenerationHook hook)
+		protected override IEnumerable<MembersCollector> GetCollectors()
 		{
-			Debug.Assert(hook != null, "hook != null");
 			foreach (var @interface in interfaces)
 			{
 				var item = new InterfaceMembersCollector(@interface);
-				item.CollectMembersToProxy(hook);
 				yield return item;
 			}
 		}