Report path failures through the return type, and cover path, xdg and error

docs/sphinx/source/api/index.rst

@@ -53,10 +53,6 @@ Some operations are exceptions to that rule:
   ``array::operator[]`` and ``array::value_at()`` — throw
   ``std::out_of_range`` when the key or index is not present. Ask
   ``dictionary::contains()`` or ``array::length()`` before indexing.
-- ``path::expand()``, despite returning ``std::expected``, lets a
-  ``std::filesystem::filesystem_error`` escape for any canonicalisation
-  failure on a ``~`` path. ``path::resolve()`` catches those and returns
-  them.
 - ``path``'s ``exists()`` calls the throwing form of
   ``std::filesystem::exists``, so an error while querying the path — as
   opposed to the path simply being absent — escapes as a

docs/sphinx/source/api/platform.rst

@@ -58,12 +58,12 @@ The ``path`` class provides filesystem path operations:
         dross::path config_path = home->append(".config").append("app");
         std::cout << "Config path: " << config_path.string() << std::endl;
 
-        // Create the directory, including any missing parents. mkdir()
-        // reports failure when the directory is already there, so tell that
-        // case apart by its code(): zero means it was already present.
+        // Create the directory, including any missing parents. mkdir() is
+        // idempotent: it succeeds whether it creates the directory or
+        // finds it already there.
         if (auto created = dross::path::mkdir(config_path.string())) {
             std::cout << "Created: " << created->string() << std::endl;
-        } else if (created.error().code()) {
+        } else {
             std::cerr << "mkdir: " << created.error().what() << std::endl;
         }
     }
@@ -85,12 +85,16 @@ The ``path`` class provides filesystem path operations:
     }
 
     // Expand a leading ~ to the home directory. For a ~ path, expand()
-    // canonicalises without catching, so any canonicalisation failure --
-    // a missing target, a permission problem, a symlink loop -- escapes as
-    // a filesystem_error instead of being returned. resolve() catches it.
+    // canonicalises and turns any failure the standard library reports as
+    // a std::filesystem::filesystem_error -- a missing target, a
+    // permission problem, a symlink loop -- into the returned
+    // std::expected instead of letting it escape. A path that does not
+    // begin with ~ is returned unchanged.
     dross::path user_config{std::string{"~/.config/app"}};
     if (auto expanded = user_config.expand()) {
         std::cout << "Expanded: " << expanded->string() << std::endl;
+    } else {
+        std::cerr << "Expand failed: " << expanded.error().what() << std::endl;
     }
 
 Path Operations
@@ -120,20 +124,30 @@ Operations that consult the filesystem:
 
 Some caveats apply to the current implementation:
 
-- ``mkdir()`` succeeds only when it actually creates the directory. If the
-  path already exists it returns an error, but one whose ``code()`` is zero,
-  so a caller can tell it apart from a real filesystem failure, which
-  carries a nonzero code. Testing with ``exists()`` beforehand is not a
-  better answer — see below, and it races with other processes anyway.
-- ``expand()`` does not route every failure through its return type. For a
-  path beginning with ``~`` it canonicalises without catching, so *any*
-  canonicalisation failure — a missing target, a permission problem, a
-  symlink loop, an invalid component — escapes as a
-  ``std::filesystem::filesystem_error`` instead of being returned, which
-  terminates a program that is not catching it. A path that does not begin
-  with ``~`` is returned unchanged and never throws. ``resolve()`` catches
-  the same failures and returns them, so prefer it when the target may not
-  be reachable.
+- ``mkdir()`` is idempotent: it succeeds whether it creates
+  the directory or finds it already there. It fails only when
+  ``std::filesystem::create_directories`` reports an actual error, such as
+  a path component that exists and is not a directory. The operation is
+  not atomic — directories created before the failure may remain. Some
+  failures are rejected before anything is created at all. Because an
+  already-present directory is accepted without inspection, a directory,
+  or a symbolic link that resolves to one, left there by another party is
+  accepted too. Checking beforehand does not close that gap — the check
+  and the use are separate operations, and the entry can be replaced in
+  between. This call does not check who owns the directories along the
+  path, what their permissions are, or where any links beneath them point,
+  and it does not set the permissions of the directories it creates: those
+  are left to the platform's default for new directories, which can be
+  group- or world-writable. A caller who needs any of that has to arrange
+  it separately.
+- ``expand()`` routes canonicalisation failures through its return type.
+  For a path beginning with ``~`` it canonicalises and converts any error
+  the standard library reports as a ``std::filesystem::filesystem_error``
+  — a missing target, a permission problem, a symlink loop, an invalid
+  component — into the returned ``std::expected`` rather than letting it
+  escape. A path that does not begin with ``~`` is returned unchanged.
+  Home directory resolution failing (``path::home()`` returning
+  ``std::nullopt``) is reported the same way.
 - ``exists()`` calls the throwing form of ``std::filesystem::exists``. An
   absent path is simply ``false``, but an error while querying it — an
   over-long name, or a directory the process may not traverse — escapes as a
@@ -197,9 +211,8 @@ the application name passed to the constructor:
 
 Every accessor returns ``std::optional<std::string>`` and yields
 ``std::nullopt`` when the home directory cannot be determined. The directory
-itself is not created for you — pass the result to ``path::mkdir()``, keeping
-in mind that ``mkdir()`` reports an already-present directory as an error,
-recognisable by its zero ``code()``.
+itself is not created for you — pass the result to ``path::mkdir()``, which
+accepts an already-present directory as success.
 
 Example Usage
 ~~~~~~~~~~~~~
@@ -216,12 +229,11 @@ Creating application directories:
     dross::xdg app{"myapp"};
 
     // Create the config directory, then name a file inside it. mkdir()
-    // reports an already-present directory as a failure too, but with a
-    // zero code(), so only a nonzero one is a real problem.
+    // accepts an already-present directory as success, so any failure
+    // here is a real problem.
     if (auto config_home = app.config_home()) {
         const dross::path config_dir{*config_home};
-        if (auto created = dross::path::mkdir(*config_home);
-            !created && created.error().code()) {
+        if (auto created = dross::path::mkdir(*config_home); !created) {
             std::cerr << "mkdir: " << created.error().what() << std::endl;
         }
         dross::path config_file = config_dir.append("settings.toml");

docs/sphinx/source/changelog.rst

@@ -29,6 +29,12 @@ Changed
 - Enhanced error handling: ``timezone::from_string()`` returns ``std::optional<timezone>``
 - Simplified API: removed redundant timezone methods (``is_local()``, ``has_offset()``)
 - Updated documentation to reflect timestamp and timezone APIs
+- ``path::mkdir()`` now succeeds when the target directory already exists
+  instead of reporting it as a failure with a zero ``code()``; it fails
+  only when the underlying filesystem operation reports an actual error
+- ``path::expand()`` now reports canonicalisation failures on a ``~`` path
+  through its ``std::expected`` return value instead of letting a
+  ``std::filesystem::filesystem_error`` escape
 
 v0.1.0 - 2024-01-20
 -------------------

docs/sphinx/source/user-guide/index.rst

@@ -155,9 +155,8 @@ Platform Utilities
     xdg app{"myapp"};
     const std::string app_data = app.data_home().value_or(config_path.string());
 
-    // Create the directory. mkdir() succeeds only when it actually creates
-    // one, so an already-present directory comes back as an error too --
-    // but with a zero code(), unlike a real failure.
-    if (auto result = path::mkdir(app_data); !result && result.error().code()) {
+    // Create the directory. mkdir() is idempotent: it succeeds whether it
+    // creates the directory or finds it already there.
+    if (auto result = path::mkdir(app_data); !result) {
         std::cerr << "mkdir: " << result.error().what() << std::endl;
     }

include/dross/platform.h

@@ -26,7 +26,7 @@
  * // Filesystem operations
  * path config_dir = path::home().value_or(path{"/tmp"}) / "myapp";
  * if (auto result = path::mkdir(config_dir.string()); result) {
- *     // Directory created successfully
+ *     // Directory created, or already there
  * }
  * 
  * // XDG directories
@@ -40,8 +40,8 @@
  * - Uses std::optional for operations that may not return a value
  * - Uses std::expected for operations that may fail with detailed error info
  * - The platform layer throws nothing of its own, but std::filesystem
- * exceptions do propagate: see path::exists(), path::expand() and the
- * default path constructor
+ * exceptions do propagate: see path::exists() and the default path
+ * constructor
  * 
  * Platform support:
  * - Unix-like systems (Linux, macOS, BSD)

include/dross/platform/path.h

@@ -26,8 +26,8 @@ namespace dross {
  * - Uses std::expected<path, std::filesystem::filesystem_error> for fallible operations
  * - Uses std::optional<path> for operations that may not return a value
  * - No exceptions thrown directly, but std::filesystem ones propagate:
- *   exists(), expand() on a ~ path, and the default constructor all call
- *   throwing std::filesystem functions
+ *   exists() and the default constructor call throwing std::filesystem
+ *   functions
  * 
  * Performance characteristics:
  * - Thin wrapper over std::filesystem with minimal overhead
@@ -71,10 +71,11 @@ class path {
      * @return Expected containing the created path on success, or filesystem_error on failure
      * 
      * Creates the specified directory and any necessary parent directories.
-     * Succeeds only when a directory is actually created: if dir_path is
-     * already present the call reports failure. That case is still
-     * recognisable — the reported error's code() is zero, whereas a real
-     * filesystem failure carries a nonzero code.
+     * Succeeds both when it creates the directory and when dir_path is
+     * already a directory — the call is idempotent. It fails only when the
+     * underlying std::filesystem::create_directories call reports an
+     * actual error. See the std::filesystem::path overload for the
+     * failure and safety notes.
      *
      * @code
      * if (auto result = path::mkdir("/tmp/myapp/data")) {
@@ -91,11 +92,26 @@ class path {
      * @param dir_path The directory path to create as a filesystem::path
      * @return Expected containing the created path on success, or filesystem_error on failure
      * 
-     * Creates the specified directory and any necessary parent directories.
-     * Succeeds only when a directory is actually created: if dir_path is
-     * already present the call reports failure, with an error whose code()
-     * is zero; a real filesystem failure carries a nonzero code. This
-     * overload holds the logic; the std::string one forwards to it.
+     * Creates the specified directory and any necessary parent
+     * directories. Succeeds both when it creates the directory and
+     * when dir_path is already a directory — the call is idempotent,
+     * closer to "ensure this directory exists" than a strict create. It
+     * fails only when std::filesystem::create_directories reports an
+     * actual error, for example when a path component exists and is not
+     * a directory. The operation is not atomic — directories created
+     * before the failure may remain. Some failures are rejected before
+     * anything is created at all. Because an already-present directory is
+     * accepted without inspection, a directory, or a symbolic link that
+     * resolves to one, left there by another party is accepted too.
+     * Checking beforehand does not close that gap — the check and the use
+     * are separate operations, and the entry can be replaced in between.
+     * This call does not check who owns the directories along the path,
+     * what their permissions are, or where any links beneath them point,
+     * and it does not set the permissions of the directories it creates:
+     * those are left to the platform's default for new directories, which
+     * can be group- or world-writable. A caller who needs any of that has
+     * to arrange it separately. This overload holds the logic; the
+     * std::string one forwards to it.
      */
     static std::expected<path, std::filesystem::filesystem_error> mkdir(const std::filesystem::path& dir_path);
 
@@ -200,14 +216,22 @@ class path {
     /**
      * @brief Expand user home directory (~) in the path.
      * @return Expected containing the expanded path on success, or filesystem_error on failure
-     * 
-     * Expands tilde (~) notation to the actual home directory path.
-     * Only processes paths that start with "~" or "~/".
-     * 
+     *
+     * Expands tilde (~) notation to the actual home directory path, then
+     * canonicalises the result — the returned path has symbolic links
+     * resolved. Because canonicalisation requires the target to exist,
+     * expand() returns unexpected when the expanded path does not (yet)
+     * exist. A path that does not start with "~" is returned unchanged
+     * and always succeeds.
+     *
      * @code
-     * path user_config{"~/.config/myapp"};
+     * path user_config{std::string{"~/.config/myapp"}};
      * if (auto expanded = user_config.expand()) {
      *     // expanded contains something like "/home/user/.config/myapp"
+     * } else {
+     *     // Suppose the target does not exist yet. mkdir() does not
+     *     // expand ~, so build the path from path::home() before
+     *     // creating it.
      * }
      * @endcode
      */

include/dross/platform/xdg.h

@@ -99,7 +99,8 @@ class xdg {
      *     // Typically returns something like "/home/user/.config/myapp"
      *     path config_path{*config_dir};
      *     if (auto result = path::mkdir(config_path.string())) {
-     *         // Directory created and ready for config files
+     *         // Directory ready for config files, whether just created or
+     *         // already there
      *     }
      * }
      * @endcode

src/platform/path.cpp

@@ -14,13 +14,13 @@ std::expected<path, std::filesystem::filesystem_error> path::mkdir(const std::st
 
 std::expected<path, std::filesystem::filesystem_error> path::mkdir(const std::filesystem::path& absolute_path)
 {
-    try {
-        std::error_code err;
-        if (! std::filesystem::create_directories(absolute_path, err)) {
-            return std::unexpected(std::filesystem::filesystem_error("failed", absolute_path, err));
-        }
-    } catch (const std::filesystem::filesystem_error& e) {
-        return std::unexpected(e);
+    std::error_code err;
+    // create_directories() returns false both when the directory already
+    // existed and when it failed to create one, so the return value alone
+    // cannot tell success from failure; check err instead.
+    std::filesystem::create_directories(absolute_path, err);
+    if (err) {
+        return std::unexpected(std::filesystem::filesystem_error("failed", absolute_path, err));
     }
 
     return path{absolute_path};
@@ -97,7 +97,11 @@ std::expected<path, std::filesystem::filesystem_error> path::expand() const
             return std::make_optional(p.append(_path.string().replace(0, 1, "")).string());
         });
         if (expanded) {
-            return path { std::filesystem::canonical(std::filesystem::path{expanded.value()}) };
+            try {
+                return path { std::filesystem::canonical(std::filesystem::path{expanded.value()}) };
+            } catch (const std::filesystem::filesystem_error& e) {
+                return std::unexpected(e);
+            }
         } else {
             return std::unexpected(std::filesystem::filesystem_error("fail to expand tilde", _path, std::make_error_code(std::errc::no_such_file_or_directory)));
         }

test/CMakeLists.txt

@@ -19,10 +19,13 @@ add_executable(${TEST_NAME})
 
 target_sources(${TEST_NAME} PRIVATE
     ${CMAKE_CURRENT_SOURCE_DIR}/platform/test_environment.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/platform/test_path.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/platform/test_xdg.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_array.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_boolean.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_data.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_dictionary.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/type/test_error.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_number.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_string.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/type/test_timestamp.cpp