Merge pull request #29 from nventive/dev/cdb/immutables-misc-fixes

Fixed a potential stack overflow

src/Uno.CodeGen.Tests.ExternalClasses/AbstractExternalGenericClass.cs

@@ -17,9 +17,9 @@
 namespace Uno.CodeGen.Tests.ExternalClasses
 {
 	[GeneratedImmutable]
-	public abstract partial class AbstractExternalGenericClass<T>
+	public abstract partial class AbstractExternalGenericClass<TId>
 	{
 		[EqualityHash]
-		public T Id { get; }
+		public TId Id { get; }
 	}
 }

src/Uno.CodeGen.Tests/Given_ImmutableEntity.Inheritance.cs

@@ -14,6 +14,7 @@
 // limitations under the License.
 //
 // ******************************************************************
+using FluentAssertions;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Uno.Equality;
 using AbstractExternalClass = Uno.CodeGen.Tests.ExternalClasses.AbstractExternalClass;
@@ -25,40 +26,26 @@ namespace Uno.CodeGen.Tests
 		[TestMethod]
 		public void Immutable_When_Abstracted_Base_Class()
 		{
-			// var sut1 = InheritanceDerivedClass.Default.WithKeyValue(null);
+			InheritanceDerivedClassFromExternal sut1 = InheritanceDerivedClassFromExternal.Default.WithId("id1");
+			InheritanceDerivedClassFromExternal sut2 = new InheritanceDerivedClassFromExternal.Builder() {Id = "id1"};
+
+			sut1.Equals(sut2).Should().BeTrue();
 		}
 	}
 
-	//public interface IImmutable<out TBuilder>
-	//{
-	//	TBuilder GetBuilder();
-	//}
-
 	[GeneratedImmutable]
-	public abstract partial class InheritanceAbstractBaseClass<T> // : IImmutable<InheritanceAbstractBaseClass<T>.Builder>
+	public abstract partial class InheritanceAbstractBaseClass<T>
 		where T : IKeyEquatable<T>
 	{
 		[EqualityKey]
 		public T KeyValue { get; }
-
-		//public abstract TBuilder GetBuilder<TBuilder>() where TBuilder : Builder;
-
-		//Builder IImmutable<Builder>.GetBuilder()
-		//{
-		//	throw new System.NotImplementedException();
-		//}
 	}
 
-	//[GeneratedImmutable]
-	//public abstract partial class InheritanceAbstractBaseClass2<T> : InheritanceAbstractBaseClass<T>
-	//	where T : IKeyEquatable<T>
-	//{
-	//	//Builder IImmutable<InheritanceAbstractBaseClass2<T>.Builder>.GetBuilder()
-	//	//{
-	//	//	throw new System.NotImplementedException();
-	//	//}
-
-	//}
+	[GeneratedImmutable]
+	public abstract partial class InheritanceAbstractBaseClass2<T> : InheritanceAbstractBaseClass<T>
+		where T : IKeyEquatable<T>
+	{
+	}
 
 	[GeneratedImmutable]
 	public partial class InheritanceHashedClass
@@ -70,10 +57,6 @@ namespace Uno.CodeGen.Tests
 	[GeneratedImmutable]
 	public partial class InheritanceDerivedClass : InheritanceAbstractBaseClass<InheritanceHashedClass>
 	{
-		//public override TBuilder GetBuilder<TBuilder>() where TBuilder : Builder
-		//{
-		//	throw new System.NotImplementedException();
-		//}
 	}
 
 	public partial class InheritanceDerivedClassFromExternal : AbstractExternalClass

src/Uno.CodeGen/Helpers/TypeSymbolExtensions.cs

@@ -29,11 +29,6 @@ namespace Uno.Helpers
 {
 	public static class TypeSymbolExtensions
 	{
-		public static bool IsGenericArgument(this ITypeSymbol type)
-		{
-			return type.GetType().Name.Equals("SourceTypeParameterSymbol");
-		}
-
 		public static bool IsPrimitive(this ITypeSymbol type)
 		{
 			switch (type.SpecialType)

src/Uno.CodeGen/ImmutableGenerator.cs

@@ -643,11 +643,11 @@ public static implicit operator {symbolNameWithGenerics}(Builder builder)
 				}
 
 				builder.AppendLine();
-				using (builder.BlockInvariant($"{typeSymbol.GetAccessibilityAsCSharpCodeString()} static partial class {symbolName}Extensions"))
-				{
-					var propertiesForWithExtensions = GetPropertiesForWithExtensions(baseTypeInfo.baseBuilderType, properties);
+				var propertiesForWithExtensions = GetPropertiesForWithExtensions(baseTypeInfo.baseBuilderType, properties);
 
-					if (propertiesForWithExtensions.Any() && !typeSymbol.IsAbstract)
+				if (propertiesForWithExtensions.Any() && !typeSymbol.IsAbstract)
+				{
+					using (builder.BlockInvariant($"{typeSymbol.GetAccessibilityAsCSharpCodeString()} static partial class {symbolName}Extensions"))
 					{
 						var builderName = $"{symbolNameWithGenerics}.Builder";
 
@@ -675,6 +675,8 @@ public static implicit operator {symbolNameWithGenerics}(Builder builder)
 								builder.AppendLineInvariant($"return ({builderName})(new {builderName}(entity).With{prop.Name}(value));");
 							}
 
+							builder.AppendLine();
+
 							builder.AppendLineInvariant("/// <summary>");
 							builder.AppendLineInvariant($"/// Set property {prop.Name} in a fluent declaration by projecting previous value.");
 							builder.AppendLineInvariant("/// </summary>");
@@ -818,16 +820,18 @@ $@"public sealed class {symbolName}BuilderJsonConverterTo{symbolName}{genericArg
 					$"#error {nameof(ImmutableGenerator)}: Type {symbolNames.SymbolNameWithGenerics} **MUST** derive from an immutable class.");
 			}
 
-			void CheckTypeImmutable(ITypeSymbol type, string typeSource)
+			void CheckTypeImmutable(ITypeSymbol type, string typeSource, bool constraintsChecked = false)
 			{
-				if (type.IsGenericArgument())
+				if (type is ITypeParameterSymbol typeParameter)
 				{
-					var typeParameter = type as ITypeParameterSymbol;
-					if (typeParameter?.ConstraintTypes.Any() ?? false)
+					if (typeParameter.ConstraintTypes.Any())
 					{
-						foreach (var constraintType in typeParameter.ConstraintTypes)
+						if (!constraintsChecked)
 						{
-							CheckTypeImmutable(constraintType, $"{typeSymbol} / Generic type {typeParameter}:{constraintType}");
+							foreach (var constraintType in typeParameter.ConstraintTypes)
+							{
+								CheckTypeImmutable(constraintType, $"{typeSymbol} / Generic type {typeParameter}:{constraintType}", constraintsChecked: true);
+							}
 						}
 					}
 					else
@@ -865,7 +869,7 @@ $@"public sealed class {symbolName}BuilderJsonConverterTo{symbolName}{genericArg
 				{
 					foreach (var typeArgument in namedType.TypeArguments)
 					{
-						CheckTypeImmutable(typeArgument, $"{typeSource} (argument type {typeArgument})");
+						CheckTypeImmutable(typeArgument, $"{typeSource} (argument type {typeArgument})", constraintsChecked);
 					}
 				}
 			}