xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.44k stars 508 forks source link

Use of INSCopying in binding library causes CS8767 nullability warning #17130

Closed mattjohnsonpint closed 2 weeks ago

mattjohnsonpint commented 1 year ago

Steps to Reproduce

  1. Create an iOS binding library: dotnet new iosbinding -n MyBindingProject
  2. Update ApiDefinition.cs to:

    using Foundation;
    
    namespace MyBindingProject
    {
        [BaseType (typeof (NSObject))]
        interface Widget : INSCopying
        {
        }
    }
  3. Compile: dotnet build

Expected Behavior

Compiles without warnings, and generated Widget class contains:

public virtual NSObject Copy (NSZone? zone)

Actual Behavior

Gives CS8767 warning:

/Users/matt/temp/MyBindingProject/obj/Debug/net7.0-ios/iOS/MyBindingProject/Widget.g.cs(87,27): warning CS8767: Nullability of reference types in type of parameter 'zone' of 'NSObject Widget.Copy(NSZone zone)' doesn't match implicitly implemented member 'NSObject INSCopying.Copy(NSZone? zone)' (possibly because of nullability attributes). [/Users/matt/temp/MyBindingProject/MyBindingProject.csproj]

Generated Widget class contains:

public virtual NSObject Copy (NSZone zone)

... and its implementation (in Widget.g.cs) assumes zone is non-nullable, so will throw when passed a null.

[Export ("copyWithZone:")]
[BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)]
[Preserve (Conditional = true)]
public virtual NSObject Copy (NSZone zone)
{
    var zone__handle__ = zone!.GetNonNullHandle (nameof (zone));
    if (IsDirectBinding) {
        return Runtime.GetNSObject (global::ApiDefinition.Messaging.NativeHandle_objc_msgSend_NativeHandle (this.Handle, Selector.GetHandle ("copyWithZone:"), zone.Handle))!;
    } else {
        return Runtime.GetNSObject (global::ApiDefinition.Messaging.NativeHandle_objc_msgSendSuper_NativeHandle (this.SuperHandle, Selector.GetHandle ("copyWithZone:"), zone.Handle))!;
    }
}

Environment

Version information ``` Visual Studio Enterprise 2022 for Mac Preview Version 17.5 Preview (17.5 build 437) Installation UUID: 6031c5dd-6198-4923-98fb-3d9a34ae3f55 Runtime .NET 7.0.0 (64-bit) Architecture: Arm64 Roslyn (Language Service) 4.4.0-3.22461.4+8ab250290a4010c11a21521f78dbc87dbb7aac81 NuGet Version: 6.3.1.1 .NET SDK (Arm64) SDK: /usr/local/share/dotnet/sdk/7.0.101/Sdks SDK Versions: 7.0.101 7.0.100 6.0.404 6.0.403 6.0.402 MSBuild SDKs: /Applications/Visual Studio (Preview).app/Contents/MonoBundle/MSBuild/Current/bin/Sdks .NET SDK (x64) SDK Versions: 6.0.404 6.0.403 3.1.426 3.1.425 .NET Runtime (Arm64) Runtime: /usr/local/share/dotnet/dotnet Runtime Versions: 7.0.1 7.0.0 6.0.12 6.0.11 6.0.10 .NET Runtime (x64) Runtime: /usr/local/share/dotnet/x64/dotnet Runtime Versions: 6.0.12 6.0.11 3.1.32 3.1.31 3.1.30 Xamarin.Profiler Version: 1.8.0.19 Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler Updater Version: 11 Apple Developer Tools Xcode 14.2 (21534) Build 14C18 Xamarin.Mac Version: 8.12.0.2 (Visual Studio Enterprise) Hash: 87f98a75e Branch: d17-3 Build date: 2022-07-25 20:18:54-0400 Xamarin.iOS Version: 16.0.0.72 (Visual Studio Enterprise) Hash: 6756a1146 Branch: release/6.0.4xx-xcode14 Build date: 2022-09-21 08:51:06-0400 Xamarin Designer Version: 17.5.1.13 Hash: 14bdde937f Branch: remotes/origin/d17-5 Build date: 2022-11-02 21:36:49 UTC Xamarin.Android Version: 13.1.0.1 (Visual Studio Enterprise) Commit: xamarin-android/d17-4/13ba222 Android SDK: /Users/matt/Library/Android/sdk Supported Android versions: 12.1 (API level 32) 12.0 (API level 31) 8.1 (API level 27) 11.0 (API level 30) 10.0 (API level 29) 9.0 (API level 28) 13.0 (API level 33) SDK Command-line Tools Version: 7.0 SDK Platform Tools Version: 33.0.3 SDK Build Tools Version: 33.0.0 rc4 Build Information: Mono: a96bde9 Java.Interop: xamarin/java.interop/d17-4@fcc33ce2 SQLite: xamarin/sqlite/3.39.3@23e1ae7 Xamarin.Android Tools: xamarin/xamarin-android-tools/main@0be567a Microsoft Build of OpenJDK Java SDK: /Library/Java/JavaVirtualMachines/microsoft-11.jdk 11.0.16.1 Android Designer EPL code available here: https://github.com/xamarin/AndroidDesigner.EPL Eclipse Temurin JDK Java SDK: /Library/Java/JavaVirtualMachines/temurin-8.jdk 1.8.0.302 Android Designer EPL code available here: https://github.com/xamarin/AndroidDesigner.EPL Android SDK Manager Version: 17.5.0.8 Hash: 6f27afd Branch: remotes/origin/HEAD Build date: 2022-11-02 21:36:54 UTC Android Device Manager Version: 0.0.0.1217 Hash: 6175224 Branch: main Build date: 2022-11-02 21:36:54 UTC Build Information Release ID: 1705000437 Git revision: a5d0fb913c1ff781c80fe725a3e0511c34e26a08 Build date: 2022-11-02 21:34:22+00 Build branch: release-17.5 Build lane: release-17.5 Operating System Mac OS X 12.6.1 Darwin 21.6.0 Darwin Kernel Version 21.6.0 Thu Sep 29 20:13:56 PDT 2022 root:xnu-8020.240.7~1/RELEASE_ARM64_T6000 arm64 ```

Build Logs

msbuild.binlog.zip

Example Project

MyBindingProject.zip

mattjohnsonpint commented 1 year ago

Because the zone parameter is ignored (per Apple docs), it seems a viable workaround is to ignore the warning (optionally with a NoWarn in the csproj) and to always pass NSZone.Default.

Even better, an extension method such as follows, which also keeps the result strongly typed.

public static T Copy<T>(this T obj) where T : NSObject, INSCopying => (T)obj.Copy(NSZone.Default);
mattjohnsonpint commented 1 year ago

Oh wait, nevermind about my last suggestion. It seems that NSObject.Copy already exists. https://developer.apple.com/documentation/objectivec/nsobject/1418807-copy

So I guess there's not much point in even applying INSCopying in the bindings at all? Copy should still work, right?

rolfbjarne commented 1 year ago

This is the implementation of NSObject.Copy:

[Export ("copy")]
[return: ReleaseAttribute ()]
[BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)]
public virtual NSObject Copy ()
{
    if (!(this is INSCopying)) throw new InvalidOperationException ("Type does not conform to NSCopying");
    NSObject? ret;
    if (IsDirectBinding) {
        ret = Runtime.GetNSObject (global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, Selector.GetHandle ("copy")))!;
    } else {
        ret = Runtime.GetNSObject (global::ObjCRuntime.Messaging.NativeHandle_objc_msgSendSuper (this.SuperHandle, Selector.GetHandle ("copy")))!;
    }
    if (ret != null)
        global::ObjCRuntime.Messaging.void_objc_msgSend (ret.Handle, Selector.GetHandle ("release"));
    return ret!;
}

so since it checks if the current type implements INSCopying, it seems you need to implement the INSCopying interface in your binding.

bruno-garcia commented 3 weeks ago

Any updates on this one? (going through our bindings script and checking links for updates). Updating bindings is extremely difficult

rolfbjarne commented 3 weeks ago

Any updates on this one? (going through our bindings script and checking links for updates). Updating bindings is extremely difficult

I've just implemented a fix for this. It might take a little while until it's released though (at the very latest it'll be with .NET 9 later in the fall).

rolfbjarne commented 3 weeks ago

Any updates on this one? (going through our bindings script and checking links for updates). Updating bindings is extremely difficult

I've just implemented a fix for this. It might take a little while until it's released though (at the very latest it'll be with .NET 9 later in the fall).

Scratch that, I got confused with #17109 and posted here instead of in #17109.

bruno-garcia commented 21 hours ago

Thanks @rolfbjarne. Will this land on .NET 9?

rolfbjarne commented 14 hours ago

Thanks @rolfbjarne. Will this land on .NET 9?

Yes, this will be included in .NET 9.