From f5ecf80e083793782309eb55aeba8c994b2a2c92 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 May 2025 07:50:26 +0000 Subject: [PATCH 1/5] Initial plan for issue From 61c376b27df17765eca3bd096f275071f3b9476b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 May 2025 08:00:18 +0000 Subject: [PATCH 2/5] Fix nullable Event<'Delegate, 'Args> for INotifyPropertyChanged Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/FSharp.Core/event.fs | 12 ++++++------ .../Nullness/Files/EventNullabilityTest.fs | 10 ++++++++++ .../Nullness/NullableReferenceTypesTests.fs | 19 ++++++++++++++++++- 3 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/Language/Nullness/Files/EventNullabilityTest.fs diff --git a/src/FSharp.Core/event.fs b/src/FSharp.Core/event.fs index 483ea38d64f..2de4db1620b 100644 --- a/src/FSharp.Core/event.fs +++ b/src/FSharp.Core/event.fs @@ -83,7 +83,7 @@ type EventWrapper<'Delegate, 'Args> = delegate of 'Delegate * objnull * 'Args -> type Event<'Delegate, 'Args when 'Delegate: delegate<'Args, unit> and 'Delegate :> System.Delegate and 'Delegate: not struct>() = - let mutable multicast: 'Delegate = Unchecked.defaultof<_> + let mutable multicast: 'Delegate | null = Unchecked.defaultof<_> static let mi, argTypes = let instanceBindingFlags = @@ -146,10 +146,10 @@ type Event<'Delegate, 'Args "" interface IEvent<'Delegate, 'Args> with member e.AddHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> 'Delegate) &multicast + Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> ('Delegate | null)) &multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> 'Delegate) &multicast + Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> ('Delegate | null)) &multicast interface System.IObservable<'Args> with member e.Subscribe(observer) = let obj = new EventDelegee<'Args>(observer) @@ -166,7 +166,7 @@ type Event<'Delegate, 'Args [] type Event<'T> = - val mutable multicast: Handler<'T> + val mutable multicast: Handler<'T> | null new() = { multicast = null } member x.Trigger(arg: 'T) = @@ -180,10 +180,10 @@ type Event<'T> = "" interface IEvent<'T> with member e.AddHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> Handler<'T>) &x.multicast + Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> Handler<'T> | null) &x.multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> Handler<'T>) &x.multicast + Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> Handler<'T> | null) &x.multicast interface System.IObservable<'T> with member e.Subscribe(observer) = let h = new Handler<_>(fun sender args -> observer.OnNext(args)) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/Files/EventNullabilityTest.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/Files/EventNullabilityTest.fs new file mode 100644 index 00000000000..e8795b28e70 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/Files/EventNullabilityTest.fs @@ -0,0 +1,10 @@ +namespace Nullness + +open System.ComponentModel + +type XViewModel() = + let propertyChanged = Event() + + interface INotifyPropertyChanged with + [] + member this.PropertyChanged = propertyChanged.Publish \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index ced7e02fdca..4de226d0cdb 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -1540,4 +1540,21 @@ let main _ = |> compile //|> verifyIL ["abc"] |> run - |> verifyOutputContains [|"Test true;,1 true,2 true,3 true,4 true,5 true,6 false,7 true,8 false,9 false,10 false,11 false,12 true"|] \ No newline at end of file + |> verifyOutputContains [|"Test true;,1 true,2 true,3 true,4 true,5 true,6 false,7 true,8 false,9 false,10 false,11 false,12 true"|] + +[] +let ``INotifyPropertyChanged with Event using PropertyChangedEventHandler`` () = + FSharp """namespace Nullness + +open System.ComponentModel + +type XViewModel() = + let propertyChanged = Event() + + interface INotifyPropertyChanged with + [] + member this.PropertyChanged = propertyChanged.Publish +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed \ No newline at end of file From 0e34deb86b4b6748b3491f8046fb5968ab0eb4d3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 May 2025 12:43:27 +0000 Subject: [PATCH 3/5] Remove nullable union types from delegate casts Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/FSharp.Core/event.fs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/FSharp.Core/event.fs b/src/FSharp.Core/event.fs index 2de4db1620b..68ce6fd0624 100644 --- a/src/FSharp.Core/event.fs +++ b/src/FSharp.Core/event.fs @@ -146,10 +146,10 @@ type Event<'Delegate, 'Args "" interface IEvent<'Delegate, 'Args> with member e.AddHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> ('Delegate | null)) &multicast + Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> 'Delegate) &multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> ('Delegate | null)) &multicast + Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> 'Delegate) &multicast interface System.IObservable<'Args> with member e.Subscribe(observer) = let obj = new EventDelegee<'Args>(observer) @@ -180,10 +180,10 @@ type Event<'T> = "" interface IEvent<'T> with member e.AddHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> Handler<'T> | null) &x.multicast + Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> Handler<'T>) &x.multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> Handler<'T> | null) &x.multicast + Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> Handler<'T>) &x.multicast interface System.IObservable<'T> with member e.Subscribe(observer) = let h = new Handler<_>(fun sender args -> observer.OnNext(args)) From 8c95c0b82cc11067945d9e82bb4df081fcc54246 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 May 2025 18:00:29 +0000 Subject: [PATCH 4/5] Update nullable casts for System.Delegate.Remove and CreateDelegate Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/FSharp.Core/event.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FSharp.Core/event.fs b/src/FSharp.Core/event.fs index 68ce6fd0624..07fedbd27da 100644 --- a/src/FSharp.Core/event.fs +++ b/src/FSharp.Core/event.fs @@ -149,12 +149,12 @@ type Event<'Delegate, 'Args Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> 'Delegate) &multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> 'Delegate) &multicast + Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> ('Delegate | null)) &multicast interface System.IObservable<'Args> with member e.Subscribe(observer) = let obj = new EventDelegee<'Args>(observer) - let h = Delegate.CreateDelegate(typeof<'Delegate>, obj, invokeInfo) :?> 'Delegate + let h = Delegate.CreateDelegate(typeof<'Delegate>, obj, invokeInfo) :?> ('Delegate | null) (e :?> IDelegateEvent<'Delegate>).AddHandler(h) From 638df497c49563e7a08a67146e85e6f04d2d738b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 May 2025 10:20:35 +0000 Subject: [PATCH 5/5] Use pattern matching for nullable delegate handling in event.fs Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/FSharp.Core/event.fs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/FSharp.Core/event.fs b/src/FSharp.Core/event.fs index 07fedbd27da..69a9fc53918 100644 --- a/src/FSharp.Core/event.fs +++ b/src/FSharp.Core/event.fs @@ -149,12 +149,18 @@ type Event<'Delegate, 'Args Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> 'Delegate) &multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> ('Delegate | null)) &multicast + Atomic.setWith (fun value -> + match System.Delegate.Remove(value, d) with + | null -> null + | result -> result :?> 'Delegate) &multicast interface System.IObservable<'Args> with member e.Subscribe(observer) = let obj = new EventDelegee<'Args>(observer) - let h = Delegate.CreateDelegate(typeof<'Delegate>, obj, invokeInfo) :?> ('Delegate | null) + let h = + match Delegate.CreateDelegate(typeof<'Delegate>, obj, invokeInfo) with + | null -> null + | result -> result :?> 'Delegate (e :?> IDelegateEvent<'Delegate>).AddHandler(h) @@ -183,7 +189,10 @@ type Event<'T> = Atomic.setWith (fun value -> System.Delegate.Combine(value, d) :?> Handler<'T>) &x.multicast member e.RemoveHandler(d) = - Atomic.setWith (fun value -> System.Delegate.Remove(value, d) :?> Handler<'T>) &x.multicast + Atomic.setWith (fun value -> + match System.Delegate.Remove(value, d) with + | null -> null + | result -> result :?> Handler<'T>) &x.multicast interface System.IObservable<'T> with member e.Subscribe(observer) = let h = new Handler<_>(fun sender args -> observer.OnNext(args))