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

[Filesystem] Add the readFile() method #54173

Merged
merged 1 commit into from
Mar 17, 2024
Merged

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Mar 6, 2024

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #47215
License MIT

Our Filesystem class is a tool for leveraging PHP's native filesystem functions is a sane way.

For example, it provides a method dumpFile() which attempts to write the contents of a string to a file and throws if that attempt fails. However, it lacks a method to read the content of a file. The native function for this task, file_get_contents() comes with a couple of traps that I don't want to think about or step into when working with a file system:

  • If reading a file fails, file_get_contents() returns false and triggers an E_WARNING. I need to check the return value against false and if I want feedback on the failure, I need to work with custom error handlers. Yuck.
  • Passing the path to a directory to file_get_contents() is almost certainly a mistake that I would want to get an exception for. However, file_get_contents() will return an empty string in that case. Fun times.

With this PR, I'm proposing to add a readFile() method that either returns the content of a given file or throws an IOException. Furthermore, I'm dogfooding this method in components that already depend on the Filesystem component.

@fabpot
Copy link
Member

fabpot commented Mar 17, 2024

Thank you @derrabus.

@fabpot fabpot merged commit d4cab3a into symfony:7.1 Mar 17, 2024
6 of 10 checks passed
@derrabus derrabus deleted the feature/read-file branch March 17, 2024 09:54
@fabpot fabpot mentioned this pull request May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: FileSystem::getContent
6 participants