Fix custom attribute with enum on generic type (#827)

* Fix custom attribute with enum on generic type

Fixes both the reader and the write to correctly handle values of type enum on a generic type.
Cecil represents generic instantiations as typeref which has etype GenericInst, so the exising check for etype doesn't work. Also since attributes only allow simple values and enums (and types), there's technically no other way to get a GenericInst then the enum case.

Added several test for various combinations of boxed an unboxed enums on generic type.

Added a test case provided by @mrvoorhe with array of such enums.

* Disable the new tests on .NET 4

The CodeDom compiler doesn't support parsing enums on generic types in attributes (uses the "old" csc.exe from framework).

Mono.Cecil/AssemblyReader.cs

@@ -3598,6 +3598,12 @@ CustomAttributeArgument ReadCustomAttributeElement (TypeReference type)
 		object ReadCustomAttributeElementValue (TypeReference type)
 		{
 			var etype = type.etype;
+			if (etype == ElementType.GenericInst) {
+				// The only way to get a generic here is that it's an enum on a generic type
+				// so for enum we don't need to know the generic arguments (they have no effect)
+				type = type.GetElementType ();
+				etype = type.etype;
+			}
 
 			switch (etype) {
 			case ElementType.String:

Mono.Cecil/AssemblyWriter.cs

@@ -2993,6 +2993,10 @@ void WriteCustomAttributeValue (TypeReference type, object value)
 				else
 					WriteCustomAttributeEnumValue (type, value);
 				break;
+			case ElementType.GenericInst:
+				// Generic instantiation can only happen for an enum (no other generic like types can appear in attribute value)
+				WriteCustomAttributeEnumValue (type, value);
+				break;
 			default:
 				WritePrimitiveValue (value);
 				break;
@@ -3099,6 +3103,14 @@ void WriteCustomAttributeFieldOrPropType (TypeReference type)
 					WriteTypeReference (type);
 				}
 				return;
+			case ElementType.GenericInst:
+				// Generic instantiation can really only happen if it's an enum type since no other
+				// types are allowed in the attribute value.
+				// Enums are special in attribute data, they're encoded as ElementType.Enum followed by a typeref
+				// followed by the value.
+				WriteElementType (ElementType.Enum);
+				WriteTypeReference (type);
+				return;
 			default:
 				WriteElementType (etype);
 				return;

Test/Mono.Cecil.Tests/CompilationService.cs

@@ -186,7 +186,7 @@ static Compilation GetCompilation (string name)
 			case ".cs":
 				return CS.CSharpCompilation.Create (
 					assemblyName, 
-					new [] { CS.SyntaxFactory.ParseSyntaxTree (source) },
+					new [] { CS.SyntaxFactory.ParseSyntaxTree (source, new CS.CSharpParseOptions (preprocessorSymbols: new string [] { "NET_CORE" })) },
 					references, 
 					new CS.CSharpCompilationOptions (OutputKind.DynamicallyLinkedLibrary, optimizationLevel: OptimizationLevel.Release));
 			default:

Test/Mono.Cecil.Tests/CustomAttributesTests.cs

@@ -546,6 +546,105 @@ public void DefineCustomAttributeFromBlob ()
 			module.Dispose ();
 		}
 
+#if NET_CORE
+		[Test]
+		public void BoxedEnumOnGenericArgumentOnType ()
+		{
+			TestCSharp ("CustomAttributes.cs", module => {
+				var valueEnumGenericType = module.GetType ("BoxedValueEnumOnGenericType");
+
+				Assert.IsTrue (valueEnumGenericType.HasCustomAttributes);
+				Assert.AreEqual (1, valueEnumGenericType.CustomAttributes.Count);
+
+				var attribute = valueEnumGenericType.CustomAttributes [0];
+				Assert.AreEqual ("System.Void FooAttribute::.ctor(System.Object,System.Object)",
+					attribute.Constructor.FullName);
+
+				Assert.IsTrue (attribute.HasConstructorArguments);
+				Assert.AreEqual (2, attribute.ConstructorArguments.Count);
+
+				AssertCustomAttributeArgument ("(Object:(GenericWithEnum`1/OnGenericNumber<System.Int32>:0))", attribute.ConstructorArguments [0]);
+				AssertCustomAttributeArgument ("(Object:(GenericWithEnum`1/OnGenericNumber<System.String>:1))", attribute.ConstructorArguments [1]);
+			});
+		}
+
+		[Test]
+		public void EnumOnGenericArgumentOnType ()
+		{
+			TestCSharp ("CustomAttributes.cs", module => {
+				var valueEnumGenericType = module.GetType ("ValueEnumOnGenericType");
+
+				Assert.IsTrue (valueEnumGenericType.HasCustomAttributes);
+				Assert.AreEqual (1, valueEnumGenericType.CustomAttributes.Count);
+
+				var attribute = valueEnumGenericType.CustomAttributes [0];
+				Assert.AreEqual ("System.Void FooAttribute::.ctor(GenericWithEnum`1/OnGenericNumber<Bingo>)",
+					attribute.Constructor.FullName);
+
+				Assert.IsTrue (attribute.HasConstructorArguments);
+				Assert.AreEqual (1, attribute.ConstructorArguments.Count);
+
+				AssertCustomAttributeArgument ("(GenericWithEnum`1/OnGenericNumber<Bingo>:1)", attribute.ConstructorArguments [0]);
+			});
+		}
+
+		[Test]
+		public void EnumOnGenericFieldOnType ()
+		{
+			TestCSharp ("CustomAttributes.cs", module => {
+				var valueEnumGenericType = module.GetType ("FieldEnumOnGenericType");
+
+				Assert.IsTrue (valueEnumGenericType.HasCustomAttributes);
+				Assert.AreEqual (1, valueEnumGenericType.CustomAttributes.Count);
+
+				var attribute = valueEnumGenericType.CustomAttributes [0];
+				var argument = attribute.Fields.Where (a => a.Name == "NumberEnumField").First ().Argument;
+
+				AssertCustomAttributeArgument ("(GenericWithEnum`1/OnGenericNumber<System.Byte>:0)", argument);
+			});
+		}
+
+		[Test]
+		public void EnumOnGenericPropertyOnType ()
+		{
+			TestCSharp ("CustomAttributes.cs", module => {
+				var valueEnumGenericType = module.GetType ("PropertyEnumOnGenericType");
+
+				Assert.IsTrue (valueEnumGenericType.HasCustomAttributes);
+				Assert.AreEqual (1, valueEnumGenericType.CustomAttributes.Count);
+
+				var attribute = valueEnumGenericType.CustomAttributes [0];
+				var argument = attribute.Properties.Where (a => a.Name == "NumberEnumProperty").First ().Argument;
+
+				AssertCustomAttributeArgument ("(GenericWithEnum`1/OnGenericNumber<System.Byte>:1)", argument);
+			});
+		}
+
+		[Test]
+		public void EnumDeclaredInGenericTypeArray ()
+		{
+			TestCSharp ("CustomAttributes.cs", module => {
+				var type = module.GetType ("WithAttributeUsingNestedEnumArray");
+				var attributes = type.CustomAttributes;
+				Assert.AreEqual (1, attributes.Count);
+				var attribute = attributes [0];
+				Assert.AreEqual (1, attribute.Fields.Count);
+				var arg = attribute.Fields [0].Argument;
+				Assert.AreEqual ("System.Object", arg.Type.FullName);
+
+				var argumentValue = (CustomAttributeArgument)arg.Value;
+				Assert.AreEqual ("GenericWithEnum`1/OnGenericNumber<System.String>[]", argumentValue.Type.FullName);
+				var argumentValues = (CustomAttributeArgument [])argumentValue.Value;
+
+				Assert.AreEqual ("GenericWithEnum`1/OnGenericNumber<System.String>", argumentValues [0].Type.FullName);
+				Assert.AreEqual (0, (int)argumentValues [0].Value);
+
+				Assert.AreEqual ("GenericWithEnum`1/OnGenericNumber<System.String>", argumentValues [1].Type.FullName);
+				Assert.AreEqual (1, (int)argumentValues [1].Value);
+			});
+		}
+#endif
+
 		static void AssertCustomAttribute (string expected, CustomAttribute attribute)
 		{
 			Assert.AreEqual (expected, PrettyPrint (attribute));
@@ -643,7 +742,7 @@ static void PrettyPrint (TypeReference type, StringBuilder signature)
 			if (type.IsArray) {
 				ArrayType array = (ArrayType) type;
 				signature.AppendFormat ("{0}[]", array.ElementType.etype.ToString ());
-			} else if (type.etype == ElementType.None) {
+			} else if (type.etype == ElementType.None || type.etype == ElementType.GenericInst) {
 				signature.Append (type.FullName);
 			} else
 				signature.Append (type.etype.ToString ());

Test/Resources/cs/CustomAttributes.cs

@@ -11,6 +11,14 @@ enum Bingo : short {
 	Binga = 4,
 }
 
+class GenericWithEnum<T> {
+	public enum OnGenericNumber
+    {
+		One,
+		Two
+    }
+}
+
 /*
 in System.Security.AccessControl
 
@@ -70,13 +78,20 @@ public FooAttribute (Type type)
 	{
 	}
 
+	public FooAttribute (GenericWithEnum<Bingo>.OnGenericNumber number)
+    {
+    }
+
 	public int Bang { get { return 0; } set {} }
 	public string Fiou { get { return "fiou"; } set {} }
 
 	public object Pan;
 	public string [] PanPan;
 
 	public Type Chose;
+
+	public GenericWithEnum<byte>.OnGenericNumber NumberEnumField;
+	public GenericWithEnum<byte>.OnGenericNumber NumberEnumProperty { get; set; }
 }
 
 [Foo ("bar")]
@@ -160,3 +175,25 @@ public class Child {
 [Foo ("Foo\0Bar\0")]
 class NullCharInString {
 }
+
+#if NET_CORE
+[Foo (GenericWithEnum<int>.OnGenericNumber.One, GenericWithEnum<string>.OnGenericNumber.Two)]
+class BoxedValueEnumOnGenericType {
+}
+
+[Foo (GenericWithEnum<Bingo>.OnGenericNumber.Two)]
+class ValueEnumOnGenericType {
+}
+
+[Foo (NumberEnumField = GenericWithEnum<byte>.OnGenericNumber.One)]
+class FieldEnumOnGenericType {
+}
+
+[Foo(NumberEnumProperty = GenericWithEnum<byte>.OnGenericNumber.Two)]
+class PropertyEnumOnGenericType {
+}
+
+[Foo(Pan = new[] { GenericWithEnum<string>.OnGenericNumber.One, GenericWithEnum<string>.OnGenericNumber.Two })]
+class WithAttributeUsingNestedEnumArray {
+}
+#endif