feat: make it possible to disable cleanup in-place after creation (#355)

This makes it possible to disable automatic cleanup of temporary files &
directories _after_ they've been created.

1. Rename `Builder::keep` into `Builder::disable_cleanup` for
consistency. `::keep` on `NamedTempDir`, `TempDir`, `TempPath`, etc.
provide a way to _convert_ (one-way) a temporary directory/file into a
persistent temporary directory/file but aren't a way to simply disable
automatic cleanup in-place.
2. Add `disable_cleanup` methods to all named temporary file types,
making it possible to replicate the behavior of
`Builder::disable_cleanup` _after_ creating a temporary file. Given that
this operation is in-place, it can be undone.

fixes #347

Author: Stebalien

src/dir/imp/any.rs

@@ -10,7 +10,7 @@ fn not_supported<T>(msg: &str) -> io::Result<T> {
 pub fn create(
     path: PathBuf,
     permissions: Option<&std::fs::Permissions>,
-    keep: bool,
+    disable_cleanup: bool,
 ) -> io::Result<TempDir> {
     if permissions.map_or(false, |p| p.readonly()) {
         return not_supported("changing permissions is not supported on this platform");
@@ -19,6 +19,6 @@ pub fn create(
         .with_err_path(|| &path)
         .map(|_| TempDir {
             path: path.into_boxed_path(),
-            keep,
+            disable_cleanup,
         })
 }

src/dir/imp/unix.rs

@@ -6,7 +6,7 @@ use std::path::PathBuf;
 pub fn create(
     path: PathBuf,
     permissions: Option<&std::fs::Permissions>,
-    keep: bool,
+    disable_cleanup: bool,
 ) -> io::Result<TempDir> {
     let mut dir_options = std::fs::DirBuilder::new();
     #[cfg(not(target_os = "wasi"))]
@@ -21,6 +21,6 @@ pub fn create(
         .with_err_path(|| &path)
         .map(|_| TempDir {
             path: path.into_boxed_path(),
-            keep,
+            disable_cleanup,
         })
 }

src/dir/mod.rs

@@ -182,7 +182,7 @@ pub fn tempdir_in<P: AsRef<Path>>(dir: P) -> io::Result<TempDir> {
 /// [`std::process::exit()`]: http://doc.rust-lang.org/std/process/fn.exit.html
 pub struct TempDir {
     path: Box<Path>,
-    keep: bool,
+    disable_cleanup: bool,
 }
 
 impl TempDir {
@@ -394,6 +394,9 @@ impl TempDir {
     /// This consumes the [`TempDir`] without deleting directory on the filesystem, meaning that
     /// the directory will no longer be automatically deleted.
     ///
+    /// If you want to disable automatic cleanup of the temporary directory in-place, keeping the
+    /// `TempDir` as-is, use [`TempDir::disable_cleanup`] instead.
+    ///
     /// [`TempDir`]: struct.TempDir.html
     /// [`PathBuf`]: http://doc.rust-lang.org/std/path/struct.PathBuf.html
     ///
@@ -414,13 +417,19 @@ impl TempDir {
     /// # Ok::<(), std::io::Error>(())
     /// ```
     #[must_use]
-    pub fn keep(self) -> PathBuf {
-        // Prevent the Drop impl from being called.
-        let mut this = mem::ManuallyDrop::new(self);
+    pub fn keep(mut self) -> PathBuf {
+        self.disable_cleanup(true);
+        mem::replace(&mut self.path, PathBuf::new().into_boxed_path()).into()
+    }
 
-        // replace this.path with an empty Box, since an empty Box does not
-        // allocate any heap memory.
-        mem::replace(&mut this.path, PathBuf::new().into_boxed_path()).into()
+    /// Disable cleanup of the temporary directory. If `disable_cleanup` is `true`, the temporary
+    /// directory will not be deleted when this `TempDir` is dropped. This method is equivalent to
+    /// calling [`Builder::disable_cleanup`] when creating the `TempDir`.
+    ///
+    /// **NOTE:** this method is primarily useful for testing/debugging. If you want to simply turn
+    /// a temporary directory into a non-temporary directory, prefer [`TempDir::keep`].
+    pub fn disable_cleanup(&mut self, disable_cleanup: bool) {
+        self.disable_cleanup = disable_cleanup
     }
 
     /// Closes and removes the temporary directory, returning a `Result`.
@@ -490,7 +499,7 @@ impl fmt::Debug for TempDir {
 
 impl Drop for TempDir {
     fn drop(&mut self) {
-        if !self.keep {
+        if !self.disable_cleanup {
             let _ = remove_dir_all(self.path());
         }
     }
@@ -499,9 +508,9 @@ impl Drop for TempDir {
 pub(crate) fn create(
     path: PathBuf,
     permissions: Option<&std::fs::Permissions>,
-    keep: bool,
+    disable_cleanup: bool,
 ) -> io::Result<TempDir> {
-    imp::create(path, permissions, keep)
+    imp::create(path, permissions, disable_cleanup)
 }
 
 mod imp;

src/file/mod.rs

@@ -124,11 +124,12 @@ impl error::Error for PathPersistError {
 /// This is useful when the temporary file needs to be used by a child process,
 /// for example.
 ///
-/// When dropped, the temporary file is deleted unless `keep(true)` was called
-/// on the builder that constructed this value.
+/// When dropped, the temporary file is deleted unless `disable_cleanup(true)` was called on the
+/// builder that constructed this temporary file and/or was called on either this `TempPath` or the
+/// `NamedTempFile` from which this `TempPath` was constructed.
 pub struct TempPath {
     path: Box<Path>,
-    keep: bool,
+    disable_cleanup: bool,
 }
 
 impl TempPath {
@@ -298,12 +299,13 @@ impl TempPath {
     pub fn keep(mut self) -> Result<PathBuf, PathPersistError> {
         match imp::keep(&self.path) {
             Ok(_) => {
-                // Don't drop `self`. We don't want to try deleting the old
-                // temporary file path. (It'll fail, but the failure is never
-                // seen.)
-                let path = mem::replace(&mut self.path, PathBuf::new().into_boxed_path());
-                mem::forget(self);
-                Ok(path.into())
+                self.disable_cleanup(true);
+                Ok(mem::replace(
+                    &mut self.path,
+                    // Replace with an empty boxed path buf, this doesn't allocate.
+                    PathBuf::new().into_boxed_path(),
+                )
+                .into_path_buf())
             }
             Err(e) => Err(PathPersistError {
                 error: e,
@@ -312,6 +314,17 @@ impl TempPath {
         }
     }
 
+    /// Disable cleanup of the temporary file. If `disable_cleanup` is `true`, the temporary file
+    /// will not be deleted when this `TempPath` is dropped. This method is equivalent to calling
+    /// [`Builder::disable_cleanup`] when creating the original `NamedTempFile`, which see for
+    /// relevant warnings.
+    ///
+    /// **NOTE:** this method is primarily useful for testing/debugging. If you want to simply turn
+    /// a temporary file-path into a non-temporary file-path, prefer [`TempPath::keep`].
+    pub fn disable_cleanup(&mut self, disable_cleanup: bool) {
+        self.disable_cleanup = disable_cleanup
+    }
+
     /// Create a new TempPath from an existing path. This can be done even if no
     /// file exists at the given path.
     ///
@@ -321,14 +334,14 @@ impl TempPath {
     pub fn from_path(path: impl Into<PathBuf>) -> Self {
         Self {
             path: path.into().into_boxed_path(),
-            keep: false,
+            disable_cleanup: false,
         }
     }
 
-    pub(crate) fn new(path: PathBuf, keep: bool) -> Self {
+    pub(crate) fn new(path: PathBuf, disable_cleanup: bool) -> Self {
         Self {
             path: path.into_boxed_path(),
-            keep,
+            disable_cleanup,
         }
     }
 }
@@ -341,7 +354,7 @@ impl fmt::Debug for TempPath {
 
 impl Drop for TempPath {
     fn drop(&mut self) {
-        if !self.keep {
+        if !self.disable_cleanup {
             let _ = fs::remove_file(&self.path);
         }
     }
@@ -775,7 +788,6 @@ impl<F> NamedTempFile<F> {
     /// Keep the temporary file from being deleted. This function will turn the
     /// temporary file into a non-temporary file without moving it.
     ///
-    ///
     /// # Errors
     ///
     /// On some platforms (e.g., Windows), we need to mark the file as
@@ -806,6 +818,17 @@ impl<F> NamedTempFile<F> {
         }
     }
 
+    /// Disable cleanup of the temporary file. If `disable_cleanup` is `true`, the temporary file
+    /// will not be deleted when this `TempPath` is dropped. This method is equivalent to calling
+    /// [`Builder::disable_cleanup`] when creating the original `NamedTempFile`, which see for
+    /// relevant warnings.
+    ///
+    /// **NOTE:** this method is primarily useful for testing/debugging. If you want to simply turn
+    /// a temporary file into a non-temporary file, prefer [`NamedTempFile::keep`].
+    pub fn disable_cleanup(&mut self, disable_cleanup: bool) {
+        self.path.disable_cleanup(disable_cleanup)
+    }
+
     /// Get a reference to the underlying file.
     pub fn as_file(&self) -> &F {
         &self.file
@@ -1048,7 +1071,7 @@ pub(crate) fn create_named(
         .map(|file| NamedTempFile {
             path: TempPath {
                 path: path.into_boxed_path(),
-                keep,
+                disable_cleanup: keep,
             },
             file,
         })

src/lib.rs

@@ -228,7 +228,7 @@ pub struct Builder<'a, 'b> {
     suffix: &'b OsStr,
     append: bool,
     permissions: Option<std::fs::Permissions>,
-    keep: bool,
+    disable_cleanup: bool,
 }
 
 impl Default for Builder<'_, '_> {
@@ -239,7 +239,7 @@ impl Default for Builder<'_, '_> {
             suffix: OsStr::new(""),
             append: false,
             permissions: None,
-            keep: false,
+            disable_cleanup: false,
         }
     }
 }
@@ -465,31 +465,42 @@ impl<'a, 'b> Builder<'a, 'b> {
         self
     }
 
-    /// Set the file/folder to be kept even when the [`NamedTempFile`]/[`TempDir`] goes out of
-    /// scope.
+    /// Disable cleanup of the file/folder to even when the [`NamedTempFile`]/[`TempDir`] goes out
+    /// of scope. Prefer [`NamedTempFile::keep`] and `[`TempDir::keep`] where possible,
+    /// `disable_cleanup` is provided for testing & debugging.
     ///
     /// By default, the file/folder is automatically cleaned up in the destructor of
-    /// [`NamedTempFile`]/[`TempDir`]. When `keep` is set to `true`, this behavior is suppressed.
+    /// [`NamedTempFile`]/[`TempDir`]. When `disable_cleanup` is set to `true`, this behavior is
+    /// suppressed. If you wish to disable cleanup after creating a temporary file/directory, call
+    /// [`NamedTempFile::disable_cleanup`] or [`TempDir::disable_cleanup`].
     ///
-    /// If you wish to keep a temporary file or directory after creating it, call
-    /// [`NamedTempFile::keep`] or [`TempDir::keep`] respectively to turn it into a regular
-    /// file/path (respectively) that won't be automatically deleted.
+    /// # Warnings
+    ///
+    /// On some platforms (for now, only Windows), temporary files are marked with a special
+    /// "temporary file" (`FILE_ATTRIBUTE_TEMPORARY`) attribute. Disabling cleanup _will not_ unset
+    /// this attribute while calling [`NamedTempFile::keep`] will.
     ///
     /// # Examples
     ///
     /// ```
     /// use tempfile::Builder;
     ///
     /// let named_tempfile = Builder::new()
-    ///     .keep(true)
+    ///     .disable_cleanup(true)
     ///     .tempfile()?;
     /// # Ok::<(), std::io::Error>(())
     /// ```
-    pub fn keep(&mut self, keep: bool) -> &mut Self {
-        self.keep = keep;
+    pub fn disable_cleanup(&mut self, disable_cleanup: bool) -> &mut Self {
+        self.disable_cleanup = disable_cleanup;
         self
     }
 
+    /// Deprecated alias for [`Builder::disable_cleanup`].
+    #[deprecated = "Use Builder::disable_cleanup"]
+    pub fn keep(&mut self, keep: bool) -> &mut Self {
+        self.disable_cleanup(keep)
+    }
+
     /// Create the named temporary file.
     ///
     /// # Security
@@ -555,7 +566,7 @@ impl<'a, 'b> Builder<'a, 'b> {
                     path,
                     OpenOptions::new().append(self.append),
                     self.permissions.as_ref(),
-                    self.keep,
+                    self.disable_cleanup,
                 )
             },
         )
@@ -616,7 +627,7 @@ impl<'a, 'b> Builder<'a, 'b> {
             self.prefix,
             self.suffix,
             self.random_len,
-            |path| dir::create(path, self.permissions.as_ref(), self.keep),
+            |path| dir::create(path, self.permissions.as_ref(), self.disable_cleanup),
         )
     }
 
@@ -741,7 +752,7 @@ impl<'a, 'b> Builder<'a, 'b> {
             move |path| {
                 Ok(NamedTempFile::from_parts(
                     f(&path)?,
-                    TempPath::new(path, self.keep),
+                    TempPath::new(path, self.disable_cleanup),
                 ))
             },
         )

tests/namedtempfile.rs

@@ -411,22 +411,40 @@ fn test_keep() {
 }
 
 #[test]
-fn test_builder_keep() {
+fn test_disable_cleanup() {
     configure_wasi_temp_dir();
 
-    let mut tmpfile = Builder::new().keep(true).tempfile().unwrap();
-    write!(tmpfile, "abcde").unwrap();
-    let path = tmpfile.path().to_owned();
-    drop(tmpfile);
+    // Case 0: never mark as "disable cleanup"
+    // Case 1: enable "disable cleanup" in the builder, don't touch it after.
+    // Case 2: enable "disable cleanup" in the builder, turn it off after.
+    // Case 3: don't enable disable cleanup in the builder, turn it on after.
+
+    for case in 0..4 {
+        let in_builder = case & 1 > 0;
+        let toggle = case & 2 > 0;
+        let mut tmpfile = Builder::new()
+            .disable_cleanup(in_builder)
+            .tempfile()
+            .unwrap();
+        write!(tmpfile, "abcde").unwrap();
+        if toggle {
+            tmpfile.disable_cleanup(!in_builder);
+        }
 
-    {
-        // Try opening it again.
-        let mut f = File::open(&path).unwrap();
-        let mut buf = String::new();
-        f.read_to_string(&mut buf).unwrap();
-        assert_eq!("abcde", buf);
+        let path = tmpfile.path().to_owned();
+        drop(tmpfile);
+
+        if in_builder ^ toggle {
+            // Try opening it again.
+            let mut f = File::open(&path).unwrap();
+            let mut buf = String::new();
+            f.read_to_string(&mut buf).unwrap();
+            assert_eq!("abcde", buf);
+            std::fs::remove_file(&path).unwrap();
+        } else {
+            assert!(!path.exists(), "tempfile wasn't deleted");
+        }
     }
-    std::fs::remove_file(&path).unwrap();
 }
 
 #[test]