IPath.WithExtension: behavior and documentation improvements

CHANGELOG.md

@@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - **Breaking:** on Windows, `LocalPath.IsAbsolute` now only returns `true` for fully qualified paths, such as `C:\Windows`. Paths rooted without a drive letter (`\Windows`), and paths relative to the current directory of a drive (`C:Windows`, `C:`) are not absolute anymore, and `AbsolutePath` now rejects them. Previously, this was following the `Path.IsPathRooted` API.
 - **Breaking:** `LocalPath.IsPrefixOf` and `StartsWith` now consider paths of different `PathKind` unrelated: an absolute path is never a prefix of a relative one, nor the other way round, and on Windows `\Windows`, `C:Windows` and `Windows` are all unrelated to each other. Previously, the result depended on a string comparison.
 - **Breaking:** on Windows, the `/` operator of `LocalPath` and `AbsolutePath` no longer delegates to `Path.Combine`, and instead follows the algorithm of C++ `std::filesystem::path::operator/` ([fs.path.append](https://eel.is/c++draft/fs.path.append)) with case-insensitive drive letters. In particular, `C:\base / \x` is now `C:\x` (was `\x`), `C:\base / C:x` is now `C:\base\x` (was `C:x`), and `C: / x` is now `C:x` (was `C:\x`). On Unix, the behavior is unchanged.
+- **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`.
 - On Windows, `AbsolutePath / LocalPath` now resolves a path relative to the current directory of another drive against the current directory of that drive, so the result is always absolute: e.g. `C:\base / D:x` is `D:\x` if the current directory of drive `D:` is its root. Previously, the result was `D:x`, which is not absolute.
 - On Windows, `LocalPath.ResolveToCurrentDirectory` now resolves `\x` against the drive of the current directory, and `D:x` against the current directory of drive `D:`.
 - **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`.

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,64 @@ 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")]
+    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
+    ///     exceptions section 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 (Windows) 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: e.g. replacing <c>.gitignore</c> with <see langword="null"/> would leave nothing at all, and
+    ///         <c>file.txt</c> with the extension <c>foo/bar</c> would add 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,24 @@ 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 an <see cref="ArgumentException"/> 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 +127,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: replacing <c>.gitignore</c> with <see langword="null"/> would leave nothing at all, and
+    ///     <c>file.txt</c> with the extension <c>foo/bar</c> would add 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 +159,21 @@ 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 an <see cref="ArgumentException"/> 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 +185,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: e.g. replacing <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 add 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 +217,46 @@ 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 an <see cref="ArgumentException"/> 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);
+    }
 }