Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs::FileType and Metadata should document that is_file, is_dir, and is_symlink are mutually exclusive. #48345

Closed
ExpHP opened this issue Feb 19, 2018 · 3 comments
Closed

fs::FileType and Metadata should document that is_file, is_dir, and is_symlink are mutually exclusive. #48345

ExpHP opened this issue Feb 19, 2018 · 3 comments
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@ExpHP
Copy link
Contributor

ExpHP commented Feb 19, 2018

fs::FileType should document that is_file(), is_dir(), and is_symlink() are all mutually exclusive. (and that it may also be the case that none of them are true)

Why? Because Path::is_file() returns true for both files and symlinks to files, and this tends to make me paranoid that a fs::FileType will share similar characteristics and perhaps claim that both x.is_file() and x.is_symlink(). The only way to convince myself that FileType's behavior is indeed sane is to write a little test program---again.

Surely I can't be alone in this.
@ExpHP ExpHP changed the title fs::FileType should document that is_file, is_dir, and is_symlink are mutually exclusive. fs::FileType should document that is_file, is_dir, and is_symlink are mutually exclusive. Feb 19, 2018
@retep998
Copy link
Member

retep998 commented Feb 19, 2018 •
edited
Loading

Path::is_file() is calling stat not lstat (and equivalent on Windows), so if the path is a symlink, then it returns the results based on what the symlink points to, not the symlink itself. So Path::is_file() respects the same mutual exclusivity rules as fs::FileType.

But yes, this ought to be documented, especially given that the mutually exclusive behavior may be surprising to Windows developers.

@retep998 retep998 added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Feb 19, 2018
@ExpHP ExpHP changed the title fs::FileType should document that is_file, is_dir, and is_symlink are mutually exclusive. fs::FileType and Metadata should document that is_file, is_dir, and is_symlink are mutually exclusive. Feb 19, 2018
@ExpHP
Copy link
Contributor Author

ExpHP commented Feb 19, 2018

Similar goes for Metadata::{is_file,is_dir}, which should document that these are always false for a Metadata obtained by calling symlink_metadata on a symlink.

@retep998
Copy link
Member

Well you can always call symlink_metadata on something that is not a symlink, in which case is_file or is_dir could be true.

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Feb 20, 2018
bobdavelisafrank pushed a commit to bobdavelisafrank/rust that referenced this issue Mar 16, 2018
Fix Issue rust-lang#48345, is_file, is_dir, and is_symlink note mutua…
aaac69f 
…l exclusion

The methods on the structures `fs::FileType` and `fs::Metadata` of (respectively) `is_file`, `is_dir`, and
`is_symlink` had some ambiguity in documentation, where it was not noted whether files will pass those tests
exclusively or not. It is now written that the tests are mutually exclusive.

Fixes rust-lang#48345.
@bobdavelisafrank bobdavelisafrank mentioned this issue Mar 16, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this issue Mar 24, 2018
@kennytm
Rollup merge of rust-lang#49076 - bobdavelisafrank:filetype-metadata-…
101f7c2 
…docfix, r=bluss

Fix Issue rust-lang#48345, is_file, is_dir, and is_symlink note mutual exclusion

The methods on the structures `fs::FileType` and `fs::Metadata` of (respectively) `is_file`, `is_dir`, and `is_symlink` had some ambiguity in documentation, where it was not noted whether files will pass those tests exclusively or not. It is now written that the tests are mutually exclusive.

Fixes rust-lang#48345.
kennytm added a commit to kennytm/rust that referenced this issue Mar 24, 2018
@kennytm
Rollup merge of rust-lang#49076 - bobdavelisafrank:filetype-metadata-…
17d39ed 
…docfix, r=bluss

Fix Issue rust-lang#48345, is_file, is_dir, and is_symlink note mutual exclusion

The methods on the structures `fs::FileType` and `fs::Metadata` of (respectively) `is_file`, `is_dir`, and `is_symlink` had some ambiguity in documentation, where it was not noted whether files will pass those tests exclusively or not. It is now written that the tests are mutually exclusive.

Fixes rust-lang#48345.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@retep998 @ExpHP @pietroalbini