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

Improve boolean handling in PropertyValidator #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

szepeviktor
Copy link
Contributor

Just that.

@PrinsFrank
Copy link
Owner

Hi @szepeviktor, thanks for all the great PRs! I don't know if I agree with this one though, can you explain why this is an improvement? IMHO a strict check is always better than a boolean NOT operator, because when the value being negated is not a boolean but a string for example, type conversion happens. for example:

$foo = 'Foo bar';

If we now check if foo is false, this will be false:

if ($foo === false) { // Will not execute }

But a negating operator will result in the code being executed:

if (! $foo) { // Will  execute! }

Ideally, we wont do boolean checks on strings, and this might be enforced by a static analysis tool like PHPStan, but currently in this project this is not yet the case. That's why I prefer to be defensive in this case.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jul 10, 2022

when the value being negated is not a boolean but a string

Yes. You are right. If your software is a trash dump you have to prepare for the worst.
But php-validated-properties is strictly typed and uses PHPStan.

In this specific case instanceof and isValid always return a boolean.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Jul 10, 2022

AFAIK PHPStan on a higher level or phpstan-strict-rules package will warn you if you use a boolean operator - e.g ! - in front of a non-boolean value.

@szepeviktor
Copy link
Contributor Author

But a negating operator will result in the code being executed

FYI :)

$ php8.0 -r '$foo = "Foo bar"; var_dump(! $foo);'
bool(false)

@szepeviktor
Copy link
Contributor Author

Type juggling and type casting belong to the bad parts of PHP (or any other language).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants