Skip to content

Conversation

ssimek
Copy link
Contributor

@ssimek ssimek commented May 13, 2020

Summary

The string.Join documentation incorrectly claims that string.Empty is returned if all input values are null.

@dotnet-bot dotnet-bot added this to the May 2020 milestone May 13, 2020
@gewarren gewarren requested a review from carlossanlop May 22, 2020 00:15
@gewarren
Copy link
Contributor

@carlossanlop Who can review this one?

@carlossanlop
Copy link
Contributor

@ssimek thank you for your contribution. Indeed, the result of concatenating an enumeration of n nulls returns a string containing n-1 separators.

There is only one .NET Framework Join method overload that returns string.Empty if it receives an enumeration of null objects. I suspect the return value was copy-pasted for all the overloads without verification.

So @ssimek, can you please edit the return value for all the overloads with the same change?

Except for the overload with the DocID M:System.String.Join(System.String,System.Object[]). This overload needs a return value saying:

<see cref="F:System.String.Empty" /> if <paramref name="values" /> has zero elements.

-or-

.NET Framework only: <see cref="F:System.String.Empty" /> if all the elements of <paramref name="values" /> are <see langword="null" />.</returns>

I tested this for validation:

.NET 5.0
using System;
using System.Collections.Generic;

namespace MyNamespace
{
  class MyCoreClass
  {
      static void Main()
      {
          char separator1 = ';';

          object[] values1 = new object[] { null, null, null };
          string s1 = string.Join(separator1, values1);
          Console.WriteLine($"Length: {s1.Length} = String: [{s1}]");

          string[] values2 = new string[] { null, null, null };
          string s2 = string.Join(separator1, values2);
          Console.WriteLine($"Length: {s2.Length} = String: [{s2}]");

          IEnumerable<string> values3 = new List<string> { null, null, null };
          string s3 = string.Join(separator1, values3);
          Console.WriteLine($"Length: {s3.Length} = String: [{s3}]");

          string separator2 = ":";

          object[] values4 = new object[] { null, null, null };
          string s4 = string.Join(separator2, values4);
          Console.WriteLine($"Length: {s4.Length} = String: [{s4}]");

          string[] values5 = new string[] { null, null, null };
          string s5 = string.Join(separator2, values5);
          Console.WriteLine($"Length: {s5.Length} = String: [{s5}]");

          IEnumerable<string> values6 = new List<string> { null, null, null };
          string s6 = string.Join(separator2, values6);
          Console.WriteLine($"Length: {s6.Length} = String: [{s6}]");

          string s7 = string.Join(separator1, values5, 0, 3);
          Console.WriteLine($"Length: {s7.Length} = String: [{s7}]");

          /*
          Output:

              Length: 2 = String: [;;]
              Length: 2 = String: [;;]
              Length: 2 = String: [;;]
              Length: 2 = String: [::]
              Length: 2 = String: [::]
              Length: 2 = String: [::]
              Length: 2 = String: [;;]
          */
      }
  }
}
.NET Framework Note: The overloads that take a `char` separator do not exist in .NET Framework.
using System;
using System.Collections.Generic;

namespace MyNamespace
{
  class MyFrameworkClass
  {
      static void Main()
      {
          string separator = ";";

          object[] values1 = new object[] { null, null, null };
          string s1 = string.Join(separator, values1);
          Console.WriteLine($"Length: {s1.Length} = String: [{s1}]");

          string[] values2 = new string[] { null, null, null };
          string s2 = string.Join(separator, values2);
          Console.WriteLine($"Length: {s2.Length} = String: [{s2}]");

          IEnumerable<string> values3 = new List<string> { null, null, null };
          string s3 = string.Join(separator, values3);
          Console.WriteLine($"Length: {s3.Length} = String: [{s3}]");

          string s4 = string.Join(separator, values2, 0, 3);
          Console.WriteLine($"Length: {s4.Length} = String: [{s4}]");

          Console.ReadLine();

          /*
          Output:

              Length: 0 = String: []
              Length: 2 = String: [;;]
              Length: 2 = String: [;;]
              Length: 2 = String: [;;]
          */
      }
  }
}

@ssimek
Copy link
Contributor Author

ssimek commented Sep 13, 2020

Hi @carlossanlop,

delving into this a bit deeper, it seems that the situation is even more complicated 😁. The overload you mention always returns an empty string if the first element is null under the original .NET Framework, but works normally otherwise (i.e. there is no "all elements are null" check, just "first element is null"). This is actually already documented on the page in the "Remarks" and "Notes to Callers" sections.

In this version I've tried to clean up the wording of all overloads as best as I could, mentioning the "bug" as well as changing "members" to "elements", clarifying ranges for the overloads that take them, etc.

Thanks!

@opbld32
Copy link

opbld32 commented Sep 13, 2020

Docs Build status updates of commit 2d8042b:

✅ Validation status: passed

File Status Preview URL Details
xml/System/String.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@jeffhandley jeffhandley removed this from the May 2020 milestone Feb 14, 2021
Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssimek I'm sorry this sat for so long. I've verified the behavior you described on .NET Framework; good catch!

            Console.WriteLine(String.Join(";", (object)null, (object) null, (object)null)); // String.Empty
            Console.WriteLine(String.Join(";", (string)null, (string) null, (string)null)); // ";;"
            Console.WriteLine(String.Join(";", (object)null, "b", "c"));                    // String.Empty
            Console.WriteLine(String.Join(";", (string)null, "b", "c"));                    // ";b;c"
            Console.WriteLine(String.Join(";", "a", (object)null, "c"));                    // "a;;c"
            Console.WriteLine(String.Join(";", "a", (string)null, "c"));                    // "a;;c"
            Console.WriteLine(String.Join(";", new List<string>() { "a", null, "c" }));     // "a;;c"
            Console.WriteLine(String.Join(";", new List<string>() { null, "b", "c" }));     // ";b;c"

@jeffhandley jeffhandley merged commit 1bb809a into dotnet:master Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants