Runtime: String.Join(string, object[]) bug when first item is null

[GSPP]

Continuing my series to break the BCL for fun and no profit here's another string.Join bug:

String.Join(",", new object[] { null, "x" })

returns "". It should return ",x".

This bug exists in CoreCLR, in 4.6.2 and in the recently merged string.Join fix.

[bbowyersmyth]

It's a documented bug so that makes it a feature :wink:
https://msdn.microsoft.com/en-us/library/dd988350(v=vs.110).aspx

@jkotas What is the viewpoint on quirking old bugs on the new core platforms?

[jkotas]

It is case by case.

cc @AlexGhiondea

[GSPP]

@bbowyersmyth oh no! Haha that is so unfortunate.

[jamesqo]

IMO I think this should be fixed for .NET Core; this behavior is unintuitive, especially considering the fact that if you cast to IEnumerable<object> (or cast to object[] from string[], since arrays are covariant) you will get different behavior. This makes it very hard to write technically correct code.

[jamesqo]

@jkotas, you mentioned in the Tuple PR that commits that may cause compatibility issues with the desktop are simply not cherry-picked. Would it make sense to remove the bug now and fix the documentation? (It is preventing me from sharing code with the new char-based Join overloads-- see https://github.com/dotnet/coreclr/pull/7942#issuecomment-257735623

[jkotas]

I do not have an option on this one.
@stephentoub Do you happen to know about similar examples in corefx, to see where we are setting the bar on issues like this one?

[stephentoub]

@stephentoub Do you happen to know about similar examples in corefx, to see where we are setting the bar on issues like this one?

I thought I responded to this a few weeks ago, but apparently my comment didn't take. I don't really know of similar examples. We've certainly taken bug fixes that haven't yet been ported back to desktop. But we've also avoided making some kinds of fixes when the benefit is very low and the observable difference could cause cross-platform compatibility issues, e.g. throwing an ArgumentOutOfRangeException instead of an IndexOutOfRangeException.

I similarly don't have a strong opinion on this particular case. This behavior is documented, which is unfortunate, but it's also quite strange and inconsistents, e.g. that these two Console.WriteLine calls output different results:

C# var args = new[] { null, "x" }; Console.WriteLine(string.Join(",", (object[])args)); Console.WriteLine(string.Join(",", (string[])args));

so I'd be ok seeing it fixed as long as it was ported back to desktop.

[AlexRadch]

@stephentoub This behavior is documented in note as issue and have number of workarounds.

[danmosemsft]

This was fixed in dotnet/coreclr#8114.

[weshaggard]

@AlexGhiondea we don't have the netfx tags in the coreclr repo what is the best way to get this tracked for desktop?

[AlexGhiondea]

@weshaggard the easiest way is to create an issue in the CoreFx repo and tag that with Netfx-port-consider.

[weshaggard]

@AlexGhiondea we should probably do that for this issue if we haven't already.

[AlexGhiondea]

I have tagged https://github.com/dotnet/corefx/pull/14083 with netfx-port-consider.

[karelz]

Updated PR # in @AlexGhiondea's comment to the new one (dotnet/corefx#14083).

@AlexRadch please consider all impact of your PR switches in future.

[danmosemsft]

I see no use for this issue. fixed by https://github.com/dotnet/corefx/pull/13747

[danmosemsft]

Oh, it was clsoed already. My bad

[jayaranigarg]

@danmosemsft : Is this ported to netfx yet?