Separate file/directory/exists assertions#291
Merged
shadowhand merged 1 commit intowebmozarts:masterfrom Oct 16, 2024
shadowhand:patch-1
Merged
Separate file/directory/exists assertions#291shadowhand merged 1 commit intowebmozarts:masterfrom shadowhand:patch-1
shadowhand merged 1 commit intowebmozarts:masterfrom
shadowhand:patch-1
Conversation
shadowhand
commented
Sep 21, 2023
| if (!\file_exists($value)) { | ||
| static::reportInvalidArgument(\sprintf( | ||
| $message ?: 'The file %s does not exist.', | ||
| $message ?: 'The path %s does not exist.', |
Collaborator
Author
There was a problem hiding this comment.
This is technically incorrect, since file_exists is equivalent to:
return is_file($path) || is_dir($path);| */ | ||
| public static function file($value, $message = '') | ||
| { | ||
| static::fileExists($value, $message); |
Collaborator
Author
There was a problem hiding this comment.
This is a pointless check.
| */ | ||
| public static function directory($value, $message = '') | ||
| { | ||
| static::fileExists($value, $message); |
Collaborator
Author
There was a problem hiding this comment.
This is a pointless check. If the directory does not exist, the next assertion will never run. If the directory does exist, then it duplicates work.
| if (!\is_dir($value)) { | ||
| static::reportInvalidArgument(\sprintf( | ||
| $message ?: 'The path %s is no directory.', | ||
| $message ?: 'The path %s is not a directory.', |
Collaborator
Author
There was a problem hiding this comment.
Made this message consistent with Assert::file().
| */ | ||
| public static function fileExists($value, $message = '') | ||
| { | ||
| static::string($value); |
Collaborator
Author
There was a problem hiding this comment.
I don't see other assertions doing this, so I removed it.
There is no need to use `file_exists` before using `is_file` and `is_dir`, this makes for confusing errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is no need to use
file_existsbefore usingis_fileandis_dir, this makes for confusing errors. For instance, if you callAssert::directory('invalid/path')then the reported error will be:Additionally, the messages for these assertions were either technically or grammatically incorrect.