Record ToString shows no difference between null or an empty string

[Turnerj]

Originally raised as an issue against Roslyn as I thought it was an implementation issue, I was told instead that it was a part of the spec and to suggest the change as part of the C# language. I also found out that the same types of rules apply to anonymous types but I haven't investigated that as it wasn't the source of my problems.

Recently I was working on some unit tests using MSTest and Assert.AreEqual(a, b). At the same time, I was trying out using records for my types which seemed like a good fit for simple objects that had a handful of properties and not much else. I knew equality checks were auto-generated which is where I saw the usefulness of records and saw how I don't need to manually assert every property myself, just assert the records are the same via Assert.AreEqual(a, b).

While testing my code, I had a few cases that the assertion failed and what it does is it tells me that and calls ToString on the object. Unfortunately, the strings it gave back were identical but it still failed the assertion.

Assert.AreEqual failed.
Expected:<VersionDetails { MajorVersion = 1, MinorVersion = 2, PatchVersion = 4, PreRelease = , BuildMetadata = , CommitHash = abcdef, CommitHeight = 0, IsTaggedRelease = True }>.
Actual:<VersionDetails { MajorVersion = 1, MinorVersion = 2, PatchVersion = 4, PreRelease = , BuildMetadata = , CommitHash = abcdef, CommitHeight = 0, IsTaggedRelease = True }>.

I actually thought the issue was because I had reference types in records or maybe I was just using records incorrectly. Only by chance did I try running the test with the debugger enabled and carefully examined each property of the records did I realise that two properties (PreRelease and BuildMetadata - both strings) were null in one instance and empty strings in the other.

Is this a huge issue or something that is insurmountable? No, not really. Could I change how I do testing to avoid tripping up on this issue like testing properties individually? Sure. Do I feel that this is a bit of a trap in the spec? Absolutely, this ambiguity doesn't need to exist.

My thought is simple, just put quotes around string values. If it is empty, it would be empty quotes. If it is null, leave it as-is. This would make my error above look like:

Assert.AreEqual failed.
Expected:<VersionDetails { MajorVersion = 1, MinorVersion = 2, PatchVersion = 4, PreRelease = , BuildMetadata = , CommitHash = "abcdef", CommitHeight = 0, IsTaggedRelease = True }>.
Actual:<VersionDetails { MajorVersion = 1, MinorVersion = 2, PatchVersion = 4, PreRelease = "", BuildMetadata = "", CommitHash = "abcdef", CommitHeight = 0, IsTaggedRelease = True }>.

I don't actually know what the full purpose of the ToString overrides are for things like records and anonymous types. My assumption is that they are to aide debugging or logging, you wouldn't actually be parsing a ToString to regenerate the object. With that in mind, removing the ambiguity could only help debugging and logging scenarios.

[jnm2]

For what it's worth, a goal that was mentioned for the ToString method was to be consistent with how ToString has been implemented on anonymous types since C# 3.

Console.WriteLine(new { Test = (string)null }.ToString());
Console.WriteLine(new { Test = "" }.ToString());

Prints:

{ Test =  }
{ Test =  }

This is not to say that either one or both couldn't be improved. A tough question that arises as soon as you introduce quotes is, what do you do with quotes in the string? How far do you go, do you implement full language-specific escaping in spite of the fact that the .NET class being created by your record declaration can be used by other languages?

[Turnerj]

You've got a good point about escaping quotes. If not using quotes around the strings, what else could be done to show null vs an empty string? Show the length of a string next to the property name (and nothing if null)?

[jnm2]

Yeah, I don't have any good ideas either. A number appearing sounds confusing if the string or adjacent members have numbers in them.

[CyrusNajmabadi]

Note that 'escaping' or 'printing' strings in a special way isn't even well defined. What way would that be? C# syntax? VB syntax? Something else?

.net printing of strings for these purposes is simple: you take each character and you print it out. If there are no characters (null or empty) you get nothing.

[Turnerj]

I can't see that making any sense.

It made complete sense given that I said that my understanding from your other comments was that this was a C# spec issue. Why would a C# spec issue not be following C# syntax unless...

The C# spec says that we simply defer to .net's ToString.

Ahhhhh. I did not get that specifically from your other comments - I don't know the C# spec inside and out. You mentioned it was the .NET behaviour for strings though I often erroneously conflate .NET as C# so it is far easier to see this being a C#-specific issue rather than something that is intrinsic to .NET as a runtime itself.

It's not even clear to me an effective way to make this happen. Where does this special string printing code exist at runtime? How do records know how to find it?

I've got no idea how it is even implemented currently - I thought it was generated by the compiler for records etc (not something handled by the runtime specifically), that's why this didn't seem like it was going to even be a big issue. I didn't factor in that with multiple libraries being used compiled at different points in time with different language versions, it would need to facilitate something in the runtime itself so the change could be rolled forward.

Finally, this worries me from a compat perspective. Today, if i have the literal string "null" and i use it in a record, i'll get ... Whatever = null .... Now, after the change, the same text will mean that we're pointing at a null string/object. I'm pretty strongly opposed to us changing how these end up being displayed, and having it effectively mean that you have to know against what version of C# you compiled the original code to know what this means.

You're right, it would change behaviour on a compatibility perspective. If we treat the lack of a value as null, this specific scenario would be somewhat mitigated but yeah, it isn't without confusion.

And yep, for a while at least, it would be a quirk between C# versions for those that call ToString on anonymous types or records. Right now though, not doing a change means all of the same people affected by that would have an ambiguity between a null value and an empty string.

I would then think what is more likely to happen more? A null value, an empty string or a string with the word "null"? Anecdotally, I've hit APIs that return null or an unexpectedly empty string (that's how we got here) but haven't hit a .NET API that returns a string with the word null.

Also in the same way with your "null" string example, those people could be equally confused by this: ... Whatever = , SomethingElse = ,... where , SomethingElse = is a string. It is a contrived (and unlikely) example but the problem goes both ways by not enclosing strings with something.

Overall, it sounds like the core problem is more that it is a backwards incompatible change that, depending how it is implemented, may affect every language that runs on .NET (or at least create inconsistencies between them)?

[CyrusNajmabadi]

I've got no idea how it is even implemented currently

It's pretty simple :) We iterate over the properties, calling .ToString() on each one. We don't dictate how objects choose to print themselves. The objects themselves get to decide. I think it would be very weird for us to say: actually, for a string, we will print it out with C# syntax, but everything else will use whatever the object specified.

it would need to facilitate something in the runtime itself so the change could be rolled forward.

Yeah, i think you'd need to get the runtime to change here. And i just don't see that happening. We're describing how strings have been printed for literally over 23 years (it's been this way since the earliest alpha's of .net). The amount of dependencies on this will be staggering.

And yep, for a while at least, it would be a quirk between C# versions for those that call ToString on anonymous types or records.

I really think changing the behavior for all customers here really doesn't meet any sort of compat acceptability bar.

I would then think what is more likely to happen more?

No, i don't think that's what's relevant. I think we have to ask: is it ok to break all the existing code out there that has come to expect this, just because some people don't like how .net strings print. I do not think C# should be in the business of overriding the choices of these types.

but the problem goes both ways by not enclosing strings with something.

Right. Except in one case we have:

1. the existing behavior that has worked this way since the beginning of .net/c#, and which people expect. vs:
2. new behavior that works entirely differently, and only for this specific area.

Again, this only addresses records. Literally every other form of printing/logging does not benefit.

Overall, it sounds like the core problem is more that it is a backwards incompatible change that

Not for me. Even if there was not back compat concern, there woudl still be the concern aobut this being a c#-ism in .net. Which doesn't make sense to me.

[jnm2]

I would then think what is more likely to happen more? A null value, an empty string or a string with the word "null"? Anecdotally, I've hit APIs that return null or an unexpectedly empty string (that's how we got here) but haven't hit a .NET API that returns a string with the word null.

This is what I think matters. I agree with this. But I'm also not happy that the C# language keyword would leak out. Sure, you're using C# to define this .NET class, but it's not normally seen as a good thing for the compiled result to remind you of this fact.

[Turnerj]

While I get the gist of what you're saying @CyrusNajmabadi , I think we're still kinda talking about two different things. Without trying to repeat too much, it seems like we both at least agree that it shouldn't be C# specific nor should have C#-isms (like if we escaped the string C# syntax-wise). From here my comments are generally speaking...

We iterate over the properties, calling .ToString() on each one. We don't dictate how objects choose to print themselves. The objects themselves get to decide. I think it would be very weird for us to say: actually, for a string, we will print it out with C# syntax, but everything else will use whatever the object specified.

I don't think it is that weird for special handling of string - it is a special type of object and fundamental to the runtime.

That said thinking about this did have me realise a new problem that none of this would solve. If some custom object has overridden ToString, we're going to still be in a mess anyway because we have no idea what the output would look like. It could be a short string, a bunch of numbers, comma separate value of its properties etc. Regardless if we change how the output is for string types, any custom type could make the output just as confusing - so maybe the whole thing really is a moot point.

Yeah, i think you'd need to get the runtime to change here. And i just don't see that happening. We're describing how strings have been printed for literally over 23 years (it's been this way since the earliest alpha's of .net). The amount of dependencies on this will be staggering.

What I meant by a change in the runtime was more along the lines of calling the string.ToString(IFormatProvider) method (as an example) with some internal quote-escaping format provider that then exists in the runtime. It doesn't solve any of the problems about what way it is escaped etc but pushes that type of thing for the runtime to handle rather than the language/compiler to handle.

I'm definitely NOT suggesting changing how all strings are handled everywhere across the entire concept of .NET - I don't see how we would be jumping to that type of huge change with what I'm thinking.

No, i don't think that's what's relevant. I think we have to ask: is it ok to break all the existing code out there that has come to expect this, just because some people don't like how .net strings print.

When we're talking about breaking existing code here, are we talking about code that is specifically parsing/processing the value of an anonymous type or record's ToString value? I really really hope no one is actually parsing the result of random anonymous types and record ToString methods and hoping to extract data out of it.

From the point of view that it is breaking behaviour that would affect people, yeah, I 100% agree that anyone that is particularly accustom to the current format without quotes would find it weird with quotes.

[Turnerj]

This is what I think matters. I agree with this. But I'm also not happy that the C# language keyword would leak out. Sure, you're using C# to define this .NET class, but it's not normally seen as a good thing for the compiled result to remind you of this fact.

You've got a good point about exposing a C# language keyword. I'd be entirely happy if null was just blank in the output like already is being displayed.

I do agree though about both your thoughts and @CyrusNajmabadi about wanting to avoid C#-isms etc in any type of change that affects the whole .NET runtime though. Assuming I managed to make Cyrus come around to the idea in general (😛), I don't think this should move forward unless a language agnostic way was found.

[qrli]

To me, you real issue has nothing to do with ToString(), but MSTest not doing a great job.

What you use is the most basic form of quality assertion, which relies on the equality defined by the type. And in this case, it is record, in a naïve way. But surely the assert output can be much better:

Instead of ToString() which lists all properties unnecessarily, it can print only the property which is different, and show the difference between null and "". That's structural equality, which I have used a lot with FluentAssertions's obj.Should().BeEquivalentTo(expected).

PS: I'd recommend you to try FluentAssertions. It is quite possible that you won't use MSTest's Assert anymore.

[Turnerj]

You do have an interesting point about assert output being better and I agree, it definitely would be better if it showed only the differences (would be a lot easier to read!). Might give FluentAssertions a look at some point soon 🙂

[CyrusNajmabadi]

I'm definitely NOT suggesting changing how all strings are handled everywhere across the entire concept of .NET

Oh. Then I definitely don't want to change anything here. I would not want records to behave differently than everything else out there in .net. This is how strings are printed. Anonymous types print things out this way. Console prints it out this way. Etc. Etc. For better or worse, the position since day one seems to be: a null printed string produces no characters. And a normal printed string just prints the contents, ubescaped, without quotes.

It's far more important to me that records just follow that then suddenly having them behave differently

[Turnerj]

I guess I see string property handling inside records/anonymous types ToString different to how I see it in something like Console writing - to me they seem like two completely different use cases even if both are fundamentally handling the printing of strings. The biggest difference for me is that I control what I pass to Console (eg. Console.WriteLine(myString ?? "null")) but I only have control over how a record prints if I override ToString (and then manually implement the output for all properties again), and for anonymous types I have no control at all.

[CyrusNajmabadi]

I really really hope no one is actually parsing the result of random anonymous types

I've seen much crazier than that :-)