code cleanup prior to optional interop improvements (#7276)

* add test cases we need to make work

* cleanup method call argument processing

Author: dsyme

src/fsharp/ConstraintSolver.fs

@@ -1931,7 +1931,7 @@ and SolveTypeIsNonNullableValueType (csenv: ConstraintSolverEnv) ndeep m2 trace
         | _ ->
             let underlyingTy = stripTyEqnsAndMeasureEqns g ty
             if isStructTy g underlyingTy then
-                if isAppTy g underlyingTy && tyconRefEq g g.system_Nullable_tcref (tcrefOfAppTy g underlyingTy) then
+                if isNullableTy g underlyingTy then
                     return! ErrorD (ConstraintSolverError(FSComp.SR.csTypeParameterCannotBeNullable(), m, m))
             else
                 return! ErrorD (ConstraintSolverError(FSComp.SR.csGenericConstructRequiresStructType(NicePrint.minimalStringOfType denv ty), m, m2))

src/fsharp/MethodCalls.fs

@@ -839,16 +839,6 @@ let tryNiceEntityRefOfTyOption ty =
     | TType_app (tcref, _) -> Some tcref
     | TType_measure (Measure.Con tcref) -> Some tcref
     | _ -> None
-
-let (|NullableTy|_|) g ty =
-    match tryAppTy g ty with 
-    | ValueSome (tcref, [tyarg]) when tyconRefEq g tcref g.system_Nullable_tcref -> Some tyarg
-    | _ -> None
-
-let (|StripNullableTy|) g ty = 
-    match tryAppTy g ty with 
-    | ValueSome (tcref, [tyarg]) when tyconRefEq g tcref g.system_Nullable_tcref -> tyarg
-    | _ -> ty
 
 let mkInstForAppTy g ty = 
     match tryAppTy g ty with
@@ -3125,6 +3115,31 @@ let destOptionTy g ty =
     | ValueSome ty -> ty
     | ValueNone -> failwith "destOptionTy: not an option type"
 
+let isNullableTy (g: TcGlobals) ty = 
+    match tryDestAppTy g ty with 
+    | ValueNone -> false
+    | ValueSome tcref -> tyconRefEq g g.system_Nullable_tcref tcref
+
+let tryDestNullableTy g ty = 
+    match argsOfAppTy g ty with 
+    | [ty1] when isNullableTy g ty -> ValueSome ty1
+    | _ -> ValueNone
+
+let destNullableTy g ty = 
+    match tryDestNullableTy g ty with 
+    | ValueSome ty -> ty
+    | ValueNone -> failwith "destNullableTy: not a Nullable type"
+
+let (|NullableTy|_|) g ty =
+    match tryAppTy g ty with 
+    | ValueSome (tcref, [tyarg]) when tyconRefEq g tcref g.system_Nullable_tcref -> Some tyarg
+    | _ -> None
+
+let (|StripNullableTy|) g ty = 
+    match tryDestNullableTy g ty with 
+    | ValueSome tyarg -> tyarg
+    | _ -> ty
+
 let isLinqExpressionTy g ty = 
     match tryDestAppTy g ty with 
     | ValueNone -> false

src/fsharp/TastOps.fs

@@ -1388,7 +1388,7 @@ val mkVoidPtrTy  : TcGlobals -> TType
 /// Build a single-dimensional array type
 val mkArrayType      : TcGlobals -> TType -> TType
 
-/// Determine is a type is an option type
+/// Determine if a type is an option type
 val isOptionTy     : TcGlobals -> TType -> bool
 
 /// Take apart an option type
@@ -1397,6 +1397,15 @@ val destOptionTy   : TcGlobals -> TType -> TType
 /// Try to take apart an option type
 val tryDestOptionTy : TcGlobals -> TType -> ValueOption<TType>
 
+/// Determine is a type is a System.Nullable type
+val isNullableTy : TcGlobals -> TType -> bool
+
+/// Try to take apart a System.Nullable type
+val tryDestNullableTy: TcGlobals -> TType -> ValueOption<TType>
+
+/// Take apart a System.Nullable type
+val destNullableTy: TcGlobals -> TType -> TType
+
 /// Determine if a type is a System.Linq.Expression type
 val isLinqExpressionTy     : TcGlobals -> TType -> bool
 

src/fsharp/TastOps.fsi

@@ -124,7 +124,7 @@
       </trans-unit>
       <trans-unit id="followingPatternMatchClauseHasWrongType">
         <source>All branches of a pattern match expression must return values of the same type as the first branch, which here is '{0}'. This branch returns a value of type '{1}'.</source>
-        <target state="translated">Todas las ramas de una expresión de coincidencia de patrón deben devolver valores del mismo tipo. La primera rama devolvió un valor de tipo "{0}", pero esta rama devolvió un valor de tipo "\{1 \}".</target>
+        <target state="new">All branches of a pattern match expression must return values of the same type as the first branch, which here is '{0}'. This branch returns a value of type '{1}'.</target>
         <note />
       </trans-unit>
       <trans-unit id="patternMatchGuardIsNotBool">

src/fsharp/TypeChecker.fs

@@ -24,3 +24,29 @@ public static void SomeMethod() { }
 
     }
 }
+namespace CSharpOptionalParameters
+{
+    // This should be preferred over the same type in lib2.cs
+    public class SomeClass
+    {
+        public SomeClass() { }
+        public static int MethodTakingOptionals(int x = 3, string y = "abc", double d = 5.0)
+        {
+            return x + y.Length + (int) d;
+        }
+        public static int MethodTakingNullableOptionalsWithDefaults(int? x = 3, string y = "abc", double? d = 5.0)
+        {
+            return (x.HasValue ? x.Value : -100) + y.Length + (int) (d.HasValue ? d.Value : 0.0);
+        }
+        public static int MethodTakingNullableOptionals(int? x = null, string y = null, double? d = null)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0);
+        }
+    }
+
+}

src/fsharp/xlf/FSComp.txt.es.xlf

@@ -71,22 +71,53 @@ let _ = test "structunion394b36" (Lib.NestedStructUnionsTests.testPattern3mut(u2
 
 // F# option implicit converter tests
 
-let testFsOpt() =
-    let testOpt (t : 'T option) =
-        test (sprintf "fsimplicitconv (%A)" t) (ApiWrapper.ConsumeOptionalParam<'T>(t) = t)
-
-    testOpt(Option<int>.None)
-    testOpt(Some 42)
-
-    // check that implicit conversion of optionals does 
-    // differentiate between 'null' and 'Some null'
-    testOpt(Option<string>.None)
-    testOpt(Option<string>.Some null)
-    testOpt(Some "")
-    testOpt(Some "test")
-
-testFsOpt()
-
+module TestConsumeOptionalParameter = 
+    let testFsOpt() =
+        let testOpt (t : 'T option) =
+            test (sprintf "fsimplicitconv (%A)" t) (ApiWrapper.ConsumeOptionalParam<'T>(t) = t)
+
+        testOpt(Option<int>.None)
+        testOpt(Some 42)
+
+        // check that implicit conversion of optionals does 
+        // differentiate between 'null' and 'Some null'
+        testOpt(Option<string>.None)
+        testOpt(Option<string>.Some null)
+        testOpt(Some "")
+        testOpt(Some "test")
+
+    testFsOpt()
+
+module TestConsumeCSharpOptionalParameter = 
+    open System
+    open CSharpOptionalParameters
+    check "csoptional23982f31" (SomeClass.MethodTakingOptionals()) 11
+    check "csoptional23982f32" (SomeClass.MethodTakingOptionals(x = 6)) 14
+    check "csoptional23982f33" (SomeClass.MethodTakingOptionals(y = "aaaaaa")) 14
+    check "csoptional23982f34" (SomeClass.MethodTakingOptionals(d = 8.0)) 14
+
+    check "csoptional23982f41" (SomeClass.MethodTakingNullableOptionalsWithDefaults()) 11
+    check "csoptional23982f42" (SomeClass.MethodTakingNullableOptionalsWithDefaults(x = Nullable 6)) 14
+    check "csoptional23982f43" (SomeClass.MethodTakingNullableOptionalsWithDefaults(y = "aaaaaa")) 14
+    check "csoptional23982f44" (SomeClass.MethodTakingNullableOptionalsWithDefaults(d = Nullable 8.0)) 14
+
+    check "csoptional23982f51" (SomeClass.MethodTakingNullableOptionals()) -3
+    check "csoptional23982f52" (SomeClass.MethodTakingNullableOptionals(x = Nullable 6)) 4
+    check "csoptional23982f53" (SomeClass.MethodTakingNullableOptionals(y = "aaaaaa")) 4
+    check "csoptional23982f54" (SomeClass.MethodTakingNullableOptionals(d = Nullable 8.0)) 6
+
+    // These require https://github.com/fsharp/fslang-suggestions/issues/774 to be implemented
+    //check "csoptional23982f3no" (SomeClass.SomeMethod(?x = Some 6)) 14
+    //check "csoptional23982f3no" (SomeClass.SomeMethod(?y = Some "aaaaaa")) 14
+    //check "csoptional23982f3no" (SomeClass.SomeMethod(?d = Some 8.0)) 14
+    //check "csoptional23982f3no" (SomeClass.SomeMethod(?x = None)) 11
+    //check "csoptional23982f3no" (SomeClass.SomeMethod(?y = None)) 11
+    //check "csoptional23982f3no" (SomeClass.SomeMethod(?d = None)) 11
+
+    //check "csoptional23982f42" (SomeClass.MethodTakingNullableOptionalsWithDefaults(x = 6)) 14
+    //check "csoptional23982f44" (SomeClass.MethodTakingNullableOptionalsWithDefaults(d = 8.0)) 14
+    //check "csoptional23982f52" (SomeClass.MethodTakingNullableOptionals(x = 6)) 4
+    //check "csoptional23982f54" (SomeClass.MethodTakingNullableOptionals(d = 8.0)) 6
 
 module NestedStructPatternMatchingAcrossAssemblyBoundaries = 
     open Lib.NestedStructUnionsTests

tests/fsharp/core/fsfromfsviacs/lib3.cs

@@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
       </trans-unit>
       <trans-unit id="RSE_GraphicSizeFormat">
         <source>{0} x {1}</source>
-        <target state="translated">{0} x {1}</target>
+        <target state="needs-review-translation">{0} x {1}</target>
         <note>Format string for showing a graphic's size
+
 #  {0} = width (as an integer)
 #  {1} = height (as an integer)
 #Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>

tests/fsharp/core/fsfromfsviacs/test.fsx

@@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
       </trans-unit>
       <trans-unit id="RSE_GraphicSizeFormat">
         <source>{0} x {1}</source>
-        <target state="translated">{0} x {1}</target>
+        <target state="needs-review-translation">{0} x {1}</target>
         <note>Format string for showing a graphic's size
+
 #  {0} = width (as an integer)
 #  {1} = height (as an integer)
 #Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>