Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[webkit] Add nullability to (generated and manual) bindings #15028

Merged
merged 16 commits into from
Jun 17, 2022
Merged

[webkit] Add nullability to (generated and manual) bindings #15028

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6e47560
Adding nullable enable
May 13, 2022
177f244
throw better exceptions
May 13, 2022
044a774
use is null
May 13, 2022
15c03f8
fix messages on throw exceptions
May 13, 2022
9fd6376
throw ObjectDisposedException
May 18, 2022
a7d4227
Merge branch 'main' into Nullable-WebKit
tj-devel709 May 20, 2022
eb6652a
throw another ObjectDisposedException
May 23, 2022
6b4ba77
Merge remote-tracking branch 'TJ/Nullable-WebKit' into Nullable-WebKit
May 23, 2022
bc1a82f
Merge branch 'main' into Nullable-WebKit
tj-devel709 May 27, 2022
f53d91d
Merge branch 'main' into Nullable-WebKit
tj-devel709 May 31, 2022
ad68797
Merge branch 'main' into Nullable-WebKit
tj-devel709 Jun 2, 2022
726192a
Merge branch 'main' into Nullable-WebKit
tj-devel709 Jun 6, 2022
a4cb953
Merge branch 'main' into Nullable-WebKit
tj-devel709 Jun 9, 2022
0bef34c
Merge branch 'main' into Nullable-WebKit
tj-devel709 Jun 13, 2022
9440e6c
Merge branch 'main' into Nullable-WebKit
tj-devel709 Jun 14, 2022
b0a178e
Merge branch 'main' into Nullable-WebKit
tj-devel709 Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/WebKit/DomCssRuleList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

namespace WebKit {

public partial class DomCssRuleList {
Expand Down
2 changes: 2 additions & 0 deletions src/WebKit/DomHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

using System;
using ObjCRuntime;
using Foundation;
Expand Down
10 changes: 6 additions & 4 deletions src/WebKit/DomNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//

#nullable enable

using System;
using System.Runtime.Versioning;

Expand Down Expand Up @@ -77,8 +79,8 @@ public IDomEventListener AddEventListener (string type, DomEventListenerHandler
public DomEventListener AddEventListener (string type, DomEventListenerHandler handler, bool useCapture)
#endif
{
if (handler == null)
throw new ArgumentNullException ("handler");
if (handler is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (handler));
var obj = new DomNodeEventProxy (this, handler);
AddEventListener (type, obj, useCapture);
return obj;
Expand All @@ -90,8 +92,8 @@ public IDomEventListener AddEventListener (string type, Action<DomEvent> callbac
public DomEventListener AddEventListener (string type, Action<DomEvent> callback, bool useCapture)
#endif
{
if (callback == null)
throw new ArgumentNullException ("callback");
if (callback is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (callback));
var obj = new DomNodeEventProxy2 (callback);
AddEventListener (type, obj, useCapture);
return obj;
Expand Down
10 changes: 8 additions & 2 deletions src/WebKit/Enumerators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

using System;
using System.Collections;
using System.Collections.Generic;
Expand All @@ -24,23 +26,27 @@ public void Dispose () {

public T Current {
get {
if (_container is null)
throw new ObjectDisposedException (nameof (_container));
return _container [_index];
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighting this !. The return type cannot be made nullable since it is implementing an interface with that method signature but it is also a getter that is not an indexer so I thought this may be the best option

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about using:

if (_container is null) {
    return default (T);
} else {
    return _container [_index]
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might make more sense to throw an ObjectDisposedException.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne good one, that exception should also be a good idea for my other comment.

}

object IEnumerator.Current {
object? IEnumerator.Current {
get { return ((IEnumerator<T>) this).Current; }
}

public bool MoveNext () {
if (_container is null)
throw new ObjectDisposedException (nameof (_container));
return ++_index < _container.Count;
}

public void Reset () {
_index = -1;
}

IIndexedContainer<T> _container;
IIndexedContainer<T>? _container;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed, if the only constructor takes a non-nullable container value and assigns it to this field?

Copy link
Contributor Author

@tj-devel709 tj-devel709 May 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne I added this, because the Dispose method assigns null to _container

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne we need a way to init that. The compiler is complaining because the default value of a IIndexedContainer is null. We could probably make the _container a private Init property with the type and remove the need of ? and remove all the Dipose code. Should make things nicer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne @mandel-macaque Should I do this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tj-devel709 yes, keep this since nulling it the field in the Dispose method might be important

int _index;
}

Expand Down
2 changes: 2 additions & 0 deletions src/WebKit/Indexers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

namespace WebKit {

public partial class DomCssRuleList {
Expand Down
2 changes: 2 additions & 0 deletions src/WebKit/WebKit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

using Foundation;

namespace WebKit {
Expand Down
8 changes: 5 additions & 3 deletions src/WebKit/WebNavigationPolicyEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//
// Copyright 2013 Xamarin Inc

#nullable enable

using System;

using Foundation;
Expand All @@ -27,14 +29,14 @@ public WebNavigationType NavigationType {
get { return (WebNavigationType)((NSNumber)ActionInformation[WebPolicyDelegate.WebActionNavigationTypeKey]).Int32Value; }
}

public NSDictionary ElementInfo {
public NSDictionary? ElementInfo {
get { return ActionInformation[WebPolicyDelegate.WebActionElementKey] as NSDictionary; }
}

public WebActionMouseButton MouseButton {
get {
var number = ActionInformation[WebPolicyDelegate.WebActionButtonKey] as NSNumber;
if (number == null) {
if (number is null) {
return WebActionMouseButton.None;
}

Expand All @@ -46,7 +48,7 @@ public uint Flags {
get { return ((NSNumber)ActionInformation[WebPolicyDelegate.WebActionModifierFlagsKey]).UInt32Value; }
}

public NSUrl OriginalUrl {
public NSUrl? OriginalUrl {
get { return ActionInformation[WebPolicyDelegate.WebActionOriginalUrlKey] as NSUrl; }
}
}
Expand Down
15 changes: 9 additions & 6 deletions src/WebKit/WebPolicyDelegate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//

#nullable enable

using System;
using Foundation;
using ObjCRuntime;
Expand All @@ -33,24 +36,24 @@ public partial class WebPolicyDelegate {

public static void DecideUse (NSObject decisionToken)
{
if (decisionToken == null)
throw new ArgumentNullException ("token");
if (decisionToken is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (decisionToken));

ObjCRuntime.Messaging.void_objc_msgSend (decisionToken.Handle, selUse);
}

public static void DecideDownload (NSObject decisionToken)
{
if (decisionToken == null)
throw new ArgumentNullException ("decisionToken");
if (decisionToken is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (decisionToken));

ObjCRuntime.Messaging.void_objc_msgSend (decisionToken.Handle, selDownload);
}

public static void DecideIgnore (NSObject decisionToken)
{
if (decisionToken == null)
throw new ArgumentNullException ("decisionToken");
if (decisionToken is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (decisionToken));

ObjCRuntime.Messaging.void_objc_msgSend (decisionToken.Handle, selIgnore);
}
Expand Down
15 changes: 9 additions & 6 deletions src/WebKit/WebView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//

#nullable enable

using System;
using Foundation;
using ObjCRuntime;
Expand All @@ -33,24 +36,24 @@ public partial class WebView {

public static void DecideUse (NSObject decisionToken)
{
if (decisionToken == null)
throw new ArgumentNullException ("token");
if (decisionToken is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (decisionToken));

ObjCRuntime.Messaging.void_objc_msgSend (decisionToken.Handle, selUse);
}

public static void DecideDownload (NSObject decisionToken)
{
if (decisionToken == null)
throw new ArgumentNullException ("decisionToken");
if (decisionToken is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (decisionToken));

ObjCRuntime.Messaging.void_objc_msgSend (decisionToken.Handle, selDownload);
}

public static void DecideIgnore (NSObject decisionToken)
{
if (decisionToken == null)
throw new ArgumentNullException ("decisionToken");
if (decisionToken is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (decisionToken));

ObjCRuntime.Messaging.void_objc_msgSend (decisionToken.Handle, selIgnore);
}
Expand Down