Fix MockDirectoryInfo.GetFiles for UNC paths

[DeveloperGuo]

Regression introduced in v3.0.2 by #418 StringExtensions.NormalizeSlashes.
Unc paths are getting stripped to a single separator.

[fgreinacher]

Thanks, this looks good to me.

@rkoeninger, additional pair of eyes?

[rkoeninger]

@fgreinacher Took me a second to see the else if clause. When there were exactly 2 slashes, it would get to the else if (> 1) and trim to a single slash.

The change to the first condition should work, but (@DeveloperGuo) the else if should check for == 1 not > 1 with this change.

[rkoeninger]

@DeveloperGuo Instead of

            // UNC Paths start with double slash but no reason
            // to have more than 2 slashes at the start of a path
            if (XFS.IsWindowsPlatform() && prefixSeps.Length >= 2)
            {
                prefixSeps = prefixSeps.Substring(0, 2);
            }
            else if (prefixSeps.Length > 1)
            {
                prefixSeps = prefixSeps.Substring(0, 1);
            }

can we make it something like

            // UNC Paths start with double slash but no reason
            // to have more than 2 slashes at the start of a path
            if (XFS.IsWindowsPlatform() && prefixSeps.Length >= 2)
            {
                prefixSeps = doubleSep;
            }
            else if (prefixSeps.Length == 1)
            {
                prefixSeps = sep;
            }
            else
            {
                prefixSeps = "";
            }

I was going to make it a ternary, but a doubly-nested ternary might be a little much.

...I don't know, how about as a ternary, your call:

            // UNC Paths start with double slash but no reason
            // to have more than 2 slashes at the start of a path
            prefixSeps =
                XFS.IsWindowsPlatform() && prefixSeps.Length >= 2 ? doubleSep :
                prefixSeps.Length == 1 ? sep :
                "";

[rkoeninger]

@fgreinacher Does that while (true) look sketchy to you? It could be for (var i = 0; i < 100; ++i) to prevent infinite loop. That could be implemented more cleanly, too.

Single-pass slash de-duplicate logic:

var builder = new StringBuilder(path.Length);
var prevCh = '\0';

foreach (var ch in path)
{
    if (ch != sep || prevCh != sep)
    {
        builder.Append(ch);
    }

    prevCh = ch;
}

return prefixSeps + builder.ToString();

[DeveloperGuo]

@DeveloperGuo Instead of

            // UNC Paths start with double slash but no reason
            // to have more than 2 slashes at the start of a path
            if (XFS.IsWindowsPlatform() && prefixSeps.Length >= 2)
            {
                prefixSeps = prefixSeps.Substring(0, 2);
            }
            else if (prefixSeps.Length > 1)
            {
                prefixSeps = prefixSeps.Substring(0, 1);
            }

can we make it something like

            // UNC Paths start with double slash but no reason
            // to have more than 2 slashes at the start of a path
            if (XFS.IsWindowsPlatform() && prefixSeps.Length >= 2)
            {
                prefixSeps = doubleSep;
            }
            else if (prefixSeps.Length == 1)
            {
                prefixSeps = sep;
            }
            else
            {
                prefixSeps = "";
            }

@rkoeninger - The prefixSep.Length == 0 and 1 cases are already handled by the prefixSeps initialization.

var prefixSeps = new string(path.TakeWhile(c => c == Path.DirectorySeparatorChar).ToArray()); path = path.Substring(prefixSeps.Length);
Based on the comment above the if/else, the intention here was to trim paths starting with more than 1 separator for non-UNC paths. Your suggestion would break this in the case of !XFS.IsWindowsPlatform() && prefixSeps.Length == 2

` // UNC Paths start with double slash but no to have more than 2 slashes at the start of a path

It may make sense to create separate pull request(s) for the other issue(s) brought up as this one is specifically addressing the regression to UNC paths.

[rkoeninger]

The prefixSep.Length == 0 and 1 cases are already handled by the prefixSeps initialization

@DeveloperGuo right, that else if is never going to be true now.

Your fix should work, so it's fine as is, I just started thinking out loud.

[fgreinacher]

Is this good to go?

[rkoeninger]

👍 Go for it

[fgreinacher]

Thanks @DeveloperGuo!