about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJacob Pratt <jacob@jhpratt.dev>2025-03-23 20:44:11 -0400
committerGitHub <noreply@github.com>2025-03-23 20:44:11 -0400
commit8e30df7f26e3a637f0bb41f877871a240bcbbe06 (patch)
tree68af7fc2d40dcf8227657af39e2cd62119981a08
parent0fc6279ce9775005f60eeef8440f3a2bcce74ce3 (diff)
parent6b2fa32f142f341ba227b5ea784ea2a1a5e79fe8 (diff)
downloadrust-8e30df7f26e3a637f0bb41f877871a240bcbbe06.tar.gz
rust-8e30df7f26e3a637f0bb41f877871a240bcbbe06.zip
Rollup merge of #138671 - ChrisDenton:filetype, r=joshtriplett
Fix `FileType` `PartialEq` implementation on Windows

Fixes #138668

On Windows the [`FileType`](https://doc.rust-lang.org/stable/std/fs/struct.FileType.html) struct was deriving `PartialEq` which in turn means it was doing a bit-for-bit comparison on the file attributes and reparse point. This is wrong because `attributes` may contain many things unrelated to file type.

`FileType` on Windows allows for four possible combinations (see also [`FileTypeExt`](https://doc.rust-lang.org/stable/std/os/windows/fs/trait.FileTypeExt.html)): `file`, `dir`, `symlink_file` and `symlink_dir`. So the new implementation makes sure both symlink and directory information match (and only those things).

This could be considered just a bug fix but it is a behaviour change so someone from libs-api might want to FCP this (or might not)...
-rw-r--r--library/std/src/fs/tests.rs17
-rw-r--r--library/std/src/sys/fs/windows.rs33
2 files changed, 32 insertions, 18 deletions
diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs
index 6dd18e4f4c8..4712e58980c 100644
--- a/library/std/src/fs/tests.rs
+++ b/library/std/src/fs/tests.rs
@@ -1719,6 +1719,23 @@ fn test_eq_direntry_metadata() {
     }
 }
 
+/// Test that windows file type equality is not affected by attributes unrelated
+/// to the file type.
+#[test]
+#[cfg(target_os = "windows")]
+fn test_eq_windows_file_type() {
+    let tmpdir = tmpdir();
+    let file1 = File::create(tmpdir.join("file1")).unwrap();
+    let file2 = File::create(tmpdir.join("file2")).unwrap();
+    assert_eq!(file1.metadata().unwrap().file_type(), file2.metadata().unwrap().file_type());
+
+    // Change the readonly attribute of one file.
+    let mut perms = file1.metadata().unwrap().permissions();
+    perms.set_readonly(true);
+    file1.set_permissions(perms).unwrap();
+    assert_eq!(file1.metadata().unwrap().file_type(), file2.metadata().unwrap().file_type());
+}
+
 /// Regression test for https://github.com/rust-lang/rust/issues/50619.
 #[test]
 #[cfg(target_os = "linux")]
diff --git a/library/std/src/sys/fs/windows.rs b/library/std/src/sys/fs/windows.rs
index 362e64abf1a..06bba019393 100644
--- a/library/std/src/sys/fs/windows.rs
+++ b/library/std/src/sys/fs/windows.rs
@@ -41,8 +41,8 @@ pub struct FileAttr {
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 pub struct FileType {
-    attributes: u32,
-    reparse_tag: u32,
+    is_directory: bool,
+    is_symlink: bool,
 }
 
 pub struct ReadDir {
@@ -1111,32 +1111,29 @@ impl FileTimes {
 }
 
 impl FileType {
-    fn new(attrs: u32, reparse_tag: u32) -> FileType {
-        FileType { attributes: attrs, reparse_tag }
+    fn new(attributes: u32, reparse_tag: u32) -> FileType {
+        let is_directory = attributes & c::FILE_ATTRIBUTE_DIRECTORY != 0;
+        let is_symlink = {
+            let is_reparse_point = attributes & c::FILE_ATTRIBUTE_REPARSE_POINT != 0;
+            let is_reparse_tag_name_surrogate = reparse_tag & 0x20000000 != 0;
+            is_reparse_point && is_reparse_tag_name_surrogate
+        };
+        FileType { is_directory, is_symlink }
     }
     pub fn is_dir(&self) -> bool {
-        !self.is_symlink() && self.is_directory()
+        !self.is_symlink && self.is_directory
     }
     pub fn is_file(&self) -> bool {
-        !self.is_symlink() && !self.is_directory()
+        !self.is_symlink && !self.is_directory
     }
     pub fn is_symlink(&self) -> bool {
-        self.is_reparse_point() && self.is_reparse_tag_name_surrogate()
+        self.is_symlink
     }
     pub fn is_symlink_dir(&self) -> bool {
-        self.is_symlink() && self.is_directory()
+        self.is_symlink && self.is_directory
     }
     pub fn is_symlink_file(&self) -> bool {
-        self.is_symlink() && !self.is_directory()
-    }
-    fn is_directory(&self) -> bool {
-        self.attributes & c::FILE_ATTRIBUTE_DIRECTORY != 0
-    }
-    fn is_reparse_point(&self) -> bool {
-        self.attributes & c::FILE_ATTRIBUTE_REPARSE_POINT != 0
-    }
-    fn is_reparse_tag_name_surrogate(&self) -> bool {
-        self.reparse_tag & 0x20000000 != 0
+        self.is_symlink && !self.is_directory
     }
 }