From 56087074c655bc17af048ad6483ecf7c32ce1762 Mon Sep 17 00:00:00 2001
From: Martin Habovstiak <martin.habovstiak@gmail.com>
Date: Thu, 9 Jun 2022 12:34:23 +0200
Subject: [PATCH] Stabilize `Path::try_exists()` and improve doc

This stabilizes the `Path::try_exists()` method which returns
`Result<bool, io::Error>` instead of `bool` allowing handling of errors
unrelated to the file not existing. (e.g permission errors)

Along with the stabilization it also:

* Warns that the `exists()` method is error-prone and suggests to use
  the newly stabilized one.
* Suggests it instead of `metadata()` to handle errors.
* Mentions TOCTOU bugs to avoid false assumption that `try_exists()` is
  completely safe fixed version of `exists()`.
* Renames the feature of still-unstable `std::fs::try_exists()` to
  `fs_try_exists` to avoid name conflict.

The tracking issue #83186 remains open to track `fs_try_exists`.
---
 compiler/rustc_error_messages/src/lib.rs |  1 -
 library/std/src/fs.rs                    |  8 ++++++--
 library/std/src/path.rs                  | 17 +++++++++++------
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs
index fd4b2daae9c13..b21ff0e90836d 100644
--- a/compiler/rustc_error_messages/src/lib.rs
+++ b/compiler/rustc_error_messages/src/lib.rs
@@ -1,6 +1,5 @@
 #![feature(let_chains)]
 #![feature(once_cell)]
-#![feature(path_try_exists)]
 #![feature(rustc_attrs)]
 #![feature(type_alias_impl_trait)]
 
diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs
index 55bd2c59406df..f46997b807ab2 100644
--- a/library/std/src/fs.rs
+++ b/library/std/src/fs.rs
@@ -2317,10 +2317,14 @@ impl AsInnerMut<fs_imp::DirBuilder> for DirBuilder {
 /// unrelated to the path not existing. (E.g. it will return `Err(_)` in case of permission
 /// denied on some of the parent directories.)
 ///
+/// Note that while this avoids some pitfalls of the `exists()` method, it still can not
+/// prevent time-of-check to time-of-use (TOCTOU) bugs. You should only use it in scenarios
+/// where those bugs are not an issue.
+///
 /// # Examples
 ///
 /// ```no_run
-/// #![feature(path_try_exists)]
+/// #![feature(fs_try_exists)]
 /// use std::fs;
 ///
 /// assert!(!fs::try_exists("does_not_exist.txt").expect("Can't check existence of file does_not_exist.txt"));
@@ -2330,7 +2334,7 @@ impl AsInnerMut<fs_imp::DirBuilder> for DirBuilder {
 /// [`Path::exists`]: crate::path::Path::exists
 // FIXME: stabilization should modify documentation of `exists()` to recommend this method
 // instead.
-#[unstable(feature = "path_try_exists", issue = "83186")]
+#[unstable(feature = "fs_try_exists", issue = "83186")]
 #[inline]
 pub fn try_exists<P: AsRef<Path>>(path: P) -> io::Result<bool> {
     fs_imp::try_exists(path.as_ref())
diff --git a/library/std/src/path.rs b/library/std/src/path.rs
index 36d6469c02d31..c56d5aa9b2e8d 100644
--- a/library/std/src/path.rs
+++ b/library/std/src/path.rs
@@ -2705,6 +2705,9 @@ impl Path {
 
     /// Returns `true` if the path points at an existing entity.
     ///
+    /// Warning: this method may be error-prone, consider using [`try_exists()`] instead!
+    /// It also has a risk of introducing time-of-check to time-of-use (TOCTOU) bugs.
+    ///
     /// This function will traverse symbolic links to query information about the
     /// destination file.
     ///
@@ -2721,7 +2724,9 @@ impl Path {
     /// # See Also
     ///
     /// This is a convenience function that coerces errors to false. If you want to
-    /// check errors, call [`fs::metadata`].
+    /// check errors, call [`Path::try_exists`].
+    ///
+    /// [`try_exists()`]: Self::try_exists
     #[stable(feature = "path_ext", since = "1.5.0")]
     #[must_use]
     #[inline]
@@ -2738,20 +2743,20 @@ impl Path {
     /// unrelated to the path not existing. (E.g. it will return `Err(_)` in case of permission
     /// denied on some of the parent directories.)
     ///
+    /// Note that while this avoids some pitfalls of the `exists()` method, it still can not
+    /// prevent time-of-check to time-of-use (TOCTOU) bugs. You should only use it in scenarios
+    /// where those bugs are not an issue.
+    ///
     /// # Examples
     ///
     /// ```no_run
-    /// #![feature(path_try_exists)]
-    ///
     /// use std::path::Path;
     /// assert!(!Path::new("does_not_exist.txt").try_exists().expect("Can't check existence of file does_not_exist.txt"));
     /// assert!(Path::new("/root/secret_file.txt").try_exists().is_err());
     /// ```
     ///
     /// [`exists()`]: Self::exists
-    // FIXME: stabilization should modify documentation of `exists()` to recommend this method
-    // instead.
-    #[unstable(feature = "path_try_exists", issue = "83186")]
+    #[stable(feature = "path_try_exists", since = "1.63.0")]
     #[inline]
     pub fn try_exists(&self) -> io::Result<bool> {
         fs::try_exists(self)