Swap the order of checks to prevent an unnecessary exception (#1926)

* Swap the order of checks to prevent an unnecessary exception from being thrown

* Tidy up usage

---------

Co-authored-by: Shaun Lawrence <[email protected]>
Co-authored-by: Brandon Minnick <[email protected]>

samples/CommunityToolkit.Maui.Sample/Pages/Converters/IsListNullOrEmptyConverterPage.xaml

@@ -33,6 +33,7 @@
 
             <Label
                 Text="{Binding StringItemSource, Converter={StaticResource IsListNullOrEmptyConverter}}"
+                AutomationProperties.IsInAccessibleTree="{Binding StringItemSource, Converter={StaticResource IsListNullOrEmptyConverter}}"
                 FontAttributes="Bold"
                 TextColor="Red" />
 

src/CommunityToolkit.Maui/Extensions/ValueConverterExtension.shared.cs

@@ -11,17 +11,23 @@ public ICommunityToolkitValueConverter ProvideValue(IServiceProvider serviceProv
 
 	private protected static bool IsNullable<T>() => IsNullable(typeof(T));
 
-	private protected static bool IsValidTargetType<T>(in Type targetType)
+	private protected static bool IsValidTargetType<TTarget>(in Type targetType, bool shouldAllowNullableValueTypes)
 	{
 		if (IsConvertingToString(targetType) && CanBeConvertedToString())
 		{
 			return true;
 		}
 
+		// Is TTarget a Value Type and targetType a Nullable Value Type? Eg TTarget is bool and targetType is bool?
+		if (shouldAllowNullableValueTypes && typeof(TTarget).IsValueType && IsValidNullableValueType(targetType))
+		{
+			return true;
+		}
+
 		try
 		{
-			var instanceOfT = default(T);
-			instanceOfT ??= (T?)Activator.CreateInstance(targetType);
+			var instanceOfT = default(TTarget);
+			instanceOfT ??= (TTarget?)Activator.CreateInstance(targetType);
 
 			var result = Convert.ChangeType(instanceOfT, targetType);
 
@@ -33,21 +39,8 @@ private protected static bool IsValidTargetType<T>(in Type targetType)
 		}
 
 		static bool IsConvertingToString(in Type targetType) => targetType == typeof(string);
-		static bool CanBeConvertedToString() => typeof(T).GetMethods().Any(x => x.Name is nameof(ToString) && x.ReturnType == typeof(string));
-	}
-
-	private protected static void ValidateTargetType<TTarget>(Type targetType, bool shouldAllowNullableValueTypes)
-	{
-		ArgumentNullException.ThrowIfNull(targetType);
-
-		// Ensure TTo can be assigned to the given Target Type
-		if (!typeof(TTarget).IsAssignableFrom(targetType) // Ensure TTarget can be assigned from targetType. Eg TTarget is IEnumerable and targetType is IList
-			&& !IsValidTargetType<TTarget>(targetType) // Ensure targetType be converted to TTarget? Eg `Convert.ChangeType()` returns a non-null value
-			&& !(shouldAllowNullableValueTypes && typeof(TTarget).IsValueType && IsValidNullableValueType(targetType))) // Is TTarget a Value Type and targetType a Nullable Value Type? Eg TTarget is bool and targetType is bool?
-		{
-			throw new ArgumentException($"targetType needs to be assignable from {typeof(TTarget)}.", nameof(targetType));
-		}
-
+		static bool CanBeConvertedToString() => typeof(TTarget).GetMethods().Any(x => x.Name is nameof(ToString) && x.ReturnType == typeof(string));
+
 		static bool IsValidNullableValueType(Type targetType)
 		{
 			if (!IsNullable(targetType))
@@ -61,6 +54,19 @@ static bool IsValidNullableValueType(Type targetType)
 		}
 	}
 
+	private protected static void ValidateTargetType<TTarget>(Type targetType, bool shouldAllowNullableValueTypes)
+	{
+		ArgumentNullException.ThrowIfNull(targetType);
+
+		// Ensure TTo can be assigned to the given Target Type
+		if (!typeof(TTarget).IsAssignableFrom(targetType) // Ensure TTarget can be assigned from targetType. Eg TTarget is IEnumerable and targetType is IList
+			&& !IsValidTargetType<TTarget>(targetType, shouldAllowNullableValueTypes) // Ensure targetType be converted to TTarget? Eg `Convert.ChangeType()` returns a non-null value
+			) 
+		{
+			throw new ArgumentException($"targetType needs to be assignable from {typeof(TTarget)}.", nameof(targetType));
+		}
+	}
+
 #pragma warning disable CS8603 // Possible null reference return. If TParam is null (e.g. `string?`), a null return value is expected
 	private protected static TParam ConvertParameter<TParam>(object? parameter) => parameter switch
 	{