Bug/#5110 fix php81 warnings

[kuasha420]

Summary

Addresses issue:

• #5110

Relevant technical choices

The remaining strpos and str_replace errors are related to how we use add_submenu_page and will be addressed in #5998.

The remaining rtrim errors are caused by load_script_textdomain function and we are consuming the function correctly in our code. It's something that will probably be fixed in Core.

PR Author Checklist

• [ ] My code is tested and passes existing unit tests.
• [ ] My code has an appropriate set of unit tests which all pass.
• [ ] My code is backward-compatible with WordPress 4.7 and PHP 5.6.
• [ ] My code follows the WordPress coding standards.
• [ ] My code has proper inline documentation.
• [ ] I have added a QA Brief on the issue linked above.
• [x] I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

• [ ] Run the code.
• [ ] Ensure the acceptance criteria are satisfied.
• [ ] Reassess the implementation with the IB.
• [ ] Ensure no unrelated changes are included.
• [ ] Ensure CI checks pass.
• [ ] Check Storybook where applicable.
• [ ] Ensure there is a QA Brief.

Merge Reviewer Checklist

• [ ] Ensure the PR has the correct target branch.
• [ ] Double-check that the PR is okay to be merged.
• [ ] Ensure the corresponding issue has a ZenHub release assigned.
• [ ] Add a changelog message to the issue.

[kuasha420]

Thank you for the review, @asvinb !

The errors you mention, I am not seeing them in InstaWP. What's your php 8.1 version. Are those development only error?

I have addressed your other comments. Cheers.

[eugene-manuilov]

Since we are supporting PHP 5, I think maybe we can add the #[ReturnTypeWillChange] attribute: https://php.watch/versions/8.1/ReturnTypeWillChange Curious to hear your thoughts @eugene-manuilov

Good question, @asvinb. I lean toward using the ReturnTypeWillChange attribute although I would also like to hear thoughts from @aaemnnosttv.

[asvinb]

@kuasha420 I'am on PHP 8.1.9 where i can see those errors via QueryMonitor.

[kuasha420]

@asvinb Thank you for giving me your PHP version.

However, I am not seeing the same errors in my local or an InstaWP site when using PHP 8.1 and rebased to latest develop. I am seeing different errors which are similar and coming from guzzlehttp package.

Other than that, I've also tried to add the attribute to the methods based off of your screenshot, but not sure if that's the correct approach and the attribute comment also conflicts with phpcs rules. I have not spent much time on it but looks like we need to upgrade our php codensiffer to properly use attributes without adding bunch of phpcs ignores around it.

        // phpcs:disable Squiz.Commenting.InlineComment.WrongStyle,Squiz.Commenting.FunctionComment.WrongStyle, Squiz.PHP.CommentedOutCode.Found
	#[\ReturnTypeWillChange]
	public function offsetUnset( $key ) {

Not sure how to proceed here. Cheers.

cc @eugene-manuilov @aaemnnosttv

[asvinb]

@kuasha420 I checked there are no no longer warnings from our code. I'll let @eugene-manuilov @aaemnnosttv check the ReturnTypeWillChange attribute

[eugene-manuilov]

Other than that, I've also tried to add the attribute to the methods based off of your screenshot, but not sure if that's the correct approach and the attribute comment also conflicts with phpcs rules. I have not spent much time on it but looks like we need to upgrade our php codensiffer to properly use attributes without adding bunch of phpcs ignores around it.

Yes, i think we can allow it in phpcs config file, but i am not sure how to do it. @aaemnnosttv do you know if it is feasible?

[aaemnnosttv]

@asvinb @eugene-manuilov Can we add the ignore inline with the attribute rather than around it?

[kuasha420]

@aaemnnosttv I have updated the phpcs disables with ignores. Feels quite a bit repitetive but that's the best we can do for now. We should explore proper solution about using PHP attributes with phpcs in a follow up issue that I've created here. Assigning to you for a merge review. Cheers.