Skip to content

Implement new PHP features up to 8.0 #119

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 4 commits into from
Feb 24, 2025

Conversation

TavoNiievez
Copy link
Member

@TavoNiievez TavoNiievez commented Feb 21, 2025

I am not a regular user of Yii, please review critically.

@samdark samdark requested a review from SamMousa February 21, 2025 06:51
@samdark
Copy link
Member

samdark commented Feb 21, 2025

What's the idea of these changes?

@SamMousa
Copy link
Collaborator

What's the idea of these changes?

Stricter typing I imagine, I'll review it.

Copy link
Collaborator

@SamMousa SamMousa left a comment

Choose a reason for hiding this comment

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

In depth review as requested!

Mostly nitpicks except the ones where you change calls to methods where we originally passed null. PHPStan does not agree but the calls are actually correct.

@TavoNiievez TavoNiievez requested a review from SamMousa February 21, 2025 14:26
Comment on lines 848 to 849
'cookie' => $_COOKIE,
'session' => $_SESSION,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sure these global variables are set?

Copy link
Member Author

Choose a reason for hiding this comment

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

if I go for:

'cookie' => $_COOKIE ?? [],

I get this warning:

phpstan: Variable $_COOKIE on left side of ?? always exists and is not nullable

In the case of $_SESSION I don't get this error, it appears that there are cases in which it may be undefined.

so I guess it should look like:

            'cookie' => $_COOKIE,
            'session' => $_SESSION ?? [],

shouldn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

PHPStan does not know

I think it's a bug in phpstan. I'd just ignore the phpstan message tbh.

@TavoNiievez TavoNiievez changed the title Implement new PHP features Implement new PHP features up to 8.0 Feb 21, 2025
Comment on lines 848 to 849
'cookie' => $_COOKIE,
'session' => $_SESSION,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'cookie' => $_COOKIE,
'session' => $_SESSION,
'cookie' => $_COOKIE ?? [],
'session' => $_SESSION ?? [],

@SamMousa SamMousa merged commit 49393a4 into Codeception:master Feb 24, 2025
0 of 5 checks passed
Copy link

🎉 This PR is included in version 2.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

3 participants