Fix a bug with type inference for nullables.

Simplify away the unnecessary portion of Mike's fix in df57e1d, and add an additional test for it.

ICSharpCode.NRefactory.CSharp/OutputVisitor/InsertParenthesesVisitor.cs

@@ -235,7 +235,7 @@ namespace ICSharpCode.NRefactory.CSharp
 					ParenthesizeIfRequired(binaryOperatorExpression.Left, Primary);
 					ParenthesizeIfRequired(binaryOperatorExpression.Right, Primary);
 				} else {
-					// ?? is right-associate
+					// ?? is right-associative
 					ParenthesizeIfRequired(binaryOperatorExpression.Left, precedence + 1);
 					ParenthesizeIfRequired(binaryOperatorExpression.Right, precedence);
 				}

ICSharpCode.NRefactory.CSharp/Resolver/TypeInference.cs

@@ -624,6 +624,11 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 				tp.LowerBounds.Add(U);
 				return;
 			}
+			// Handle nullable covariance:
+			if (NullableType.IsNullable(U) && NullableType.IsNullable(V)) {
+				MakeLowerBoundInference(NullableType.GetUnderlyingType(U), NullableType.GetUnderlyingType(V));
+				return;
+			}
 			
 			// Handle array types:
 			ArrayType arrU = U as ArrayType;
@@ -776,28 +781,6 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 				Log.WriteLine("  T was fixed " + (types.Count >= 1 ? "successfully" : "(with errors)") + " to " + tp.FixedTo);
 				return types.Count >= 1;
 			} else {
-				if (types.Count > 1) {
-					// Try to search a unique type among the candidate types from which there is an implicit conversion
-					// to all other candate types.
-					IType uniqueType = null;
-					for (int i = 0; i < types.Count; i++) {
-						if (types.All (t => conversions.ImplicitConversion (t, types[i]).IsValid)) {
-							if (uniqueType != null) {
-								Log.WriteLine("Can't determine a single unique type!");
-								uniqueType = null;
-								break;
-							}
-							uniqueType = types[i];
-						}
-					}
-					if (uniqueType != null) {
-						tp.FixedTo = uniqueType;
-						Log.WriteLine("  T was fixed successfully to " + tp.FixedTo);
-						return true;
-					}
-					// fixing with errors
-				}
-
 				tp.FixedTo = GetFirstTypePreferNonInterfaces(types);
 				Log.WriteLine("  T was fixed " + (types.Count == 1 ? "successfully" : "(with errors)") + " to " + tp.FixedTo);
 				return types.Count == 1;
@@ -880,6 +863,11 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 				.Where(c => upperBounds.All(b => conversions.ImplicitConversion(c, b).IsValid))
 				.ToList(); // evaluate the query only once
 
+			Log.WriteCollection("FindTypesInBound, Candidates=", candidateTypes);
+			
+			// According to the C# specification, we need to pick the most specific
+			// of the candidate types. (the type which has conversions to all others)
+			// However, csc actually seems to choose the least specific.
 			candidateTypes = candidateTypes.Where(
 				c => candidateTypes.All(o => conversions.ImplicitConversion(o, c).IsValid)
 			).ToList();
@@ -939,8 +927,8 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 				}
 				Log.WriteLine("Candidate type: " + candidate);
 				
-				if (lowerBounds.Count > 0) {
-					// if there were lower bounds, we aim for the most specific candidate:
+				if (upperBounds.Count == 0) {
+					// if there were only lower bounds, we aim for the most specific candidate:
 					
 					// if this candidate isn't made redundant by an existing, more specific candidate:
 					if (!candidateTypes.Any(c => c.GetDefinition().IsDerivedFrom(candidateDef))) {
@@ -950,7 +938,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 						candidateTypes.Add(candidate);
 					}
 				} else {
-					// if there only were upper bounds, we aim for the least specific candidate:
+					// if there were upper bounds, we aim for the least specific candidate:
 					
 					// if this candidate isn't made redundant by an existing, less specific candidate:
 					if (!candidateTypes.Any(c => candidateDef.IsDerivedFrom(c.GetDefinition()))) {

ICSharpCode.NRefactory.Tests/CSharp/Resolver/TypeInferenceTests.cs

@@ -21,7 +21,7 @@ using System.Collections;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Linq;
-
+using System.Runtime.CompilerServices;
 using ICSharpCode.NRefactory.Semantics;
 using ICSharpCode.NRefactory.TypeSystem;
 using ICSharpCode.NRefactory.TypeSystem.Implementation;
@@ -32,7 +32,6 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 	[TestFixture]
 	public class TypeInferenceTests : ResolverTestBase
 	{
-		readonly ICompilation compilation = new SimpleCompilation(CecilLoaderTests.Mscorlib);
 		TypeInference ti;
 		
 		[SetUp]
@@ -456,12 +455,65 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 		public void CommonSubTypeIEnumerableClonableIEnumerableComparableList()
 		{
 			Assert.AreEqual(
-				Resolve(typeof(List<string>), typeof(List<Version>), typeof(Collection<string>), typeof(Collection<Version>), typeof(ReadOnlyCollection<string>), typeof(ReadOnlyCollection<Version>)),
+				Resolve(typeof(List<string>), typeof(List<Version>), typeof(Collection<string>), typeof(Collection<Version>),
+				        typeof(ReadOnlyCollectionBuilder<string>), typeof(ReadOnlyCollectionBuilder<Version>),
+				        typeof(ReadOnlyCollection<string>), typeof(ReadOnlyCollection<Version>)),
 				FindAllTypesInBounds(Resolve(), Resolve(typeof(IEnumerable<ICloneable>), typeof(IEnumerable<IComparable>), typeof(IList))));
 		}
 		#endregion
 
-
+		[Test]
+		public void NullablePick()
+		{
+				string program = @"
+interface ICo<out T> {}
+interface IContra<in T> {}
+class Test
+{
+	static T Pick<T> (T? a, T? b)
+	{
+		return a;
+	}
+	public static void Test(int? i, long? l)
+	{
+		$Pick(i, l)$;
+	}
+}
+";
+			var mrr = Resolve<CSharpInvocationResolveResult>(program);
+			Assert.AreEqual("System.Int64", mrr.Type.FullName);
+			Assert.IsFalse(mrr.IsError);
+		}
+		
+		[Test]
+		public void CoContraPick()
+		{
+			string program = @"
+interface ICo<out T> {}
+interface IContra<in T> {}
+class Test
+{
+	static T Pick<T> (ICo<T> a, IContra<T> b)
+	{
+		return a;
+	}
+	public static void Test(ICo<string> i, IContra<object> l)
+	{
+		$Pick(i, l)$;
+	}
+}
+";
+			// String and Object are both valid choices; and csc ends up picking object,
+			// even though the C# specification says it should pick string:
+			// 7.5.2.11 Fixing - both string and object are in the candidate set;
+			// string has a conversion to object (the other candidate),
+			// object doesn't have that; so string should be chosen as the result.
+			
+			// We follow the csc behavior.
+			var mrr = Resolve<CSharpInvocationResolveResult>(program);
+			Assert.AreEqual("System.Object", mrr.Type.FullName);
+			Assert.IsFalse(mrr.IsError);
+		}
 		
 		/// <summary>
 		/// Bug 9300 - Unknown Resolve Error
@@ -488,18 +540,17 @@ class C : I<string>
 		return a;
 	}
 	
-	public static int Main ()
+	public static void Main ()
 	{
 		S s = new S ();
 		I<string> i = new C ();
-		var result = Foo (s, i);
-		Console.WriteLine ($result$);
-		return 0;
+		var result = $Foo (s, i)$;
 	}
 }
 ";
-			var mrr = Resolve<LocalResolveResult>(program);
+			var mrr = Resolve<CSharpInvocationResolveResult>(program);
 			Assert.AreEqual("System.String", mrr.Type.FullName);
+			Assert.IsFalse(mrr.IsError);
 		}
 	}
 }