[Foundation] Unify the FromObjectsAndKeys implementations for creating dictionaries. #24556

rolfbjarne · 2026-01-23T11:58:51Z

Add two validation functions, one for when FromObjectsAndKeys is called with
a count, and one for when FromObjectsAndKeys is called without a count.
Call the corresponding validation function in every FromObjectsAndKeys implementation.
In every FromObjectsAndKeys function without a count parameter, forward the implementation
to the FromObjectsAndKeys with a count parameter.
Add numerous tests.

Consequences:

Specifying count=0 now succeed (creating an empty dictionary).
Fixed numerous cases where the count parameter was ignored.
Allow arrays of different lengths for the key and values array parameters when
specifying a count.

…g dictionaries. * Add two validation functions, one for when FromObjectsAndKeys is called with a count, and one for when FromObjectsAndKeys is called without a count. * Call the corresponding validation function in every FromObjectsAndKeys implementation. * In every FromObjectsAndKeys function without a count parameter, forward the implementation to the FromObjectsAndKeys with a count parameter. * Add numerous tests. Consequences: * Specifying count=0 now succeed (creating an empty dictionary). * Fixed numerous cases where the count parameter was ignored. * Allow arrays of different lengths for the key and values array parameters when specifying a count.

Copilot

Pull request overview

This PR unifies the FromObjectsAndKeys implementations across NSDictionary, NSMutableDictionary, and their generic counterparts by introducing centralized validation functions and forwarding non-count implementations to count-based implementations.

Changes:

Added two validation helper functions in NSDictionary.cs to centralize parameter validation
Updated all FromObjectsAndKeys methods to use the validation functions and forward to count-based implementations
Added comprehensive tests for edge cases including null values, count=0, different array lengths, and invalid count values

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/Foundation/NSDictionary.cs	Added ValidateFromObjectsAndKeys helper functions; updated FromObjectsAndKeys methods to use validation and forward to count-based overloads
src/Foundation/NSMutableDictionary.cs	Updated FromObjectsAndKeys methods to use validation helpers and forward to count-based overloads
src/Foundation/NSDictionary_2.cs	Updated generic FromObjectsAndKeys methods to use validation helpers and forward to count-based overloads
src/Foundation/NSMutableDictionary_2.cs	Updated generic FromObjectsAndKeys methods to use validation helpers and forward to count-based overloads
tests/monotouch-test/Foundation/NSDictionaryTest.cs	Added test for dictionary constructor with null values
tests/monotouch-test/Foundation/NSDictionary2Test.cs	Added test for generic dictionary constructor with null values
tests/monotouch-test/Foundation/NSMutableDictionaryTest.cs	Added comprehensive tests for FromObjectsAndKeys including null handling, count validation, and array length variations
tests/monotouch-test/Foundation/NSMutableDictionary2Test.cs	Added comprehensive tests for generic FromObjectsAndKeys including null handling, count validation, and array length variations
tests/monotouch-test/Foundation/NSOrderedSetTest.cs	Added tests for NSOrderedSet constructors with null values and arrays (appears unrelated to PR's dictionary focus)
tests/monotouch-test/Foundation/NSMutableOrderedSetTest.cs	Added tests for NSMutableOrderedSet constructors with null values and arrays (appears unrelated to PR's dictionary focus)
tests/monotouch-test/Foundation/NSMutableSetTest.cs	Added tests for NSMutableSet constructors with null values and arrays (appears unrelated to PR's dictionary focus)

Copilot · 2026-01-23T12:08:49Z

tests/monotouch-test/Foundation/NSMutableOrderedSetTest.cs

+		[Test]
+		public void Ctor_WithNull ()
+		{
+			var str1 = (NSString) "1";
+			NSObject? nullObj = null;
+			using (var set = new NSMutableOrderedSet (str1, nullObj)) {
+				Assert.AreEqual (2, (int) set.Count, "Count should include null");
+				Assert.AreEqual (str1, set [0], "First item");
+				Assert.IsInstanceOf<NSNull> (set [1], "Second item should be NSNull");
+			}
+		}
+
+		[Test]
+		public void Ctor_NullArray ()
+		{
+			NSObject []? objs = null;
+			using (var set = new NSMutableOrderedSet (objs)) {
+				Assert.AreEqual (0, (int) set.Count, "Null array should create empty set");
+			}
+		}


The tests added to NSMutableOrderedSetTest.cs appear to be unrelated to the PR's stated purpose of "Unify the FromObjectsAndKeys implementations for creating dictionaries." These tests are for NSMutableOrderedSet constructors, not dictionary FromObjectsAndKeys methods. While these may be valuable tests, they should ideally be in a separate PR focused on Set functionality rather than dictionary FromObjectsAndKeys unification.

Copilot · 2026-01-23T12:08:49Z

src/Foundation/NSDictionary.cs

+			ArgumentNullException.ThrowIfNull (keys);
+
+			if (count < 0 || objects.Length < count || keys.Length < count)
+				throw new ArgumentException (nameof (count));


The error message for the count validation is too generic. When count is negative or too large, the exception message only contains the parameter name "count" without explaining what's wrong. Consider providing a more descriptive error message such as "count must be non-negative and not greater than the length of both arrays" to help developers understand the issue.

Suggested change

throw new ArgumentException (nameof (count));

throw new ArgumentException ("count must be non-negative and not greater than the length of both arrays", nameof (count));

Copilot · 2026-01-23T12:08:49Z

src/Foundation/NSDictionary_2.cs

+			if (!ValidateFromObjectsAndKeys (objects, keys))
+				return new NSDictionary<TKey, TValue> ();

 			return FromObjectsAndKeys (objects, keys, keys.Length);


Inconsistent use of array length in forwarding calls. This method uses keys.Length when forwarding to the count-based overload, while most other similar methods use objects.Length (e.g., NSMutableDictionary.cs lines 47, 61; NSDictionary_2.cs line 292; NSMutableDictionary_2.cs lines 287, 301). Since validation ensures both arrays have the same length, this doesn't affect functionality, but for maintainability and consistency, all forwarding calls should use the same pattern. Consider using objects.Length consistently across all implementations.

Suggested change

return FromObjectsAndKeys (objects, keys, keys.Length);

return FromObjectsAndKeys (objects, keys, objects.Length);

Copilot · 2026-01-23T12:08:50Z

src/Foundation/NSDictionary.cs

+			if (!ValidateFromObjectsAndKeys (objects, keys))
+				return new NSDictionary ();

 			return FromObjectsAndKeys (objects, keys, keys.Length);


Inconsistent use of array length in forwarding calls. This method uses keys.Length when forwarding to the count-based overload, while most other similar methods use objects.Length (e.g., NSMutableDictionary.cs lines 47, 61; NSDictionary_2.cs line 292; NSMutableDictionary_2.cs lines 287, 301). Since validation ensures both arrays have the same length, this doesn't affect functionality, but for maintainability and consistency, all forwarding calls should use the same pattern. Consider using objects.Length consistently across all implementations.

Suggested change

return FromObjectsAndKeys (objects, keys, keys.Length);

return FromObjectsAndKeys (objects, keys, objects.Length);

Copilot · 2026-01-23T12:08:50Z

src/Foundation/NSDictionary.cs

+			ArgumentNullException.ThrowIfNull (objects);
+			ArgumentNullException.ThrowIfNull (keys);
+
+			if (count < 0 || objects.Length < count || keys.Length < count)


Inconsistent exception type for negative count. This validation throws ArgumentException for negative count values, but ArgumentOutOfRangeException would be more appropriate and consistent with NSArray.FromNativeObjects which throws ArgumentOutOfRangeException for negative count (NSArray.cs line 238-239). Consider changing the exception type when count is negative to ArgumentOutOfRangeException, while keeping ArgumentException for other validation failures.

Suggested change

if (count < 0 || objects.Length < count || keys.Length < count)

if (count < 0)

throw new ArgumentOutOfRangeException (nameof (count));

if (objects.Length < count || keys.Length < count)

Copilot · 2026-01-23T12:08:50Z

tests/monotouch-test/Foundation/NSOrderedSetTest.cs

+		[Test]
+		public void Ctor_WithNull ()
+		{
+			var str1 = (NSString) "1";
+			NSObject? nullObj = null;
+			using (var set = new NSOrderedSet (str1, nullObj)) {
+				Assert.AreEqual (2, (int) set.Count, "Count should include null");
+				Assert.AreEqual (str1, set [0], "First item");
+				Assert.IsInstanceOf<NSNull> (set [1], "Second item should be NSNull");
+			}
+		}
+
+		[Test]
+		public void Ctor_NullArray ()
+		{
+			NSObject []? objs = null;
+			using (var set = new NSOrderedSet (objs)) {
+				Assert.AreEqual (0, (int) set.Count, "Null array should create empty set");
+			}
+		}
+
+		[Test]
+		public void MakeNSOrderedSet_WithNull ()
+		{
+			var str1 = (NSString) "1";
+			var str2 = (NSString) "2";
+			var values = new NSString? [] { str1, null, str2 };
+			using (var set = NSOrderedSet.MakeNSOrderedSet (values)) {
+				Assert.AreEqual (3, (int) set.Count, "Count should include null");
+				Assert.AreEqual (str1, set [0], "First item");
+				Assert.IsInstanceOf<NSNull> (set [1], "Second item should be NSNull");
+				Assert.AreEqual (str2, set [2], "Third item");
+			}
+		}
+
+		[Test]
+		public void MakeNSOrderedSet_NullArray ()
+		{
+			NSString []? values = null;
+			using (var set = NSOrderedSet.MakeNSOrderedSet (values)) {
+				Assert.IsNotNull (set, "Should create a set");
+				Assert.AreEqual (0, (int) set.Count, "Null array should create empty set");
+			}
+		}


The tests added to NSOrderedSetTest.cs appear to be unrelated to the PR's stated purpose of "Unify the FromObjectsAndKeys implementations for creating dictionaries." These tests are for NSOrderedSet constructors and MakeNSOrderedSet methods, not dictionary FromObjectsAndKeys methods. While these may be valuable tests, they should ideally be in a separate PR focused on Set functionality rather than dictionary FromObjectsAndKeys unification.

Copilot · 2026-01-23T12:08:51Z

tests/monotouch-test/Foundation/NSMutableSetTest.cs

+		[Test]
+		public void Ctor_WithNull ()
+		{
+			var str1 = (NSString) "1";
+			NSObject? nullObj = null;
+			using (var set = new NSMutableSet (str1, nullObj)) {
+				Assert.AreEqual ((nuint) 2, set.Count, "Count should include null");
+				Assert.IsTrue (set.Contains (str1), "Should contain string");
+				Assert.IsTrue (set.Contains (NSNull.Null), "Should contain NSNull");
+			}
+		}
+
+		[Test]
+		public void Ctor_NullArray ()
+		{
+			NSObject []? objs = null;
+			using (var set = new NSMutableSet (objs)) {
+				Assert.AreEqual ((nuint) 0, set.Count, "Null array should create empty set");
+			}
+		}


The tests added to NSMutableSetTest.cs appear to be unrelated to the PR's stated purpose of "Unify the FromObjectsAndKeys implementations for creating dictionaries." These tests are for NSMutableSet constructors, not dictionary FromObjectsAndKeys methods. While these may be valuable tests, they should ideally be in a separate PR focused on Set functionality rather than dictionary FromObjectsAndKeys unification.

vs-mobiletools-engineering-service2 · 2026-01-23T12:46:51Z

✅ [CI Build #79d4963] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T12:53:45Z

✅ [PR Build #79d4963] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T13:39:19Z

✅ API diff for current PR / commit

NET (empty diffs)

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

✅ API diff vs stable

NET (empty diffs)

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T13:46:57Z

✅ [CI Build #79d4963] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T14:00:31Z

💻 [CI Build #79d4963] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T14:01:25Z

💻 [CI Build #79d4963] Tests on macOS M1 - Mac Ventura (13) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T14:02:20Z

💻 [CI Build #79d4963] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

✅ All tests on macOS arm64 - Mac Sequoia (15) passed.

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T14:02:36Z

💻 [CI Build #79d4963] Tests on macOS arm64 - Mac Tahoe (26) passed 💻

✅ All tests on macOS arm64 - Mac Tahoe (26) passed.

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T14:04:35Z

💻 [CI Build #79d4963] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

vs-mobiletools-engineering-service2 · 2026-01-23T15:25:15Z

🔥 [CI Build #79d4963] Test results 🔥

Test results

❌ Tests failed on VSTS: test results

0 tests crashed, 3 tests failed, 114 tests passed.

Failures

❌ dotnettests tests (iOS)

1 tests failed, 0 tests passed.

Failed tests

DotNet tests: Failed (Execution failed with exit code 1)
- Xamarin.Tests.AppSizeTest.MonoVM_Interpreter(iOS,"ios-arm64"): Multiple failures or warnings in test:

No added APIs (set the environment variable WRITE_KNOWN_FAILURES=1 and run the tes...
* Xamarin.Tests.AppSizeTest.MonoVM(iOS,"ios-arm64"): Multiple failures or warnings in test:
No added APIs (set the environment variable WRITE_KNOWN_FAILURES=1 and run the tes...

Html Report (VSDrops) Download

❌ monotouch tests (MacCatalyst)

2 tests failed, 9 tests passed.

Failed tests

monotouch-test/Mac Catalyst/Debug (static registrar): Failed (Test run failed.
Tests run: 3640 Passed: 3441 Inconclusive: 13 Failed: 1 Ignored: 198)
monotouch-test/Mac Catalyst/Release (static registrar): Failed (Test run failed.
Tests run: 3640 Passed: 3435 Inconclusive: 13 Failed: 1 Ignored: 204)

Html Report (VSDrops) Download

Successes

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 9 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 79d4963f4e23b00ec5cc63fffef378eabe33f935 [PR build]

rolfbjarne requested a review from Copilot January 23, 2026 11:58

Copilot started reviewing on behalf of rolfbjarne January 23, 2026 11:59 View session

Auto-format source code

79d4963

Copilot AI reviewed Jan 23, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

dalexsoto approved these changes Jan 23, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

	throw new ArgumentException (nameof (count));
	throw new ArgumentException ("count must be non-negative and not greater than the length of both arrays", nameof (count));

	return FromObjectsAndKeys (objects, keys, keys.Length);
	return FromObjectsAndKeys (objects, keys, objects.Length);

-			if (count < 0 || objects.Length < count || keys.Length < count)
+			if (count < 0)
+				throw new ArgumentOutOfRangeException (nameof (count));
+			if (objects.Length < count || keys.Length < count)

[Foundation] Unify the FromObjectsAndKeys implementations for creating dictionaries. #24556

Are you sure you want to change the base?

[Foundation] Unify the FromObjectsAndKeys implementations for creating dictionaries. #24556

Conversation

rolfbjarne commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Jan 23, 2026

✅ [CI Build #79d4963] Build passed (Build packages) ✅

Uh oh!

vs-mobiletools-engineering-service2 commented Jan 23, 2026

✅ [PR Build #79d4963] Build passed (Detect API changes) ✅

Uh oh!

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Jan 23, 2026

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

Uh oh!

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Jan 23, 2026

✅ [CI Build #79d4963] Build passed (Build macOS tests) ✅

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Jan 23, 2026

💻 [CI Build #79d4963] Tests on macOS M1 - Mac Monterey (12) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jan 23, 2026

💻 [CI Build #79d4963] Tests on macOS M1 - Mac Ventura (13) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jan 23, 2026

💻 [CI Build #79d4963] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jan 23, 2026

💻 [CI Build #79d4963] Tests on macOS arm64 - Mac Tahoe (26) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jan 23, 2026

💻 [CI Build #79d4963] Tests on macOS X64 - Mac Sonoma (14) passed 💻

Uh oh!

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Jan 23, 2026

🔥 [CI Build #79d4963] Test results 🔥

Test results

Failures

❌ dotnettests tests (iOS)

Failed tests

❌ monotouch tests (MacCatalyst)

Failed tests

Successes