Runtime: Path.GetRelativePath throws ArgumentNullException when relativeTo is empty string

Created on 4 Jun 2019  路  7Comments  路  Source: dotnet/runtime

The following program throws an ArgumentNullException:

using System.IO;
public class Program
{
    public static void Main()
    {
         Console.WriteLine(Path.GetRelativePath("", "file.pdf"));
    }
}

ArgumentNullException: Value cannot be null. Parameter name: relativeTo

This is a misleading exception. It should be an ArgumentException since relativePath is not null.

area-System.IO easy enhancement up-for-grabs

Most helpful comment

Tests go where the type is publicly exposed. In this case it is System.Runtime.Extensions. https://apisof.net/catalog/System.IO.Path

https://github.com/dotnet/corefx/tree/master/src/System.Runtime.Extensions/tests/System/IO

System.IO.Path is actually compiled into System.Private.CoreLib. So the changes have to come through CoreCLR.

Instructions for consuming a private CoreCLR build are here: https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#testing-with-private-coreclr-bits

The general pattern is to create a PR in both repos, linking the two.

All 7 comments

cc: @JeremyKuhne

I see what you mean.

The fix would be quite simple. The very first check we do when calling GetRelativePath is this:

https://github.com/dotnet/corefx/blob/b2097cbdcb26f7f317252334ddcce101a20b7f3d/src/Common/src/CoreLib/System/IO/Path.cs#L813-L815

So the checks could be split into a string.IsNull check and a string.IsEmpty check, and throw the appropriate exception for each case.

Not breaking behavior so I'll mark it as future.

@JeremyKuhne would you agree?

would you agree?

Yeah, it should be split up.

If someone wanted to write a unit test to assert the exception throwing, where would that "PathTests.cs" file be placed? It seems tests can be found in multiple places so it's not immediately obvious where to put it.

The Path.cs file seems to be located here:
\corefx\src\Common\src\CoreLib\System\IO\Path.cs

Tests for other classes that are located in the same path as the Path class are placed here under folders named after the class they're testing:
\corefx\src\System.IO\tests\

Going by that, would it be correct to place the "PathTests.cs" file here?:
\corefx\src\System.IO\tests\Path\PathTests.cs

Tests go where the type is publicly exposed. In this case it is System.Runtime.Extensions. https://apisof.net/catalog/System.IO.Path

https://github.com/dotnet/corefx/tree/master/src/System.Runtime.Extensions/tests/System/IO

System.IO.Path is actually compiled into System.Private.CoreLib. So the changes have to come through CoreCLR.

Instructions for consuming a private CoreCLR build are here: https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#testing-with-private-coreclr-bits

The general pattern is to create a PR in both repos, linking the two.

Can be closed

Thanks @Marusyk

Was this page helpful?
0 / 5 - 0 ratings