Nullness - generated ToString() members should override object and return string WithoutNull

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

@@ -19,6 +19,7 @@
 ### Added
 
 * Support for nullable reference types ([PR #15181](https://github.com/dotnet/fsharp/pull/15181))
+* Treat .ToString() on F# types as returning non-nullable string in --checknulls+ context ([PR #17547](https://github.com/dotnet/fsharp/pull/17547))
 * Parser: recover on missing union case fields (PR [#17452](https://github.com/dotnet/fsharp/pull/17452))
 * Parser: recover on missing union case field types (PR [#17455](https://github.com/dotnet/fsharp/pull/17455))
 * Sink: report function domain type ([PR #17470](https://github.com/dotnet/fsharp/pull/17470))

src/Compiler/AbstractIL/ilnativeres.fs

@@ -1154,7 +1154,7 @@ type NativeResourceWriter() =
                             "Unknown entry %s"
                             (match e with
                              | null -> "<NULL>"
-                             | e -> e.GetType().FullName)
+                             | e -> e.GetType().FullName |> string)
 
                 if id >= 0 then
                     writer.WriteInt32 id

src/Compiler/Checking/AccessibilityLogic.fs

@@ -351,9 +351,10 @@ let CheckILFieldInfoAccessible g amap m ad finfo =
 /// when calling x.SomeMethod() we need to use 'adTyp' do verify that type of x is accessible from C 
 /// and 'ad' to determine accessibility of SomeMethod.
 /// I.e when calling x.Public() and x.Protected() -in both cases first check should succeed and second - should fail in the latter one. 
-let IsTypeAndMethInfoAccessible amap m accessDomainTy ad = function
+let rec IsTypeAndMethInfoAccessible amap m accessDomainTy ad = function
     | ILMeth (g, x, _) -> IsILMethInfoAccessible g amap m accessDomainTy ad x 
     | FSMeth (_, _, vref, _) -> IsValAccessible ad vref
+    | MethInfoWithModifiedReturnType(mi,_) -> IsTypeAndMethInfoAccessible amap m accessDomainTy ad mi
     | DefaultStructCtor(g, ty) -> IsTypeAccessible g amap m ad ty
 #if !NO_TYPEPROVIDERS
     | ProvidedMeth(amap, tpmb, _, m) as etmi -> 

src/Compiler/Checking/AttributeChecking.fs

@@ -152,10 +152,11 @@ let GetAttribInfosOfEntity g amap m (tcref:TyconRef) =
         tcref.Attribs |> List.map (fun a -> FSAttribInfo (g, a))
 
 
-let GetAttribInfosOfMethod amap m minfo = 
+let rec GetAttribInfosOfMethod amap m minfo = 
     match minfo with 
     | ILMeth (g, ilminfo, _) -> ilminfo.RawMetadata.CustomAttrs  |> AttribInfosOfIL g amap ilminfo.MetadataScope m
     | FSMeth (g, _, vref, _) -> vref.Attribs |> AttribInfosOfFS g 
+    | MethInfoWithModifiedReturnType(mi,_) -> GetAttribInfosOfMethod amap m mi
     | DefaultStructCtor _ -> []
 #if !NO_TYPEPROVIDERS
     // TODO: provided attributes
@@ -186,11 +187,12 @@ let GetAttribInfosOfEvent amap m einfo =
 
 /// Analyze three cases for attributes declared on methods: IL-declared attributes, F#-declared attributes and
 /// provided attributes.
-let BindMethInfoAttributes m minfo f1 f2 f3 = 
+let rec BindMethInfoAttributes m minfo f1 f2 f3 = 
     ignore m; ignore f3
     match minfo with 
     | ILMeth (_, x, _) -> f1 x.RawMetadata.CustomAttrs 
     | FSMeth (_, _, vref, _) -> f2 vref.Attribs
+    | MethInfoWithModifiedReturnType(mi,_) -> BindMethInfoAttributes m mi f1 f2 f3
     | DefaultStructCtor _ -> f2 []
 #if !NO_TYPEPROVIDERS
     | ProvidedMeth (_, mi, _, _) -> f3 (mi.PApply((fun st -> (st :> IProvidedCustomAttributeProvider)), m))

src/Compiler/Checking/CheckDeclarations.fs

@@ -4526,7 +4526,6 @@ module TcDeclarations =
             core, extra_vals_Inherits_Abstractslots @ extraMembers
 
     //-------------------------------------------------------------------------
-
     /// Bind a collection of mutually recursive definitions in an implementation file
     let TcMutRecDefinitions (cenv: cenv) envInitial parent typeNames tpenv m scopem mutRecNSInfo (mutRecDefns: MutRecDefnsInitialData) isMutRec =
 
@@ -5888,12 +5887,12 @@ let CheckOneSigFile (g, amap, thisCcu, checkForErrors, conditionalDefines, tcSin
             tcComputationExpression=TcComputationExpression)
 
     let envinner, moduleTyAcc = MakeInitialEnv tcEnv 
-
+    let m = sigFile.QualifiedName.Range
     let specs = [ for x in sigFile.Contents -> SynModuleSigDecl.NamespaceFragment x ]
-    let! tcEnv = TcSignatureElements cenv ParentNone sigFile.QualifiedName.Range envinner PreXmlDoc.Empty None specs
-
-    let sigFileType = moduleTyAcc.Value
+    let! tcEnv = TcSignatureElements cenv ParentNone m envinner PreXmlDoc.Empty None specs
 
+    let sigFileType = moduleTyAcc.Value    
+
     if not (checkForErrors()) then  
         try
             sigFileType |> IterTyconsOfModuleOrNamespaceType (fun tycon ->

src/Compiler/Checking/ConstraintSolver.fs

@@ -2132,6 +2132,8 @@ and MemberConstraintSolutionOfMethInfo css m minfo minst staticTyOpt =
     | FSMeth(_, ty, vref, _) ->  
        FSMethSln(ty, vref, minst, staticTyOpt)
 
+    | MethInfoWithModifiedReturnType(mi,_) -> MemberConstraintSolutionOfMethInfo css m mi minst staticTyOpt
+
     | MethInfo.DefaultStructCtor _ -> 
        error(InternalError("the default struct constructor was the unexpected solution to a trait constraint", m))
 

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -11683,6 +11683,10 @@ and ApplyAbstractSlotInference (cenv: cenv) (envinner: TcEnv) (_: Val option) (a
 
                      let declaredTypars = (if typarsFromAbsSlotAreRigid then typarsFromAbsSlot else declaredTypars)
 
+                     // Overrides can narrow the retTy from nullable to not-null.
+                     // By changing nullness to be variable we do not get in the way of eliminating nullness (=good).
+                     let retTyFromAbsSlot = retTyFromAbsSlot |> changeWithNullReqTyToVariable g
+
                      let absSlotTy = mkMethodTy g argTysFromAbsSlot retTyFromAbsSlot
 
                      UnifyTypes cenv envinner m argsAndRetTy absSlotTy

src/Compiler/Checking/InfoReader.fs

@@ -1191,7 +1191,8 @@ let GetXmlDocSigOfUnionCaseRef (ucref: UnionCaseRef) =
         ucref.UnionCase.XmlDocSig <- XmlDocSigOfUnionCase [tcref.CompiledRepresentationForNamedType.FullName; ucref.CaseName]
     Some (ccuFileName, ucref.UnionCase.XmlDocSig)
 
-let GetXmlDocSigOfMethInfo (infoReader: InfoReader)  m (minfo: MethInfo) = 
+[<TailCall>]
+let rec GetXmlDocSigOfMethInfo (infoReader: InfoReader)  m (minfo: MethInfo) = 
     let amap = infoReader.amap
     match minfo with
     | FSMeth (g, _, vref, _) ->
@@ -1219,6 +1220,7 @@ let GetXmlDocSigOfMethInfo (infoReader: InfoReader)  m (minfo: MethInfo) =
 
             Some (ccuFileName, "M:"+actualTypeName+"."+normalizedName+genericArity+XmlDocArgsEnc g (formalTypars, fmtps) args)
 
+    | MethInfoWithModifiedReturnType(mi,_) -> GetXmlDocSigOfMethInfo infoReader m mi
     | DefaultStructCtor(g, ty) ->
         match tryTcrefOfAppTy g ty with
         | ValueSome tcref ->

src/Compiler/Checking/MethodCalls.fs

@@ -523,6 +523,19 @@ type CalledMeth<'T>
        staticTyOpt: TType option)    
     =
     let g = infoReader.g
+
+    let minfo =
+        match callerObjArgTys,minfo with
+        | objTy :: [], ILMeth _ when             
+            g.checkNullness 
+            && minfo.DisplayName = "ToString"
+            && minfo.IsNullary
+            && (isAnonRecdTy g objTy || isRecdTy g objTy || isUnionTy g objTy)
+            && (  typeEquiv g g.obj_ty_noNulls minfo.ApparentEnclosingAppType
+               || typeEquiv g g.system_Value_ty minfo.ApparentEnclosingAppType)  -> 
+                MethInfoWithModifiedReturnType(minfo, g.string_ty)
+        | _ -> minfo
+
     let methodRetTy = if minfo.IsConstructor then minfo.ApparentEnclosingType else minfo.GetFSharpReturnType(infoReader.amap, m, calledTyArgs)
 
     let fullCurriedCalledArgs = MakeCalledArgs infoReader.amap m minfo calledTyArgs
@@ -1039,7 +1052,7 @@ let BuildFSharpMethodCall g m (ty, vref: ValRef) valUseFlags minst args =
 
 /// Make a call to a method info. Used by the optimizer and code generator to build 
 /// calls to the type-directed solutions to member constraints.
-let MakeMethInfoCall (amap: ImportMap) m (minfo: MethInfo) minst args staticTyOpt =
+let rec MakeMethInfoCall (amap: ImportMap) m (minfo: MethInfo) minst args staticTyOpt =
     let g = amap.g
     let ccallInfo = ComputeConstrainedCallInfo g amap m staticTyOpt args minfo
     let valUseFlags = 
@@ -1059,6 +1072,8 @@ let MakeMethInfoCall (amap: ImportMap) m (minfo: MethInfo) minst args staticTyOp
     | FSMeth(g, ty, vref, _) -> 
         BuildFSharpMethodCall g m (ty, vref) valUseFlags minst args |> fst
 
+    | MethInfoWithModifiedReturnType(mi,_) -> MakeMethInfoCall amap m mi minst args staticTyOpt
+
     | DefaultStructCtor(_, ty) -> 
        mkDefault (m, ty)
 
@@ -1108,7 +1123,7 @@ let TryImportProvidedMethodBaseAsLibraryIntrinsic (amap: Import.ImportMap, m: ra
 //   minst: the instantiation to apply for a generic method 
 //   objArgs: the 'this' argument, if any 
 //   args: the arguments, if any 
-let BuildMethodCall tcVal g amap isMutable m isProp minfo valUseFlags minst objArgs args staticTyOpt =
+let rec BuildMethodCall tcVal g amap isMutable m isProp minfo valUseFlags minst objArgs args staticTyOpt =
     let direct = IsBaseCall objArgs
 
     TakeObjAddrForMethodCall g amap minfo isMutable m staticTyOpt objArgs (fun ccallInfo objArgs -> 
@@ -1181,6 +1196,11 @@ let BuildMethodCall tcVal g amap isMutable m isProp minfo valUseFlags minst objA
             let vExpr, vExprTy = tcVal vref valUseFlags (minfo.DeclaringTypeInst @ minst) m
             BuildFSharpMethodApp g m vref vExpr vExprTy allArgs
 
+        | MethInfoWithModifiedReturnType(mi,retTy) ->
+            let expr, exprTy = BuildMethodCall tcVal g amap isMutable m isProp mi valUseFlags minst objArgs args staticTyOpt
+            let expr = mkCoerceExpr(expr, retTy, m, exprTy)
+            expr, retTy
+
         // Build a 'call' to a struct default constructor 
         | DefaultStructCtor (g, ty) -> 
             if g.langFeatureNullness && g.checkNullness then

src/Compiler/Checking/MethodOverrides.fs

@@ -148,6 +148,12 @@ module DispatchSlotChecking =
         let _, _, argInfos, retTy, _ = GetTypeOfMemberInMemberForm g overrideBy
         let nm = overrideBy.LogicalName
 
+        if g.checkNullness && nm = "ToString" && (argInfos |> List.sumBy _.Length) = 0 && retTy.IsSome then
+            let returnsString = typeEquiv g retTy.Value g.string_ty
+            let retTyNullness = (nullnessOfTy g retTy.Value).TryEvaluate()
+            if returnsString && retTyNullness = ValueSome(NullnessInfo.WithNull) then
+                warning(Error(FSComp.SR.tcNullableToStringOverride(), overrideBy.Range))
+
         let argTys = argInfos |> List.mapSquared fst
 
         let memberMethodTypars, memberToParentInst, argTys, retTy = 

src/Compiler/Checking/NameResolution.fs

@@ -678,13 +678,14 @@ let IntrinsicMethInfosOfType (infoReader: InfoReader) optFilter ad allowMultiInt
     let minfos = minfos |> ExcludeHiddenOfMethInfos g amap m
     minfos
 
-let TrySelectExtensionMethInfoOfILExtMem m amap apparentTy (actualParent, minfo, pri) = 
+let rec TrySelectExtensionMethInfoOfILExtMem m amap apparentTy (actualParent, minfo, pri) = 
     match minfo with 
     | ILMeth(_,ilminfo,_) -> 
         MethInfo.CreateILExtensionMeth (amap, m, apparentTy, actualParent, Some pri, ilminfo.RawMetadata) |> Some
     // F#-defined IL-style extension methods are not seen as extension methods in F# code
     | FSMeth(g,_,vref,_) -> 
         FSMeth(g, apparentTy, vref, Some pri) |> Some
+    | MethInfoWithModifiedReturnType(mi,_) -> TrySelectExtensionMethInfoOfILExtMem m amap apparentTy (actualParent, mi, pri)
 #if !NO_TYPEPROVIDERS
     // // Provided extension methods are not yet supported
     | ProvidedMeth(amap,providedMeth,_,m) -> 

src/Compiler/Checking/NicePrint.fs

@@ -1734,7 +1734,7 @@ module InfoMemberPrinting =
     //
     // For C# extension members:
     //          ApparentContainer.Method(argName1: argType1, ..., argNameN: argTypeN) : retType
-    let prettyLayoutOfMethInfoFreeStyle (infoReader: InfoReader) m denv typarInst methInfo =
+    let rec prettyLayoutOfMethInfoFreeStyle (infoReader: InfoReader) m denv typarInst methInfo =
         let amap = infoReader.amap
 
         match methInfo with 
@@ -1745,6 +1745,12 @@ module InfoMemberPrinting =
         | FSMeth(_, _, vref, _) -> 
             let prettyTyparInst, resL = PrintTastMemberOrVals.prettyLayoutOfValOrMember { denv with showMemberContainers=true } infoReader typarInst vref
             prettyTyparInst, resL
+        | MethInfoWithModifiedReturnType(ILMeth(_, ilminfo, _) as wrappedInfo,retTy) -> 
+            let prettyTyparInst, prettyMethInfo, minst = prettifyILMethInfo amap m wrappedInfo typarInst ilminfo
+            let prettyMethInfo = MethInfoWithModifiedReturnType(prettyMethInfo,retTy)
+            let resL = layoutMethInfoCSharpStyle amap m denv prettyMethInfo minst
+            prettyTyparInst, resL
+        | MethInfoWithModifiedReturnType(mi,_) -> prettyLayoutOfMethInfoFreeStyle infoReader m denv typarInst mi
         | ILMeth(_, ilminfo, _) -> 
             let prettyTyparInst, prettyMethInfo, minst = prettifyILMethInfo amap m methInfo typarInst ilminfo
             let resL = layoutMethInfoCSharpStyle amap m denv prettyMethInfo minst