IPath.WithExtension: behavior and documentation improvements

CHANGELOG.md

@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - **Breaking:** `LocalPath.StartsWith` and `AbsolutePath.StartsWith` now compare whole path segments instead of raw strings, which makes them exact inverses of `IsPrefixOf` as originally intended in [#43](https://github.com/ForNeVeR/TruePath/issues/43). For example, `new LocalPath("/foo1").StartsWith(new LocalPath("/foo"))` is now `false`, where it used to be `true`.
 - `LocalPath.IsPrefixOf` now returns `false` whenever the two paths differ in absoluteness: an absolute path is never a prefix of a relative one, nor the other way round. Previously, the result depended on an incidental string comparison.
 - `LocalPath.IsPrefixOf` now treats an empty path — the normalized form of `""`, `"."` and `"a/.."`, and the parent of any single-segment relative path — as the current directory, so it is a prefix of every relative path that does not begin with a `..` reference.
+- **Breaking:** `PathExtensions.WithExtension` now throws an `ArgumentException` unless the path ends with a file name and the result of the change is a file name in its place: an extension change may no longer alter the number or the kind of a path's segments. It used to do so silently — `new LocalPath(@"C:\foo\.gitignore").WithExtension(null)` returned `C:\foo`, `new LocalPath(".gitignore").WithExtension(null)` returned the empty path, and `new LocalPath(@"C:\").WithExtension("bar")` returned `C:\.bar`.
 
 ### Fixed
 - `LocalPath.IsPrefixOf` and `StartsWith` now compare path strings ordinally. Previously they used the current culture, which ignores collation-ignorable characters, so a path could be reported as a prefix of an unrelated one.

TruePath.Tests/PathExtensionsTests.cs

@@ -75,9 +75,7 @@ public void FileNameInvariantTests(string inputPath)
     }
 
     [Theory]
-    [InlineData(@"C:\", "bar", @"C:\.bar")]
     [InlineData(@"C:\filename.foo", "bar", @"C:\filename.bar")]
-    [InlineData(@"\", "bar", @"\.bar")]
     [InlineData(@"\file", "bar", @"\file.bar")]
     [InlineData(@"\file", ".bar", @"\file.bar")]
     [InlineData(@"\file.", ".bar", @"\file.bar")]
@@ -123,7 +121,6 @@ public void WithExtensionArgumentTests(string inputPath, string? newExtension, s
     }
 
     [Theory]
-    [InlineData("/", "bar", "/.bar")]
     [InlineData("/file", "bar", "/file.bar")]
     [InlineData("/file", ".bar", "/file.bar")]
     [InlineData("/file.", ".bar", "/file.bar")]
@@ -145,4 +142,65 @@ public void WithExtensionTests_Unix(string inputPath, string newExtension, strin
         // Assert
         Assert.Equal(expected, newPath.Value);
     }
+
+    [Theory]
+    [InlineData(".gitignore", null)]
+    [InlineData(".gitignore", "")]
+    [InlineData(".gitignore", ".")]
+    [InlineData("foo/.gitignore", null)]
+    [InlineData("..", null)]
+    [InlineData("..", "bar")]
+    [InlineData("../..", "txt")]
+    [InlineData("", "txt")]
+    [InlineData("a/..", "txt")]
+    [InlineData("file.txt", "foo/bar")]
+    public void WithExtensionThrowsIfTheResultIsNotAFileName(string inputPath, string? newExtension)
+    {
+        // Arrange
+        var path = new LocalPath(inputPath);
+
+        // Act & Assert
+        Assert.Throws<ArgumentException>(() => path.WithExtension(newExtension));
+    }
+
+    [Theory]
+    [InlineData(@"C:\foo\.gitignore", null)]
+    [InlineData(@"C:\.gitignore", null)]
+    [InlineData(@"C:\", "bar")]
+    [InlineData(@"\", "bar")]
+    public void WithExtensionThrowsIfTheResultIsNotAFileName_Windows(string inputPath, string? newExtension)
+    {
+        if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+        {
+            return;
+        }
+
+        // Arrange
+        var local = new LocalPath(inputPath);
+        var absolute = new AbsolutePath(inputPath);
+
+        // Act & Assert
+        Assert.Throws<ArgumentException>(() => local.WithExtension(newExtension));
+        Assert.Throws<ArgumentException>(() => absolute.WithExtension(newExtension));
+    }
+
+    [Theory]
+    [InlineData("/foo/.gitignore", null)]
+    [InlineData("/.gitignore", null)]
+    [InlineData("/", "bar")]
+    public void WithExtensionThrowsIfTheResultIsNotAFileName_Unix(string inputPath, string? newExtension)
+    {
+        if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux) && !RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+        {
+            return;
+        }
+
+        // Arrange
+        var local = new LocalPath(inputPath);
+        var absolute = new AbsolutePath(inputPath);
+
+        // Act & Assert
+        Assert.Throws<ArgumentException>(() => local.WithExtension(newExtension));
+        Assert.Throws<ArgumentException>(() => absolute.WithExtension(newExtension));
+    }
 }

TruePath/PathExtensions.cs

@@ -52,8 +52,8 @@ public static string GetFilenameWithoutExtension(this IPath path) =>
 
 #if NET8_0_OR_GREATER
     /// <summary>
-    /// Returns a new path of the same type <typeparamref name="TPath"/> with the extension of its file name component changed,
-    /// or with a new extension-like component if the original file name was empty.
+    /// Returns a new path of the same type <typeparamref name="TPath"/> with the extension of its file name
+    /// component changed.
     /// </summary>
     /// <typeparam name="TPath">The type of the path, which must implement <see cref="IPath{TPath}"/>.</typeparam>
     /// <param name="path">The original path.</param>
@@ -66,11 +66,28 @@ public static string GetFilenameWithoutExtension(this IPath path) =>
     ///         Pass <see langword="null"/> to remove the extension entirely (<c>file.txt</c> becomes <c>file</c>), or
     ///         an empty string to remove it but keep the trailing dot (<c>file.txt</c> becomes <c>file.</c>).
     ///     </para>
+    ///     <para>
+    ///         The extension may not contain a directory separator, and neither <see langword="null"/>, an empty
+    ///         string nor a lone dot may be passed for a file name that consists entirely of an extension: see the
+    ///         exception below.
+    ///     </para>
     /// </param>
     /// <returns>
     /// A new path of type <typeparamref name="TPath"/> with the modified file name component.
     /// The original <paramref name="path"/> object is not modified.
     /// </returns>
+    /// <exception cref="ArgumentException">
+    ///     <para>
+    ///         Thrown if the <paramref name="path"/> has no file name to change: it is empty (which designates the
+    ///         current directory), it consists of a root or a drive designator alone, such as <c>C:\</c>, <c>/</c>
+    ///         or <c>C:</c>, or its last segment is a parent directory reference (<c>..</c>).
+    ///     </para>
+    ///     <para>
+    ///         Also thrown if the requested change would leave something other than a file name in the file name's
+    ///         place: <c>C:\foo\.gitignore</c> with <see langword="null"/> would leave nothing at all, and
+    ///         <c>file.txt</c> with the extension <c>foo/bar</c> would leave a whole new segment.
+    ///     </para>
+    /// </exception>
     /// <remarks>
     ///     <para>
     ///         Only the last extension is replaced: for <c>archive.tar.gz</c> and the extension <c>zip</c>, the result
@@ -81,15 +98,23 @@ public static string GetFilenameWithoutExtension(this IPath path) =>
     ///     <para>
     ///         A file name consisting entirely of an extension is treated as an extension, consistently with
     ///         <see cref="GetExtensionWithDot"/>: <c>.gitignore</c> with the extension <c>hgignore</c> becomes
-    ///         <c>.hgignore</c>.
+    ///         <c>.hgignore</c>. <b>Removing</b> the extension of such a name is impossible, though: nothing would
+    ///         be left of the file name.
+    ///     </para>
+    ///     <para>
+    ///         This method changes the extension and nothing else: the path always ends with a file name both
+    ///         before and after the call, and the number and the kind of its segments are always the same. Whenever
+    ///         the requested change would break that, it throws instead of returning a path of a different shape.
     ///     </para>
     /// </remarks>
-    public static TPath WithExtension<TPath>(this TPath path, string? extension) where TPath : IPath<TPath> =>
-        TPath.Create(Path.ChangeExtension(((IPath)path).Value, extension));
+    public static TPath WithExtension<TPath>(this TPath path, string? extension) where TPath : IPath<TPath>
+    {
+        var p = (IPath)path;
+        return TPath.Create(ChangeExtension(p.Value, p.FileName, extension));
+    }
 #else
     /// <summary>
-    /// Returns a new path of type <see cref="AbsolutePath"/> with the extension of its file name component changed,
-    /// or with a new extension-like component if the original file name was empty.
+    /// Returns a new path of type <see cref="AbsolutePath"/> with the extension of its file name component changed.
     /// </summary>
     /// <param name="path">The original path.</param>
     /// <param name="extension">
@@ -101,11 +126,28 @@ public static TPath WithExtension<TPath>(this TPath path, string? extension) whe
     ///         Pass <see langword="null"/> to remove the extension entirely (<c>file.txt</c> becomes <c>file</c>), or
     ///         an empty string to remove it but keep the trailing dot (<c>file.txt</c> becomes <c>file.</c>).
     ///     </para>
+    ///     <para>
+    ///         The extension may not contain a directory separator, and neither <see langword="null"/>, an empty
+    ///         string nor a lone dot may be passed for a file name that consists entirely of an extension: see the
+    ///         exception below.
+    ///     </para>
     /// </param>
     /// <returns>
     /// A new path of type <see cref="AbsolutePath"/> with the modified file name component.
     /// The original <paramref name="path"/> object is not modified.
     /// </returns>
+    /// <exception cref="ArgumentException">
+    ///     <para>
+    ///         Thrown if the <paramref name="path"/> has no file name to change: it is empty (which designates the
+    ///         current directory), it consists of a root or a drive designator alone, such as <c>C:\</c>, <c>/</c>
+    ///         or <c>C:</c>, or its last segment is a parent directory reference (<c>..</c>).
+    ///     </para>
+    ///     <para>
+    ///         Also thrown if the requested change would leave something other than a file name in the file name's
+    ///         place: <c>C:\foo\.gitignore</c> with <see langword="null"/> would leave nothing at all, and
+    ///         <c>file.txt</c> with the extension <c>foo/bar</c> would leave a whole new segment.
+    ///     </para>
+    /// </exception>
     /// <remarks>
     ///     <para>
     ///         Only the last extension is replaced: for <c>archive.tar.gz</c> and the extension <c>zip</c>, the result
@@ -116,15 +158,20 @@ public static TPath WithExtension<TPath>(this TPath path, string? extension) whe
     ///     <para>
     ///         A file name consisting entirely of an extension is treated as an extension, consistently with
     ///         <see cref="GetExtensionWithDot"/>: <c>.gitignore</c> with the extension <c>hgignore</c> becomes
-    ///         <c>.hgignore</c>.
+    ///         <c>.hgignore</c>. <b>Removing</b> the extension of such a name is impossible, though: nothing would
+    ///         be left of the file name.
+    ///     </para>
+    ///     <para>
+    ///         This method changes the extension and nothing else: the path always ends with a file name both
+    ///         before and after the call, and the number and the kind of its segments are always the same. Whenever
+    ///         the requested change would break that, it throws instead of returning a path of a different shape.
     ///     </para>
     /// </remarks>
     public static AbsolutePath WithExtension(this AbsolutePath path, string? extension) =>
-        AbsolutePath.Create(Path.ChangeExtension(path.Value, extension));
+        AbsolutePath.Create(ChangeExtension(path.Value, path.FileName, extension));
 
     /// <summary>
-    /// Returns a new path of type <see cref="LocalPath"/> with the extension of its file name component changed,
-    /// or with a new extension-like component if the original file name was empty.
+    /// Returns a new path of type <see cref="LocalPath"/> with the extension of its file name component changed.
     /// </summary>
     /// <param name="path">The original path.</param>
     /// <param name="extension">
@@ -136,11 +183,28 @@ public static AbsolutePath WithExtension(this AbsolutePath path, string? extensi
     ///         Pass <see langword="null"/> to remove the extension entirely (<c>file.txt</c> becomes <c>file</c>), or
     ///         an empty string to remove it but keep the trailing dot (<c>file.txt</c> becomes <c>file.</c>).
     ///     </para>
+    ///     <para>
+    ///         The extension may not contain a directory separator, and neither <see langword="null"/>, an empty
+    ///         string nor a lone dot may be passed for a file name that consists entirely of an extension: see the
+    ///         exception below.
+    ///     </para>
     /// </param>
     /// <returns>
     /// A new path of type <see cref="LocalPath"/> with the modified file name component.
     /// The original <paramref name="path"/> object is not modified.
     /// </returns>
+    /// <exception cref="ArgumentException">
+    ///     <para>
+    ///         Thrown if the <paramref name="path"/> has no file name to change: it is empty (which designates the
+    ///         current directory), it consists of a root or a drive designator alone, such as <c>C:\</c>, <c>/</c>
+    ///         or <c>C:</c>, or its last segment is a parent directory reference (<c>..</c>).
+    ///     </para>
+    ///     <para>
+    ///         Also thrown if the requested change would leave something other than a file name in the file name's
+    ///         place: <c>C:\foo\.gitignore</c> with <see langword="null"/> would leave nothing at all, and
+    ///         <c>file.txt</c> with the extension <c>foo/bar</c> would leave a whole new segment.
+    ///     </para>
+    /// </exception>
     /// <remarks>
     ///     <para>
     ///         Only the last extension is replaced: for <c>archive.tar.gz</c> and the extension <c>zip</c>, the result
@@ -151,10 +215,45 @@ public static AbsolutePath WithExtension(this AbsolutePath path, string? extensi
     ///     <para>
     ///         A file name consisting entirely of an extension is treated as an extension, consistently with
     ///         <see cref="GetExtensionWithDot"/>: <c>.gitignore</c> with the extension <c>hgignore</c> becomes
-    ///         <c>.hgignore</c>.
+    ///         <c>.hgignore</c>. <b>Removing</b> the extension of such a name is impossible, though: nothing would
+    ///         be left of the file name.
+    ///     </para>
+    ///     <para>
+    ///         This method changes the extension and nothing else: the path always ends with a file name both
+    ///         before and after the call, and the number and the kind of its segments are always the same. Whenever
+    ///         the requested change would break that, it throws instead of returning a path of a different shape.
     ///     </para>
     /// </remarks>
     public static LocalPath WithExtension(this LocalPath path, string? extension) =>
-        LocalPath.Create(Path.ChangeExtension(path.Value, extension));
+        LocalPath.Create(ChangeExtension(path.Value, path.FileName, extension));
 #endif
+
+    private static bool IsFileName(string segment) =>
+        segment.Length > 0
+        && segment is not ("." or "..")
+        && segment.IndexOf(Path.DirectorySeparatorChar) < 0
+        && segment.IndexOf(Path.AltDirectorySeparatorChar) < 0;
+
+    private static string ChangeExtension(string value, string fileName, string? extension)
+    {
+        if (!IsFileName(fileName))
+            throw new ArgumentException(
+                $"Path \"{value}\" does not end with a file name, so its extension cannot be changed.",
+                "path");
+
+        // A file name consisting entirely of an extension, such as ".gitignore", has nothing left once the extension
+        // is removed: the new file name would either be empty or a lone dot that the normalization drops, and so the
+        // path would lose a segment.
+        var newFileName = Path.ChangeExtension(fileName, extension);
+        if (!IsFileName(newFileName))
+        {
+            var extensionText = extension is null ? "null" : $"\"{extension}\"";
+            throw new ArgumentException(
+                $"Changing the extension of path \"{value}\" to {extensionText} would replace its file name " +
+                $"\"{fileName}\" with \"{newFileName}\", which is not a file name.",
+                nameof(extension));
+        }
+
+        return Path.ChangeExtension(value, extension);
+    }
 }