No need to store collected members in MembersCollector

A previous commit revealed that `CompositeTypeContributor` is the sole
direct user of `MembersCollector`. It queries the latter's type element
collections (`Events`, `Methods`, and `Properties`) only to transfer
the elements somewhere else.

This means that `MembersCollector` doesn't really need its own collect-
ions; it could simply send collected type elements to wherever they need
to go. This is done via the new interface `IMembersCollectorSink`.

In principle, this also turns `CollectMembersToProxy` into a repeatable
operation: `checkedMethods` no longer needs to act double duty as a flag
to check whether the method was already called.

Author: stakx

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

@@ -49,26 +49,13 @@ public ILogger Logger
 		public void CollectElementsToProxy(IProxyGenerationHook hook, MetaType model)
 		{
 			Debug.Assert(hook != null);
+			Debug.Assert(model != null);
+
+			var sink = new MembersCollectorSink(model, this);
 
 			foreach (var collector in GetCollectors())
 			{
-				collector.CollectMembersToProxy(hook);
-
-				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);
 			}
 		}
 
@@ -149,5 +136,35 @@ 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;
+			}
+
+			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/MembersCollector.cs

@@ -28,11 +28,6 @@ internal abstract class MembersCollector
 		private const BindingFlags Flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
 		private ILogger logger = NullLogger.Instance;
 
-		private HashSet<MethodInfo> checkedMethods = new HashSet<MethodInfo>();
-		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 readonly Type type;
 
 		protected MembersCollector(Type type)
@@ -46,38 +41,17 @@ public ILogger Logger
 			set { logger = value; }
 		}
 
-		public IEnumerable<MetaMethod> Methods
-		{
-			get { return methods; }
-		}
-
-		public IEnumerable<MetaProperty> Properties
-		{
-			get { return properties; }
-		}
-
-		public IEnumerable<MetaEvent> Events
+		public virtual void CollectMembersToProxy(IProxyGenerationHook hook, IMembersCollectorSink sink)
 		{
-			get { return events; }
-		}
+			var checkedMethods = new HashSet<MethodInfo>();
 
-		public virtual void CollectMembersToProxy(IProxyGenerationHook hook)
-		{
-			if (checkedMethods == null) // this method was already called!
-			{
-				throw new InvalidOperationException(
-					string.Format("Can't call 'CollectMembersToProxy' method twice. This usually signifies a bug in custom {0}.",
-					              typeof(ITypeContributor)));
-			}
 			CollectProperties();
 			CollectEvents();
 			// Methods go last, because properties and events have methods too (getters/setters add/remove)
 			// and we don't want to get duplicates, so we collect property and event methods first
 			// then we collect methods, and add only these that aren't there yet
 			CollectMethods();
 
-			checkedMethods = null; // this is ugly, should have a boolean flag for this or something
-
 			void CollectProperties()
 			{
 				var propertiesFound = type.GetProperties(Flags);
@@ -130,13 +104,13 @@ void AddProperty(PropertyInfo property)
 				var nonInheritableAttributes = property.GetNonInheritableAttributes();
 				var arguments = property.GetIndexParameters();
 
-				properties.Add(new MetaProperty(property.Name,
-				                                property.PropertyType,
-				                                property.DeclaringType,
-				                                getter,
-				                                setter,
-				                                nonInheritableAttributes.Select(a => a.Builder),
-				                                arguments.Select(a => a.ParameterType).ToArray()));
+				sink.Add(new MetaProperty(property.Name,
+				                          property.PropertyType,
+				                          property.DeclaringType,
+				                          getter,
+				                          setter,
+				                          nonInheritableAttributes.Select(a => a.Builder),
+				                          arguments.Select(a => a.ParameterType).ToArray()));
 			}
 
 			void AddEvent(EventInfo @event)
@@ -161,8 +135,8 @@ void AddEvent(EventInfo @event)
 					return;
 				}
 
-				events.Add(new MetaEvent(@event.Name,
-				                         @event.DeclaringType, @event.EventHandlerType, adder, remover, EventAttributes.None));
+				sink.Add(new MetaEvent(@event.Name,
+				                       @event.DeclaringType, @event.EventHandlerType, adder, remover, EventAttributes.None));
 			}
 
 			MetaMethod AddMethod(MethodInfo method, bool isStandalone)
@@ -175,7 +149,7 @@ MetaMethod AddMethod(MethodInfo method, bool isStandalone)
 				var methodToGenerate = GetMethodToGenerate(method, hook, isStandalone);
 				if (methodToGenerate != null)
 				{
-					methods.Add(methodToGenerate);
+					sink.Add(methodToGenerate);
 				}
 
 				return methodToGenerate;

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

@@ -28,9 +28,9 @@ public WrappedClassMembersCollector(Type type) : base(type)
 		{
 		}
 
-		public override void CollectMembersToProxy(IProxyGenerationHook hook)
+		public override void CollectMembersToProxy(IProxyGenerationHook hook, IMembersCollectorSink sink)
 		{
-			base.CollectMembersToProxy(hook);
+			base.CollectMembersToProxy(hook, sink);
 			CollectFields(hook);
 			// TODO: perhaps we should also look for nested classes...
 		}