Let 'string' function return "None" if input is None

[abelbraaksma]

This is dependent on #9549, after that's merged, the diff will be much smaller.

This is a prototype for fsharp/fslang-suggestions#891, just to find out whether it's possible and gives the result we'd like. I'm totally aware that the language suggestion may or may not be accepted, just leaving this here "as is", and will close if we're not going to pursue this change.

This change is written from branch & PR of #9549, it includes those changes as well.

Marked as draft.

[KevinRansom]

@abelbraaksma , I really don't like this, I have thought about it several time.

I think of string as give me a string created from the arguments. The following examples behave exactly as I would expect. I would hate passing option types to this api and get back something other than an empty string for None.

let a = string null
let b = string None
let c = string ""
let d = string 1
let e = string "Hello"
let f = int 2

Currently yields:
val a : string = ""
val b : string = ""
val c : string = ""
val d : string = "1"
val e : string = "Hello"
val f : int = 2

It's also a breaking change.

Does this make sense?

Kevin

[abelbraaksma]

@KevinRansom, it does make sense, and I was aware it could get the response and consideration you just gave (hence "draft" and not yet fixing the build errors).

I understand the concerns (and don't have a good argument against them, other than parity with voption, which behaves now differently, unfortunately).

Btw, as @charlesroddie noticed, currently the behavior is all over the place, so whether this is really "as expected" I'm not sure:

string None // ""
None.ToString() // Null Reference Exception
sprintf "%A" None // "None"
sprintf "%O" None // "<null>"

That said, maybe we can turn this into a better debugging experience instead, where you currently see "null" when something is either None or null. This wouldn't be a breaking change, as it would only be used inside the DebuggerDisplay methods.

Can I kindly ask you to check my other PRs apologies, you already did, thanks!

[charlesroddie]

I think of string as give me a string created from the arguments.

@KevinRansom If I interpret you right, you are saying that None represents "nothing" so there are "no arguments" and string None should print any thing there, and there is no thing, so should print nothing, and printing nothing is to return the empty string. This would imply that Some 1 represents one thing, and has an argument 1 and so string(Some 1) should return "1", but it doesn't currently: it returns "Some(1)".

The idea of finding some "thing" isn't helpful in thinking about the option type: is there a "thing/argument" in Some None and what should string (Some None) return? It currently returns "Some(null)"! String printing of options just hasn't been thought through yet and is inconsistent and illogical - even considering the stringquasi-function in isolation and ignoring the myriad inconsistency of the other string printers.

This is what is should be:

string None // "None"
string (Some 1) // "Some(1)"
string (Some None) // "Some(None)"

[KevinRansom]

@charlesroddie:

you have a point that string arg is simply an alias to .ToString(). In my view it is not a very good alias since cultures are very real and quite important. The type unit, and values null and None represent no value, and they each fail to have a .ToString() method, so "Alias to ToString()" suddenly becomes extended with "and does something value dependent on values with no ToString()".

It would probably have been preferable to constrain string to types with a .ToString() and not nullable. The developer of the string api choose to represent these types as empty strings and that seems as good a choice as any, having the function choose to render the value differently for each given type seems a bit of a stretch as well as a breaking change.

> let unitValue = string ()
- let nullValue = string null
- let NoneValue = string None;;
val unitValue : string = ""
val nullValue : string = ""
val NoneValue : string = ""

I can easily imagine developers writing code along the lines of:

let s = string myValue
if s != "" then
     // Do something when s has info
else
    // Do something else when s is empty

Anyway .. thats my 2 cents and worth every penny we paid for them.

[abelbraaksma]

Yeah, I think in the end the real blocker will always be that it's a breaking change, even though I believe it would be a good thing to at least get the same output everywhere: whether that's an empty string, "null", or the value ("None" or whatever the name is of the UseNullAsTrueValue field). I mean, it's weird that:

[<CompilationRepresentation(CompilationRepresentationFlags.UseNullAsTrueValue)>] 
type X =
   | A         // string gives "" 
   | B of int  // string gives "B(42)"
let x = string A // "" 

Just saying that the absence of something doesn't always mean absence of everything.

Oh, and if you remove the attribute, which should really be an internal thing only, you'll suddenly get different output, string A becomes A (but the general problem is much harder to fix than only for the option type).

Be as it may, if we want to normalize the 4 or 5 different outputs you can now get depending on what method of stringification you choose, that'll always be a breaking change (remember when we included the type name for required access types?).

I mean, I don't like it, but I understand that we cannot take the change, even if we could reach consensus on what to output.

As another aside, I think we should fix None.ToString() and the like, but that's already an open request and doesn't have the backward compat burden.

[KevinRansom]

@abelbraaksma, developers should use ToString() not string.

If we want an api like string that has a consistent set of outputs, then design it as a new api.

  For example: tostring arg

Yes the good name has been taken, but breaking an existing developers application for this does not seem like the responsible way to go, when it is relatively cheap to provide a well-designed alternative. However, formatters and Culture should probably be designed in too.

Kevin

[abelbraaksma]

That's a good point, maybe we should add a new, and better function. I don't think developers think they should refrain from using string (it's odd to tell people to stay away from core functions), and personally I find x.ToString() cumbersome, as you'll have to add a null check, or introduce a lambda when piping/composing.

I think we ought to be able to deliver something that works predictably, maybe tostring is a way forward. And some way to deal with i18n would be cool too.

[charlesroddie]

The developer of the string api choose to represent these types as empty strings and that seems as good a choice as any

Having an ad-hoc decision for each case, as in the current behaviour below, is not as good as doing it consistently:

string None // "" - idea as dicsussed above, miminic the pseudocode None |> Seq.map string |> String.concat ""
string Some 1 // "Some 1" - idea is to mimic .ToString()
string (Some None) // "Some(null)" - broken

If string None returns "", then to be consistent with that string (Some 1) should return "1", and string(Some None) becomes very hard to specify, or accepting the first inconsistency with string (Some 1) returning "Some 1", string(Some None) should give Some() since the inner None is represented by "", but that gives inconsistency.

I can easily imagine developers writing code along the lines of:

FSharp minimizes matching on strings, and who would match on strings when there is an option to work with anyway? Fixing string functions isn't going to cause problems here and the problem is a bug as described above so fixing it is allowed. It's worth discussing with serialization libraries to make sure they don't use it for persistence, but who would use such a badly broken function for this purpose?

maybe we should add a new, and better function

We have way too many functions and none of them works. We need to get .ToString() to work because otherwise it will be very hard to get a new function to work.

There is a new thing being added and that is string interpolation. $"{Some None}" instead of string (Some None). So making sure this works for basic cases ($"{None}" giving "None") is important, and might allow making it work for all cases in future.

Then we'd need to make sure that string is deprecated, but that would be way more hassle for people than fixing it.

[abelbraaksma]

The interpolated version also returns the empty string.

Anyway, I'd like to make all this consistent somehow, some day, but I realize this isn't the way to do that, for backward compat reasons. @KevinRansom and @charlesroddie, thanks for your input here.