[coregraphics] Add nullability to (generated and manual) bindings #15067

tj-devel709 · 2022-05-19T18:47:34Z

This PR aims to bring nullability changes to CoreGraphics.
Following the steps here:

I am adding nullable enable to all manual files that are not "API_SOURCES" in src/frameworks.sources and making the required nullability changes
Changing all throw new ArgumentNullException ("object")); to ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (object)); for size saving optimization as well to mark that this framework contains nullability changes
Changing any == null or != null to is null and is not null

mandel-macaque

Looks good, I'd like to know why some of the noise changes were done

mandel-macaque · 2022-05-19T23:57:41Z

src/CoreGraphics/CGAffineTransform.cs

@@ -88,13 +90,16 @@ public struct CGAffineTransform {
 		[Obsolete ("Use 'Ty' instead.")]
 		public /* CGFloat */ nfloat y0;   // ty

+#pragma warning disable CS0618 // Type or member is obsolete


I don't know how people feel about the individual warnings, but I'm thinking just add it once per file around a #If NET rather than making so much noise in the diff.

I think it's fine to disable the warnings only when they need to be, it's more code, but it's also more explicit, and it will be removed once we remove legacy Xamarin code.

I slightly prefer once per file but don't care enough either way.

mandel-macaque · 2022-05-19T23:58:12Z

src/CoreGraphics/CGAffineTransform.cs

-			String s = $"[{A}, {B}, {C}, {D}, {Tx}, {Ty}]";
+			var s = $"[{A}, {B}, {C}, {D}, {Tx}, {Ty}]";


why this change?

@mandel-macaque there were 3 string declarations right next to each other and one needed to be a string? so I thought it would be a style improvement to use var for all three, but I can revert the other two for the git blame if wanted!

rolfbjarne · 2022-05-20T09:32:20Z

src/CoreGraphics/CGAffineTransform.cs

@@ -88,13 +90,16 @@ public struct CGAffineTransform {
 		[Obsolete ("Use 'Ty' instead.")]
 		public /* CGFloat */ nfloat y0;   // ty

+#pragma warning disable CS0618 // Type or member is obsolete


I think it's fine to disable the warnings only when they need to be, it's more code, but it's also more explicit, and it will be removed once we remove legacy Xamarin code.

vs-mobiletools-engineering-service2 · 2022-05-31T15:38:12Z

🔥 [PR Build] Build failed 🔥

Build failed for the job 'Generate API diff'

Pipeline on Agent
Hash:

vs-mobiletools-engineering-service2 · 2022-05-31T16:57:33Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1101.Monterey'
Hash: bee3a03e7fd8000b44a7684291fe9c3516fb1710

vs-mobiletools-engineering-service2 · 2022-05-31T16:58:02Z

💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻

✅ All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: bee3a03e7fd8000b44a7684291fe9c3516fb1710

vs-mobiletools-engineering-service2 · 2022-05-31T17:00:36Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: bee3a03e7fd8000b44a7684291fe9c3516fb1710

vs-mobiletools-engineering-service2 · 2022-05-31T17:08:55Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: bee3a03e7fd8000b44a7684291fe9c3516fb1710

vs-mobiletools-engineering-service2 · 2022-05-31T17:11:10Z

💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻

✅ All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: bee3a03e7fd8000b44a7684291fe9c3516fb1710

vs-mobiletools-engineering-service2 · 2022-05-31T17:15:21Z

📋 [PR Build] API Diff 📋

API diff (for current PR)

ℹ️ API Diff (from PR only) (please review changes)

API diff: vsdrops gist

Xamarin

iOS vsdrops gist
macOS vsdrops gist
tvOS vsdrops gist
watchOS vsdrops gist

.NET

dotnet-iOS vsdrops gist
dotnet-macOS vsdrops gist
dotnet-tvOS vsdrops gist
dotnet-MacCatalyst vsdrops gist

Xamarin vs .NET

dotnet-legacy-iOS vsdrops gist
dotnet-legacy-macOS vsdrops gist
dotnet-legacy-tvOS vsdrops gist

iOS vs Mac Catalyst (.NET)

dotnet-macCatiOS vsdrops gist

API diff (vs stable)

✅ API Diff from stable

API diff: vsdrops gist

Xamarin

iOS vsdrops gist
macOS vsdrops gist
tvOS vsdrops gist
watchOS vsdrops gist

.NET

dotnet-iOS vsdrops gist
dotnet-macOS vsdrops gist
dotnet-tvOS vsdrops gist
dotnet-MacCatalyst vsdrops gist

Xamarin vs .NET

dotnet-legacy-iOS vsdrops gist
dotnet-legacy-macOS vsdrops gist
dotnet-legacy-tvOS vsdrops gist

iOS vs Mac Catalyst (.NET)

dotnet-macCatiOS vsdrops gist

Generator diff

✅ Generator Diff (no change)

Pipeline on Agent XAMBOT-1100.Monterey'
Hash: bee3a03e7fd8000b44a7684291fe9c3516fb1710

vs-mobiletools-engineering-service2 · 2022-05-31T23:26:35Z

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

2 tests failed, 146 tests passed.

Failed tests

interdependent-binding-projects/watchOS 32-bits - simulator/Debug: Crashed
introspection/watchOS 32-bits - simulator/Debug: Crashed

Pipeline on Agent XAMBOT-1030.Monterey'
Merge bee3a03 into c17fd25

vs-mobiletools-engineering-service2 · 2022-05-31T23:27:05Z

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

1 tests failed, 147 tests passed.

Failed tests

fsharp/watchOS 32-bits - simulator/Debug: Crashed

Pipeline on Agent XAMBOT-1172.Monterey'
Merge bee3a03 into c17fd25

tj-devel709 · 2022-06-02T13:41:06Z

Unrelated Test Failure - fsharp/watchOS 32-bits - simulator/Debug: Crashed

tj_devel709 added 3 commits May 19, 2022 10:20

enable nullability

ff63348

throw better exceptions

22525e5

use is null and is not null

824ac72

tj-devel709 added the not-notes-worthy Ignore for release notes label May 19, 2022

tj-devel709 added this to the Future milestone May 19, 2022