Skip to content

[CLEANUP] Use null for unset value in DeclarationBlock::parse() #988

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

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

JakeQZ
Copy link
Collaborator

@JakeQZ JakeQZ commented Feb 25, 2025

The variable is either a string or it isn't.
The best practice for when it isn't is for it to be null or unset().

Using false for when it isn't can lead to type-safety issues. Reference:
https://vulke.medium.com/when-should-variables-be-null-false-undefined-or-an-empty-string-in-php-4ebd73c7a954

Also use === in a comparison in the affected code.

Resolves #975.
Part of #976.

The variable is either a string or it isn't.
The best practice for when it isn't is for it to be `null` or `unset()`.

Using `false` for when it isn't can lead to type-safety issues.
Reference:
https://vulke.medium.com/when-should-variables-be-null-false-undefined-or-an-empty-string-in-php-4ebd73c7a954

Also use `===` in a comparison in the affected code.

Resolves #975.
Part of #976.
@coveralls
Copy link

Coverage Status

coverage: 53.68% (-0.03%) from 53.705%
when pulling 85e70b7 on cleanup/declaration-block-parse/null
into afdd72f on main.

@JakeQZ
Copy link
Collaborator Author

JakeQZ commented Feb 25, 2025

It's possible to use === null and = null instead of isset() and unset(), if that's preferred. But given PHP offers these functions, why not use them?

@oliverklee
Copy link
Collaborator

It's possible to use === null and = null instead of isset() and unset(), if that's preferred. But given PHP offers these functions, why not use them?

I slightly prefer setting them to null.

But what I find most important is that we keep things consistent, i.e., use isset and unset if it's about set/not set variables, and use null checks for cases where a variable is set, but might be null. (This is what we're doing here.)

@oliverklee oliverklee merged commit 6ff4aa5 into main Feb 25, 2025
21 checks passed
@oliverklee oliverklee deleted the cleanup/declaration-block-parse/null branch February 25, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a null default value in DeclarationBlock::parse() instead of false
3 participants