[bgen] Improve nullability detection to detect the nullability attributes the C# compiler generates. Fixes #17130.

src/bgen/AttributeManager.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
 using System.Reflection;
 #if !NET
@@ -672,4 +673,40 @@ public bool IsStatic (ICustomAttributeProvider provider)
 		}
 		return false;
 	}
+
+	public bool IsNullable (ICustomAttributeProvider provider)
+	{
+		var attributes = GetAttributes (provider);
+		if (attributes is null)
+			return false;
+
+		// first check if any of the attributes are [NullAllowed]
+		foreach (var attrib in attributes) {
+			var attribType = attrib.GetAttributeType ();
+			if (attribType.Name == "NullAllowedAttribute")
+				return true;
+		}
+
+		// then check for [Nullable]
+		foreach (var attrib in attributes) {
+			var attribType = attrib.GetAttributeType ();
+			if (attribType.Name == "NullableAttribute") {
+				// https://codeblog.jonskeet.uk/2019/02/10/nullableattribute-and-c-8/
+				if (attrib.ConstructorArguments.Count == 1) {
+					var argType = attrib.ConstructorArguments [0].ArgumentType;
+					if (argType.Namespace == "System" && argType.Name == "Byte")
+						return ((byte) attrib.ConstructorArguments [0].Value) == 2;
+					if (argType.IsArray && argType.GetElementType ().Namespace == "System" && argType.GetElementType ().Name == "Byte") {
+						var valueCollection = (ReadOnlyCollection<CustomAttributeTypedArgument>) attrib.ConstructorArguments [0].Value;
+						// Getting complex nullability right means we'll have to completely rework how we render types.
+						// So don't do that for now, just look at the outermost type (the first number in the array),
+						// and return nullability depending on that value.
+						return ((byte) valueCollection [0].Value) == 2;
+					}
+				}
+			}
+		}
+
+		return false;
+	}
 }

src/bgen/Filters.cs

@@ -200,7 +200,7 @@ void GenerateProperties (Type type, Type? originalType = null, bool fromProtocol
 					nullable = true;
 				break;
 			}
-			if (AttributeManager.HasAttribute<NullAllowedAttribute> (p))
+			if (AttributeManager.IsNullable (p))
 				nullable = true;
 			print ("public {0}{1} {2} {{", ptype, nullable ? "?" : "", p.Name);
 			indent++;

src/bgen/Generator.cs

@@ -807,7 +807,7 @@ public string MarshalParameter (MethodInfo mi, ParameterInfo pi, bool null_allow
 			if (mai.PlainString)
 				return safe_name;
 			else {
-				bool allow_null = null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi);
+				bool allow_null = null_allowed_override || AttributeManager.IsNullable (pi);
 
 				if (mai.ZeroCopyStringMarshal) {
 					if (allow_null)
@@ -828,15 +828,15 @@ public string MarshalParameter (MethodInfo mi, ParameterInfo pi, bool null_allow
 
 		MarshalType mt;
 		if (marshalTypes.TryGetMarshalType (pi.ParameterType, out mt)) {
-			if (null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi))
+			if (null_allowed_override || AttributeManager.IsNullable (pi))
 				return safe_name + "__handle__";
 			return String.Format (mt.ParameterMarshal, safe_name);
 		}
 
 		if (pi.ParameterType.IsArray) {
 			//Type etype = pi.ParameterType.GetElementType ();
 
-			if (null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi))
+			if (null_allowed_override || AttributeManager.IsNullable (pi))
 				return String.Format ("nsa_{0}.GetHandle ()", pi.Name);
 			return "nsa_" + pi.Name + ".Handle";
 		}
@@ -875,13 +875,13 @@ public string MarshalParameter (MethodInfo mi, ParameterInfo pi, bool null_allow
 		}
 
 		if (TypeManager.IsDictionaryContainerType (pi.ParameterType)) {
-			if (null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi))
+			if (null_allowed_override || AttributeManager.IsNullable (pi))
 				return String.Format ("{0} is null ? NativeHandle.Zero : {0}.Dictionary.Handle", safe_name);
 			return safe_name + ".Dictionary.Handle";
 		}
 
 		if (pi.ParameterType.IsGenericParameter) {
-			if (null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi))
+			if (null_allowed_override || AttributeManager.IsNullable (pi))
 				return string.Format ("{0}.GetHandle ()", safe_name);
 			return safe_name + ".Handle";
 		}
@@ -895,14 +895,14 @@ public bool ParameterNeedsNullCheck (ParameterInfo pi, MethodInfo mi, PropertyIn
 		if (pi.ParameterType.IsByRef)
 			return false;
 
-		if (AttributeManager.HasAttribute<NullAllowedAttribute> (pi))
+		if (AttributeManager.IsNullable (pi))
 			return false;
 
 		if (IsSetter (mi)) {
-			if (AttributeManager.HasAttribute<NullAllowedAttribute> (mi)) {
+			if (AttributeManager.IsNullable (mi)) {
 				return false;
 			}
-			if ((propInfo is not null) && AttributeManager.HasAttribute<NullAllowedAttribute> (propInfo)) {
+			if ((propInfo is not null) && AttributeManager.IsNullable (propInfo)) {
 				return false;
 			}
 		}
@@ -2022,7 +2022,7 @@ void GenerateEventArgsFile ()
 				var is_internal = prop.IsInternal (this);
 				var export = attrs [0];
 				var use_export_as_string_constant = export.ArgumentSemantic != ArgumentSemantic.None;
-				var null_allowed = AttributeManager.HasAttribute<NullAllowedAttribute> (prop);
+				var null_allowed = AttributeManager.IsNullable (prop);
 				var nullable_type = prop.PropertyType.IsValueType && null_allowed;
 				var propertyType = prop.PropertyType;
 				var propNamespace = prop.DeclaringType.Namespace;
@@ -2716,12 +2716,12 @@ public string MakeSignature (MemberInformation minfo, bool is_async, ParameterIn
 
 				var bindAsAttrib = GetBindAsAttribute (minfo.mi);
 				sb.Append (prefix + TypeManager.FormatType (bindAsAttrib.Type.DeclaringType, bindAsAttrib.Type));
-				if (!bindAsAttrib.Type.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (mi.ReturnParameter))
+				if (!bindAsAttrib.Type.IsValueType && AttributeManager.IsNullable (mi.ReturnParameter))
 					sb.Append ('?');
 			} else {
 				sb.Append (prefix);
 				sb.Append (TypeManager.FormatType (minfo.type, mi.ReturnType));
-				if (!mi.ReturnType.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (mi.ReturnParameter))
+				if (!mi.ReturnType.IsValueType && AttributeManager.IsNullable (mi.ReturnParameter))
 					sb.Append ('?');
 			}
 
@@ -2815,18 +2815,18 @@ public void MakeSignatureFromParameterInfo (bool comma, StringBuilder sb, Member
 				PrintBindAsAttribute (pi, sb);
 				var bt = bindAsAtt.Type;
 				sb.Append (TypeManager.FormatType (bt.DeclaringType, bt));
-				if (!bt.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (pi))
+				if (!bt.IsValueType && AttributeManager.IsNullable (pi))
 					sb.Append ('?');
 			} else {
 				sb.Append (TypeManager.FormatType (declaringType, parType));
 				// some `IntPtr` are decorated with `[NullAttribute]`
 				if (!parType.IsValueType) {
-					if (AttributeManager.HasAttribute<NullAllowedAttribute> (pi)) {
+					if (AttributeManager.IsNullable (pi)) {
 						sb.Append ('?');
 					} else if (pi.Position == 0 && mi is MethodInfo minfo) {
 						// only need to check for setter, since we wouldn't get here for a getter.
 						var propertyInfo = GetProperty (minfo, getter: false, setter: true);
-						if (AttributeManager.HasAttribute<NullAllowedAttribute> (propertyInfo))
+						if (AttributeManager.IsNullable (propertyInfo))
 							sb.Append ('?');
 					}
 				}
@@ -3288,7 +3288,7 @@ void GenerateTypeLowering (MethodInfo mi, bool null_allowed_override, out String
 
 			// Construct conversions
 			if (mai.Type == TypeCache.System_String && !mai.PlainString) {
-				bool probe_null = null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi);
+				bool probe_null = null_allowed_override || AttributeManager.IsNullable (pi);
 
 				convs.AppendFormat (GenerateMarshalString (probe_null, !mai.ZeroCopyStringMarshal), pi.Name, pi.Name.GetSafeParamName ());
 				disposes.AppendFormat (GenerateDisposeString (probe_null, !mai.ZeroCopyStringMarshal), pi.Name);
@@ -3300,7 +3300,7 @@ void GenerateTypeLowering (MethodInfo mi, bool null_allowed_override, out String
 				} else if (HasBindAsAttribute (propInfo)) {
 					disposes.AppendFormat ("\nnsb_{0}?.Dispose ();", propInfo.Name);
 				} else if (etype == TypeCache.System_String) {
-					if (null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi)) {
+					if (null_allowed_override || AttributeManager.IsNullable (pi)) {
 						convs.AppendFormat ("var nsa_{0} = {1} is null ? null : NSArray.FromStrings ({1});\n", pi.Name, pi.Name.GetSafeParamName ());
 						disposes.AppendFormat ("if (nsa_{0} is not null)\n\tnsa_{0}.Dispose ();\n", pi.Name);
 					} else {
@@ -3310,7 +3310,7 @@ void GenerateTypeLowering (MethodInfo mi, bool null_allowed_override, out String
 				} else if (etype == TypeCache.Selector) {
 					exceptions.Add (ErrorHelper.CreateError (1065, mai.Type.FullName, string.IsNullOrEmpty (pi.Name) ? $"#{pi.Position}" : pi.Name, mi.DeclaringType.FullName, mi.Name));
 				} else {
-					if (null_allowed_override || AttributeManager.HasAttribute<NullAllowedAttribute> (pi)) {
+					if (null_allowed_override || AttributeManager.IsNullable (pi)) {
 						convs.AppendFormat ("var nsa_{0} = {1} is null ? null : NSArray.FromNSObjects ({1});\n", pi.Name, pi.Name.GetSafeParamName ());
 						disposes.AppendFormat ("if (nsa_{0} is not null)\n\tnsa_{0}.Dispose ();\n", pi.Name);
 					} else {
@@ -3320,11 +3320,11 @@ void GenerateTypeLowering (MethodInfo mi, bool null_allowed_override, out String
 				}
 			} else if (mai.Type.IsSubclassOf (TypeCache.System_Delegate)) {
 				string trampoline_name = MakeTrampoline (pi.ParameterType).StaticName;
-				bool null_allowed = AttributeManager.HasAttribute<NullAllowedAttribute> (pi);
+				bool null_allowed = AttributeManager.IsNullable (pi);
 				if (!null_allowed) {
 					var property = GetProperty (mi);
 					if (property is not null)
-						null_allowed = AttributeManager.HasAttribute<NullAllowedAttribute> (property);
+						null_allowed = AttributeManager.IsNullable (property);
 				}
 				var createBlockMethod = null_allowed ? "CreateNullableBlock" : "CreateBlock";
 				convs.AppendFormat ("using var block_{0} = Trampolines.{1}.{3} ({2});\n", pi.Name, trampoline_name, safe_name, createBlockMethod);
@@ -3427,7 +3427,7 @@ void GenerateTypeLowering (MethodInfo mi, bool null_allowed_override, out String
 
 	void GenerateArgumentChecks (MethodInfo mi, bool null_allowed_override, PropertyInfo propInfo = null)
 	{
-		if (AttributeManager.HasAttribute<NullAllowedAttribute> (mi))
+		if (AttributeManager.IsNullable (mi))
 			ErrorHelper.Show (new BindingException (1118, false, mi));
 
 		foreach (var pi in mi.GetParameters ()) {
@@ -3945,7 +3945,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
 		Type inlinedType = pi.DeclaringType == type ? null : type;
 		GetAccessorInfo (pi, out var getter, out var setter, out var generate_getter, out var generate_setter);
 
-		var nullable = !pi.PropertyType.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (pi);
+		var nullable = !pi.PropertyType.IsValueType && AttributeManager.IsNullable (pi);
 
 		// So we don't hide the get or set of a parent property with the same name, we need to see if we have a parent declaring the same property
 		PropertyInfo parentBaseType = TypeManager.GetParentTypeWithSameNamedProperty (ReflectionExtensions.GetBaseTypeAttribute (type, this), pi.Name);
@@ -4066,7 +4066,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
 			var bindAsAttrib = GetBindAsAttribute (minfo.mi);
 			propertyTypeName = TypeManager.FormatType (bindAsAttrib.Type.DeclaringType, bindAsAttrib.Type);
 			// it remains nullable only if the BindAs type can be null (i.e. a reference type)
-			nullable = !bindAsAttrib.Type.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (pi);
+			nullable = !bindAsAttrib.Type.IsValueType && AttributeManager.IsNullable (pi);
 		} else {
 			propertyTypeName = TypeManager.FormatType (minfo.type, pi.PropertyType);
 		}
@@ -4178,10 +4178,10 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
 		}
 		if (generate_setter) {
 			var ba = GetBindAttribute (setter);
-			bool null_allowed = AttributeManager.HasAttribute<NullAllowedAttribute> (setter);
+			bool null_allowed = AttributeManager.IsNullable (setter);
 			if (null_allowed)
 				ErrorHelper.Show (new BindingException (1118, false, setter));
-			null_allowed |= AttributeManager.HasAttribute<NullAllowedAttribute> (pi);
+			null_allowed |= AttributeManager.IsNullable (pi);
 			var not_implemented_attr = AttributeManager.GetCustomAttribute<NotImplementedAttribute> (setter);
 			string sel;
 
@@ -4639,7 +4639,7 @@ void GenerateMethod (MemberInformation minfo)
 					// we do not need the information if it's a getter (it won't change generated code)
 					pinfo = GetProperty (method, getter: false, setter: true);
 					if (pinfo is not null)
-						null_allowed = AttributeManager.HasAttribute<NullAllowedAttribute> (pinfo);
+						null_allowed = AttributeManager.IsNullable (pinfo);
 				}
 				GenerateMethodBody (minfo, minfo.Method, minfo.selector, null_allowed, null, BodyOption.None, pinfo);
 				if (minfo.is_autorelease) {
@@ -5130,7 +5130,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName,
 			if (minfo.is_unsafe)
 				mod = "unsafe ";
 			// IsValueType check needed for `IntPtr` signatures (which can't become `IntPtr?`)
-			var nullable = !pi.PropertyType.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (pi) ? "?" : String.Empty;
+			var nullable = !pi.PropertyType.IsValueType && AttributeManager.IsNullable (pi) ? "?" : String.Empty;
 			GetAccessorInfo (pi, out var getMethod, out var setMethod, out var generate_getter, out var generate_setter);
 			print ("{0}{1}{2} {3} {{", mod, TypeManager.FormatType (type, pi.PropertyType), nullable, pi.Name, generate_getter ? "get;" : string.Empty, generate_setter ? "set;" : string.Empty);
 			indent++;
@@ -6251,8 +6251,8 @@ public void Generate (Type type)
 								throw new BindingException (1037, true, pi.Name, type.Name);
 						}
 
-						if (AttributeManager.HasAttribute<NullAllowedAttribute> (pi)) {
-							var nonNullableProperty = protocolsThatHaveThisProp.SingleOrDefault (v => !AttributeManager.HasAttribute<NullAllowedAttribute> (v));
+						if (AttributeManager.IsNullable (pi)) {
+							var nonNullableProperty = protocolsThatHaveThisProp.SingleOrDefault (v => !AttributeManager.IsNullable (v));
 							if (nonNullableProperty is not null) {
 								// We're getting the same property from multiple interfaces, and the nullability attributes don't match.
 								// This results in a warning (which turn into an error because we've turned on warnaserror):
@@ -7195,7 +7195,7 @@ public void Generate (Type type)
 				foreach (var p in pars.Skip (minPars).OrderBy (p => p.Name, StringComparer.Ordinal)) {
 					var pt = p.ParameterType;
 					var bareType = pt.IsByRef ? pt.GetElementType () : pt;
-					var nullable = !pt.IsValueType && AttributeManager.HasAttribute<NullAllowedAttribute> (p);
+					var nullable = !pt.IsValueType && AttributeManager.IsNullable (p);
 
 					print ("public {0}{1} {2} {{ get; set; }}", TypeManager.RenderType (bareType), nullable ? "?" : "", GetPublicParameterName (p));
 				}

src/bgen/Models/AsyncMethodInfo.cs

@@ -28,7 +28,7 @@ public AsyncMethodInfo (Generator generator, IMemberGatherer gather, Type type,
 		var lastParam = cbParams.LastOrDefault ();
 		if (lastParam is not null && lastParam.ParameterType.Name == "NSError") {
 			HasNSError = true;
-			IsNSErrorNullable = generator.AttributeManager.HasAttribute<NullAllowedAttribute> (lastParam);
+			IsNSErrorNullable = generator.AttributeManager.IsNullable (lastParam);
 			cbParams = cbParams.DropLast ();
 		}
 

tests/generator/BGenTests.cs

@@ -230,6 +230,13 @@ public void Bug34042 ()
 			BuildFile (Profile.iOS, "bug34042.cs");
 		}
 
+		[Test]
+		[TestCase (Profile.iOS)]
+		public void NSCopyingNullability (Profile profile)
+		{
+			var bgen = BuildFile (profile, "tests/nscopying-nullability.cs");
+			bgen.AssertNoWarnings ();
+		}
 
 		static string RenderArgument (CustomAttributeArgument arg)
 		{

tests/generator/tests/nscopying-nullability.cs

@@ -0,0 +1,9 @@
+using System;
+using Foundation;
+using ObjCRuntime;
+
+namespace NS {
+	[BaseType (typeof (NSObject))]
+	interface MyNSCopyingWidget : INSCopying {
+	}
+}