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.

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

Add documentation to point to File::open or OpenOptions::open instead of is_file to check read/write possibility #73243

Merged
merged 5 commits into from
Jun 28, 2020

Conversation

poliorcetics
Copy link
Contributor

Fixes #64170.

This adds documentation to point user towards !is_dir instead of is_file when all they want to is read from a source.

I ran rg "fn is_file\(" to find all is_file methods, I hope I did not miss one.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2020
@poliorcetics
Copy link
Contributor Author

@rustbot modify labels: T-doc, T-libs, C-enhancement

@rustbot rustbot 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 T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 11, 2020
@Dylan-DPC-zz
Copy link

r? @Amanieu

@rust-highfive rust-highfive assigned Amanieu and unassigned cramertj Jun 23, 2020
Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting, grammar, etc, of this all look good to me. I'll let libs decide if they want to add this or not.

@Amanieu
Copy link
Member

Amanieu commented Jun 27, 2020

You are forgetting about unix sockets which are another type of file that you can't just read/write bytes to.

The proper way of checking whether you can read/write bytes to a file is to try opening it. This will fail for directories, symlinks (if open is called with O_NOFOLLOW) and sockets.

@poliorcetics
Copy link
Contributor Author

@Amanieu good catch ! I changed the text to point to File::open and OpenOptions::open instead, is it better ?

@poliorcetics poliorcetics changed the title Add documentation to point to !is_dir instead of is_file for Add documentation to point to File::open or OpenOptions::open instead of is_file to check read/write possibility Jun 27, 2020
src/libstd/path.rs Outdated Show resolved Hide resolved
@Amanieu
Copy link
Member

Amanieu commented Jun 27, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 27, 2020

📌 Commit 8e8c54a has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 27, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 27, 2020
…Amanieu

Add documentation to point to `File::open` or `OpenOptions::open` instead of `is_file` to check read/write possibility

Fixes rust-lang#64170.

This adds documentation to point user towards `!is_dir` instead of `is_file` when all they want to is read from a source.

I ran `rg "fn is_file\("` to find all `is_file` methods, I hope I did not miss one.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 28, 2020
…Amanieu

Add documentation to point to `File::open` or `OpenOptions::open` instead of `is_file` to check read/write possibility

Fixes rust-lang#64170.

This adds documentation to point user towards `!is_dir` instead of `is_file` when all they want to is read from a source.

I ran `rg "fn is_file\("` to find all `is_file` methods, I hope I did not miss one.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 28, 2020
…Amanieu

Add documentation to point to `File::open` or `OpenOptions::open` instead of `is_file` to check read/write possibility

Fixes rust-lang#64170.

This adds documentation to point user towards `!is_dir` instead of `is_file` when all they want to is read from a source.

I ran `rg "fn is_file\("` to find all `is_file` methods, I hope I did not miss one.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2020
…arth

Rollup of 10 pull requests

Successful merges:

 - rust-lang#72796 (MIR sanity check: validate types on assignment)
 - rust-lang#73243 (Add documentation to point to `File::open` or `OpenOptions::open` instead of `is_file` to check read/write possibility)
 - rust-lang#73525 (Prepare for LLVM 11)
 - rust-lang#73672 (Adds a clearer message for when the async keyword is missing from a f…)
 - rust-lang#73708 (Explain move errors that occur due to method calls involving `self` (take two))
 - rust-lang#73758 (improper_ctypes: fix remaining `Reveal:All`)
 - rust-lang#73763 (errors: use `-Z terminal-width` in JSON emitter)
 - rust-lang#73796 (replace more `DefId`s with `LocalDefId`)
 - rust-lang#73797 (fix typo in self-profile.md)
 - rust-lang#73809 (Add links to fs::DirEntry::metadata)

Failed merges:

r? @ghost
@bors bors merged commit 6a944c1 into rust-lang:master Jun 28, 2020
@poliorcetics poliorcetics deleted the discourage-is-file branch June 28, 2020 14:30
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File and Metadata should document reasons to prefer *not* to use is_file()
9 participants