diff --git a/src/bgen/AttributeManager.cs b/src/bgen/AttributeManager.cs index 66b5bd9a271d..c82d1fdeb770 100644 --- a/src/bgen/AttributeManager.cs +++ b/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) 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; + } } diff --git a/src/bgen/Filters.cs b/src/bgen/Filters.cs index bf8fb6721bca..822d20968d1b 100644 --- a/src/bgen/Filters.cs +++ b/src/bgen/Filters.cs @@ -200,7 +200,7 @@ void GenerateProperties (Type type, Type? originalType = null, bool fromProtocol nullable = true; break; } - if (AttributeManager.HasAttribute (p)) + if (AttributeManager.IsNullable (p)) nullable = true; print ("public {0}{1} {2} {{", ptype, nullable ? "?" : "", p.Name); indent++; diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index a03610250377..e8194f563203 100644 --- a/src/bgen/Generator.cs +++ b/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 (pi); + bool allow_null = null_allowed_override || AttributeManager.IsNullable (pi); if (mai.ZeroCopyStringMarshal) { if (allow_null) @@ -828,7 +828,7 @@ 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 (pi)) + if (null_allowed_override || AttributeManager.IsNullable (pi)) return safe_name + "__handle__"; return String.Format (mt.ParameterMarshal, safe_name); } @@ -836,7 +836,7 @@ public string MarshalParameter (MethodInfo mi, ParameterInfo pi, bool null_allow if (pi.ParameterType.IsArray) { //Type etype = pi.ParameterType.GetElementType (); - if (null_allowed_override || AttributeManager.HasAttribute (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 (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 (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 (pi)) + if (AttributeManager.IsNullable (pi)) return false; if (IsSetter (mi)) { - if (AttributeManager.HasAttribute (mi)) { + if (AttributeManager.IsNullable (mi)) { return false; } - if ((propInfo is not null) && AttributeManager.HasAttribute (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 (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 (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 (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 (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 (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 (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 (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 (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 (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 (pi); + bool null_allowed = AttributeManager.IsNullable (pi); if (!null_allowed) { var property = GetProperty (mi); if (property is not null) - null_allowed = AttributeManager.HasAttribute (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 (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 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 (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 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 (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 instance_fields_ } if (generate_setter) { var ba = GetBindAttribute (setter); - bool null_allowed = AttributeManager.HasAttribute (setter); + bool null_allowed = AttributeManager.IsNullable (setter); if (null_allowed) ErrorHelper.Show (new BindingException (1118, false, setter)); - null_allowed |= AttributeManager.HasAttribute (pi); + null_allowed |= AttributeManager.IsNullable (pi); var not_implemented_attr = AttributeManager.GetCustomAttribute (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 (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 (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 (pi)) { - var nonNullableProperty = protocolsThatHaveThisProp.SingleOrDefault (v => !AttributeManager.HasAttribute (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 (p); + var nullable = !pt.IsValueType && AttributeManager.IsNullable (p); print ("public {0}{1} {2} {{ get; set; }}", TypeManager.RenderType (bareType), nullable ? "?" : "", GetPublicParameterName (p)); } diff --git a/src/bgen/Models/AsyncMethodInfo.cs b/src/bgen/Models/AsyncMethodInfo.cs index 2d4cfe9901bb..951057afd346 100644 --- a/src/bgen/Models/AsyncMethodInfo.cs +++ b/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 (lastParam); + IsNSErrorNullable = generator.AttributeManager.IsNullable (lastParam); cbParams = cbParams.DropLast (); } diff --git a/tests/generator/BGenTests.cs b/tests/generator/BGenTests.cs index 0bb9374d673b..5d46f9df6711 100644 --- a/tests/generator/BGenTests.cs +++ b/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) { diff --git a/tests/generator/tests/nscopying-nullability.cs b/tests/generator/tests/nscopying-nullability.cs new file mode 100644 index 000000000000..01bb83b9dcfb --- /dev/null +++ b/tests/generator/tests/nscopying-nullability.cs @@ -0,0 +1,9 @@ +using System; +using Foundation; +using ObjCRuntime; + +namespace NS { + [BaseType (typeof (NSObject))] + interface MyNSCopyingWidget : INSCopying { + } +}