Bugfix :: Nullness :: Add warnings for instantiations of (T|null) which are not allowed in F# (frequent at Deserialize from S.T.Json) (#18057)

* basic check

* handle units of measure

* fantomas

* test expectation adjusted

* Update src/Compiler/FSComp.txt

Co-authored-by: Vlad Zarytovskii <[email protected]>

* change wording

---------

Co-authored-by: Vlad Zarytovskii <[email protected]>

Author: T-Gro

FSharp.sln

@@ -48,6 +48,8 @@ Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "TestTP", "tests\service\dat
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Docs", "Docs", "{4E4F41D9-86A7-4F5D-B735-1A0744AB68AC}"
 	ProjectSection(SolutionItems) = preProject
+		docs\builder-caches.md = docs\builder-caches.md
+		docs\changing-the-ast.md = docs\changing-the-ast.md
 		docs\coding-standards.md = docs\coding-standards.md
 		docs\compiler-startup-performance.md = docs\compiler-startup-performance.md
 		docs\debug-emit.md = docs\debug-emit.md
@@ -57,12 +59,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Docs", "Docs", "{4E4F41D9-8
 		docs\index.md = docs\index.md
 		docs\large-inputs-and-stack-overflows.md = docs\large-inputs-and-stack-overflows.md
 		docs\memory-usage.md = docs\memory-usage.md
-		docs\optimizations.md = docs\optimizations.md
+		docs\names.md = docs\names.md
 		docs\optimizations-equality.md = docs\optimizations-equality.md
+		docs\optimizations.md = docs\optimizations.md
 		docs\overview.md = docs\overview.md
-		docs\builder-caches.md = docs\builder-caches.md
-		docs\changing-the-ast.md = docs\changing-the-ast.md
-		docs\names.md = docs\names.md
 		docs\perf-discussions-archive.md = docs\perf-discussions-archive.md
 		docs\project-builds.md = docs\project-builds.md
 		docs\representations.md = docs\representations.md
@@ -84,8 +84,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "fcs", "fcs", "{B86EBFF1-E03
 		docs\fcs\symbols.fsx = docs\fcs\symbols.fsx
 		docs\fcs\tokenizer.fsx = docs\fcs\tokenizer.fsx
 		docs\fcs\typedtree.fsx = docs\fcs\typedtree.fsx
-		docs\fcs\untypedtree.fsx = docs\fcs\untypedtree.fsx
 		docs\fcs\untypedtree-apis.fsx = docs\fcs\untypedtree-apis.fsx
+		docs\fcs\untypedtree.fsx = docs\fcs\untypedtree.fsx
 	EndProjectSection
 EndProject
 Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "fsc", "src\fsc\fscProject\fsc.fsproj", "{10D15DBB-EFF0-428C-BA83-41600A93EEC4}"
@@ -142,6 +142,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".FSharp.Compiler.Service",
 		docs\release-notes\.FSharp.Compiler.Service\8.0.200.md = docs\release-notes\.FSharp.Compiler.Service\8.0.200.md
 		docs\release-notes\.FSharp.Compiler.Service\8.0.202.md = docs\release-notes\.FSharp.Compiler.Service\8.0.202.md
 		docs\release-notes\.FSharp.Compiler.Service\8.0.300.md = docs\release-notes\.FSharp.Compiler.Service\8.0.300.md
+		docs\release-notes\.FSharp.Compiler.Service\8.0.400.md = docs\release-notes\.FSharp.Compiler.Service\8.0.400.md
+		docs\release-notes\.FSharp.Compiler.Service\9.0.100.md = docs\release-notes\.FSharp.Compiler.Service\9.0.100.md
+		docs\release-notes\.FSharp.Compiler.Service\9.0.200.md = docs\release-notes\.FSharp.Compiler.Service\9.0.200.md
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".FSharp.Core", ".FSharp.Core", "{23798638-A1E9-4DAE-9C9C-F5D87499ADD6}"
@@ -158,8 +161,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".Language", ".Language", "{
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".VisualStudio", ".VisualStudio", "{AF70EC5A-8E7C-4FDA-857D-AF08082CFC64}"
 	ProjectSection(SolutionItems) = preProject
-		docs\release-notes\.VisualStudio\17.9.md = docs\release-notes\.VisualStudio\17.9.md
 		docs\release-notes\.VisualStudio\17.10.md = docs\release-notes\.VisualStudio\17.10.md
+		docs\release-notes\.VisualStudio\17.9.md = docs\release-notes\.VisualStudio\17.9.md
 	EndProjectSection
 EndProject
 Global

docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

@@ -15,13 +15,15 @@
 * Fix locals allocating for the special `copyOfStruct` defensive copy ([PR #18025](https://github.com/dotnet/fsharp/pull/18025))
 * Fix lowering of computed array expressions when the expression consists of a simple mapping from a `uint64` or `unativeint` array. [PR #18081](https://github.com/dotnet/fsharp/pull/18081)
 
+
 ### Added
 
 * Let `dotnet fsi --help` print a link to the documentation website. ([PR #18006](https://github.com/dotnet/fsharp/pull/18006))
 * Deprecate places where `seq` can be omitted. ([Language suggestion #1033](https://github.com/fsharp/fslang-suggestions/issues/1033), [PR #17772](https://github.com/dotnet/fsharp/pull/17772))
 * Support literal attribute on decimals ([PR #17769](https://github.com/dotnet/fsharp/pull/17769))
 * Added type conversions cache, only enabled for compiler runs, guarded by language version preview ([PR #17668](https://github.com/dotnet/fsharp/pull/17668))
 * Added project property ParallelCompilation which turns on graph based type checking, parallel ILXGen and parallel optimization. By default on for users of langversion=preview ([PR #17948](https://github.com/dotnet/fsharp/pull/17948))
+* Adding warning when consuming generic method returning T|null for types not supporting nullness (structs,anons,tuples) ([PR #18057](https://github.com/dotnet/fsharp/pull/18057))
 
 ### Changed
 

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -9728,10 +9728,13 @@ and TcMethodApplicationThen
         let (CallerNamedArg(id, _)) = List.head attributeAssignedNamedItems
         errorR(Error(FSComp.SR.tcNamedArgumentDidNotMatch(id.idText), id.idRange))
 
-
     // Resolve the "delayed" lookups
     let exprTy = (tyOfExpr g expr)
 
+    for problematicTy in GetDisallowedNullness g exprTy do
+        let denv = env.DisplayEnv
+        warning(Error(FSComp.SR.tcDisallowedNullableApplication(methodName,NicePrint.minimalStringOfType denv problematicTy), m))
+
     PropagateThenTcDelayed cenv overallTy env tpenv mWholeExpr (MakeApplicableExprNoFlex cenv expr) exprTy atomicFlag delayed
 
 /// Infer initial type information at the callsite from the syntax of an argument, prior to overload resolution.

src/Compiler/FSComp.txt

@@ -1538,6 +1538,7 @@ tcPassingWithoutNullToNonNullQuickAP,"You can remove this |NonNullQuick| pattern
 tcPassingWithoutNullTononNullFunction,"You can remove this `nonNull` assertion."
 3263,tcNullableToStringOverride,"With nullness checking enabled, overrides of .ToString() method must return a non-nullable string. You can handle potential nulls via the built-in string function."
 3264,tcDowncastFromNullableToWithoutNull,"Nullness warning: Downcasting from '%s' into '%s' can introduce unexpected null values. Cast to '%s|null' instead or handle the null before downcasting."
+3265,tcDisallowedNullableApplication,"Application of method '%s' attempted to create a nullable type ('T | null) for '%s'. Nullness warnings won't be reported correctly for such types."
 3268,csNullNotNullConstraintInconsistent,"The constraints 'null' and 'not null' are inconsistent"
 3271,tcNullnessCheckingNotEnabled,"The 'nullness checking' language feature is not enabled. This use of a nullness checking construct will be ignored."
 csTypeHasNullAsTrueValue,"The type '%s' uses 'null' as a representation value but a non-null type is expected"

src/Compiler/TypedTree/TypedTreeOps.fs

@@ -9215,6 +9215,57 @@ let reqTyForArgumentNullnessInference g actualTy reqTy =
         changeWithNullReqTyToVariable g reqTy       
     | _ -> reqTy
 
+
+let GetDisallowedNullness (g:TcGlobals) (ty:TType) =
+    if g.checkNullness then
+        let rec hasWithNullAnyWhere ty alreadyWrappedInOuterWithNull = 
+            match ty with
+            | TType_var (tp, n) -> 
+                let withNull = alreadyWrappedInOuterWithNull || n.TryEvaluate() = (ValueSome NullnessInfo.WithNull)
+                match tp.Solution with
+                | None -> []
+                | Some t -> hasWithNullAnyWhere t withNull
+
+            | TType_app (tcr, tinst, nullnessOrig) -> 
+                let tyArgs = tinst |> List.collect (fun t -> hasWithNullAnyWhere t false)
+                
+                match alreadyWrappedInOuterWithNull, tcr.TypeAbbrev with
+                | true, _ when isStructTyconRef tcr -> ty :: tyArgs
+                | true, _ when tcr.IsMeasureableReprTycon -> 
+                    match tcr.TypeReprInfo with
+                    | TMeasureableRepr realType ->
+                        if hasWithNullAnyWhere realType true |> List.isEmpty then
+                            []
+                        else [ty]
+                    | _ -> []
+                | true, Some tAbbrev -> (hasWithNullAnyWhere tAbbrev true) @ tyArgs
+                | _ -> tyArgs
+
+            | TType_tuple (_,tupTypes) ->
+                let inner = tupTypes |> List.collect (fun t -> hasWithNullAnyWhere t false)
+                if alreadyWrappedInOuterWithNull then ty :: inner else inner
+
+            | TType_anon (anon,tys) -> 
+                let inner = tys |> List.collect (fun t -> hasWithNullAnyWhere t false)
+                if alreadyWrappedInOuterWithNull then ty :: inner else inner
+            | TType_fun (d, r, _) ->
+                (hasWithNullAnyWhere d false) @ (hasWithNullAnyWhere r false)
+
+            | TType_forall _ -> []
+            | TType_ucase _ -> []
+            | TType_measure m ->
+                if alreadyWrappedInOuterWithNull then 
+                    let measuresInside = 
+                        ListMeasureVarOccs m 
+                        |> List.choose (fun x -> x.Solution)
+                        |> List.collect (fun x -> hasWithNullAnyWhere x true)
+                    ty :: measuresInside
+                else []
+
+        hasWithNullAnyWhere ty false
+    else
+        []
+
 let TypeHasAllowNull (tcref:TyconRef) g m =
     not tcref.IsStructOrEnumTycon &&
     not (isByrefLikeTyconRef g m tcref) && 

src/Compiler/TypedTree/TypedTreeOps.fsi

@@ -1815,6 +1815,11 @@ val TypeNullIsTrueValue: TcGlobals -> TType -> bool
 
 val TypeNullIsExtraValue: TcGlobals -> range -> TType -> bool
 
+/// A type coming via interop from C# can be holding a nullness combination not supported in F#.
+/// Prime example are APIs marked as T|null applied to structs, tuples and anons.
+/// Unsupported values can also be nested within generic type arguments, e.g. a List<Tuple<string,T|null>> applied to an anon.
+val GetDisallowedNullness: TcGlobals -> TType -> TType list
+
 val TypeHasAllowNull: TyconRef -> TcGlobals -> range -> bool
 
 val TypeNullIsExtraValueNew: TcGlobals -> range -> TType -> bool