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

Fix: strnatcasecmp(): Passing null to parameter #1 ($string1) of type string is deprecated #21413

Merged
merged 3 commits into from Jan 4, 2024

Conversation

blankse
Copy link
Contributor

@blankse blankse commented Oct 18, 2023

Description:

Since php 8.1 it is deprecated to pass null to the strnatcasecmp() method. The category of the reports can be null.

Review

@sgiehl
Copy link
Member

sgiehl commented Oct 30, 2023

@blankse Thanks for creating this PR. Would you mind explaining where the error occurred? Also would you maybe mind to recreate this PR against 5.x-dev.
We won't merge any fixes for 4.x-dev unless it has been fixed for 5.x-dev already, to avoid any fixes from not being included in the next major release. Thanks

@blankse blankse changed the base branch from 4.x-dev to 5.x-dev October 30, 2023 14:43
@blankse
Copy link
Contributor Author

blankse commented Oct 30, 2023

@sgiehl I rebased to 5.x-dev

The deprecation warning is skipped by the ErrorHandler. You need to remove this lines to see it:

// if the error has been suppressed by the @ we don't handle the error
if (!(error_reporting() & $errno)) {
return;
}

Or create a Breakpoint in a xdebug session.

I think this should be used only for the @ sign. Here there isn't a @ sign.

I think the @ sign is a bad practice. Often the warnings for not setting array keys can be avoided by ?? null. I can create a additional PR if you want.

Copy link
Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Nov 15, 2023
@sgiehl sgiehl added Needs Review PRs that need a code review and removed Stale The label used by the Close Stale Issues action labels Nov 15, 2023
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Nov 23, 2023
Copy link
Contributor

github-actions bot commented Jan 4, 2024

This PR was last updated more than one month ago, maybe it's time to close it. Please check if there is anything we still can do or close this PR. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale for long The label used by the Close Stale Issues action label Jan 4, 2024
@sgiehl sgiehl merged commit 558354f into matomo-org:5.x-dev Jan 4, 2024
20 of 25 checks passed
@sgiehl sgiehl added this to the 5.0.1 milestone Jan 4, 2024
@sgiehl sgiehl added Bug For errors / faults / flaws / inconsistencies etc. and removed Needs Review PRs that need a code review Stale The label used by the Close Stale Issues action Stale for long The label used by the Close Stale Issues action labels Jan 4, 2024
@blankse blankse deleted the fix_deprecation branch January 4, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants