Simplify code in Match.Matches and It.IsAny<T> (#911)

In `Match.Matches`:

* Split up `if` conditions so only null checks remain at top level
* Turn second `if` into `else`
* Instead of `return`ing early, set `canCastValueToT` bool to `false`
* Rename variable `matchType` -> `t` for clarity
* Remove redundant `is` check in `It`
  ... because `Match` matchers will already have checked `value is T`
  by the time the predicate delegate gets invoked.

In `It.IsAny<T>`, `It.IsNotNull<T>`:

* Simplify `It.IsAny<T>`, `It.IsNotNull<T>` matchers
  by documenting that `IsAssignableFrom` generally yields the same
  results as the `is` operator when run on an `object`-typed variable.

  Note that this is removing the `IsComObject` check as well. This
  check was originally added because it was found that `IsAssignable-
  From` does not always work properly when RCW `__ComObject` gets in-
  volved, whereas still `is` works even in that case. Since we're
  generally going back on `is`, the COM check becomes redundant.

src/Moq/It.cs

@@ -8,9 +8,6 @@
 using System.Text.RegularExpressions;
 using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
-#if FEATURE_COM
-using static System.Runtime.InteropServices.Marshal;
-#endif
 
 using Moq.Protected;
 
@@ -49,12 +46,7 @@ public static class Ref<TValue>
 		public static TValue IsAny<TValue>()
 		{
 			return Match<TValue>.Create(
-#if FEATURE_COM
-				value => value == null || (typeof(TValue).IsAssignableFrom(value.GetType())
-				                           || (IsComObject(value) && value is TValue)),
-#else
-				value => value == null || typeof(TValue).IsAssignableFrom(value.GetType()),
-#endif
+				value => value is TValue || value == null,
 				() => It.IsAny<TValue>());
 		}
 
@@ -72,12 +64,7 @@ internal static MethodCallExpression IsAny(Type genericArgument)
 		public static TValue IsNotNull<TValue>()
 		{
 			return Match<TValue>.Create(
-#if FEATURE_COM
-				value => value != null && (typeof(TValue).IsAssignableFrom(value.GetType())
-				                           || (IsComObject(value) && value is TValue)),
-#else
-				value => value != null && typeof(TValue).IsAssignableFrom(value.GetType()),
-#endif
+				value => value is TValue,
 				() => It.IsNotNull<TValue>());
 		}
 

src/Moq/Match.cs

@@ -108,26 +108,19 @@ internal Match(Predicate<T> condition, Expression<Func<T>> renderExpression, Act
 
 		internal override bool Matches(object value)
 		{
-			if (value != null && !(value is T))
+			bool canCastValueToT;
+
+			if (value != null)
 			{
-				return false;
+				canCastValueToT = value is T;
 			}
-
-			var matchType = typeof(T);
-			if (value == null && matchType.IsValueType
-				&& (!matchType.IsGenericType || matchType.GetGenericTypeDefinition() != typeof(Nullable<>)))
+			else
 			{
-				// If this.Condition expects a value type and we've been passed null,
-				// it can't possibly match.
-				// This tends to happen when you are trying to match a parameter of type int?
-				// with IsAny<int> but then pass null into the mock.
-				// We have to return early from here because you can't cast null to T
-				// when T is a value type.
-				//
-				// See GitHub issue #90: https://github.com/moq/moq4/issues/90
-				return false;
+				var t = typeof(T);
+				canCastValueToT = !t.IsValueType || (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(Nullable<>));
 			}
-			return this.Condition((T)value);
+
+			return canCastValueToT && this.Condition((T)value);
 		}
 
 		internal override void SetupEvaluatedSuccessfully(object value)

tests/Moq.Tests/Matchers/IsAssignableFromVsIsOperatorFixture.cs

@@ -0,0 +1,163 @@
+// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD.
+// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.
+
+using System;
+using System.Collections.Generic;
+
+using Xunit;
+
+namespace Moq.Tests.Matchers
+{
+	// The purpose behind these tests is to find cases where `value is T` and
+	// `typeof(T).IsAssignableFrom(value.GetType())` disagree (i.e. yield different
+	// results). This is to check the validity of simplifying the implementation
+	// of `It.IsAny<T>`, which used to run both checks (except when `value` == null;
+	// see commit history). If both expressions generally yield the same results
+	// (and it looks like that's the case) we can simplify and just run `value is T`.
+	public class IsAssignableFromVsIsOperatorFixture
+	{
+		[Fact]
+		public void Nullable_value_type_is_value_type()
+		{
+			int? value = 42;
+			Assert.True(value is int);
+			Assert.True(typeof(int).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_is_nullable_value_type()
+		{
+			int value = 42;
+			Assert.True(value is int?);
+			Assert.True(typeof(int?).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_is_object()
+		{
+			int value = 42;
+			Assert.True(value is object);
+			Assert.True(typeof(object).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_is_not_wider_value_type()
+		{
+			int value = 42;
+			Assert.False(value is long);
+			Assert.False(typeof(long).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_is_not_narrower_value_type()
+		{
+			long value = 42L;
+			Assert.False(value is int);
+			Assert.False(typeof(int).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_is_not_value_type_of_opposite_signedness()
+		{
+			int value = 42;
+			Assert.False(value is uint);
+			Assert.False(typeof(uint).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_as_object_is_not_wider_value_type()
+		{
+			int value = 42;
+			Assert.False((object)value is long);
+			Assert.False(typeof(long).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_array_is_not_nullable_value_type_array()
+		{
+			int[] value = new[] { 42 };
+			Assert.False(value is int?[]);
+			Assert.False(typeof(int?[]).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_array_as_object_is_not_nullable_value_type_array()
+		{
+			int[] value = new[] { 42 };
+			Assert.False((object)value is int?[]);
+			Assert.False(typeof(int?[]).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_array_is_not_object_array()
+		{
+			int[] value = new[] { 42 };
+			Assert.False(value is object[]);
+			Assert.False(typeof(object[]).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_array_is_wider_value_type_array_but_is_operator_says_no()
+		{
+			int[] value = new[] { 42 };
+			Assert.False(value is uint[]);
+			Assert.True(typeof(uint[]).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_array_is_wider_value_type_array_and_is_operator_agrees_when_value_as_object()
+		{
+			int[] value = new[] { 42 };
+			Assert.True((object)value is uint[]);
+			Assert.True(typeof(uint[]).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Value_type_is_interface()
+		{
+			int value = 42;
+			Assert.True(value is IComparable);
+			Assert.True(typeof(IComparable).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Reference_type_is_object()
+		{
+			string value = "42";
+			Assert.True(value is object);
+			Assert.True(typeof(object).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Reference_type_is_interface()
+		{
+			string value = "42";
+			Assert.True(value is IComparable);
+			Assert.True(typeof(IComparable).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Reference_type_is_super_class()
+		{
+			ArgumentNullException value = new ArgumentNullException();
+			Assert.True(value is Exception);
+			Assert.True(typeof(Exception).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Generic_type_is_not_more_general_generic_type()
+		{
+			IEnumerable<int> value = new List<int>();
+			Assert.False(value is IEnumerable<object>);
+			Assert.False(typeof(IEnumerable<object>).IsAssignableFrom(value.GetType()));
+		}
+
+		[Fact]
+		public void Generic_type_is_not_generic_type_definition()
+		{
+			List<int> value = new List<int>();
+			//Assert.True(value is List<>);
+			Assert.False(typeof(List<>).IsAssignableFrom(value.GetType()));
+		}
+	}
+}