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.

Sign up for GitHub

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

Jump to bottom

Restrict parse_str() without second argument #756

Open
ernilambar opened this issue Oct 28, 2024 · 4 comments · May be fixed by #787
Open

Restrict parse_str() without second argument #756

ernilambar opened this issue Oct 28, 2024 · 4 comments · May be fixed by #787
Labels
[Team] Plugin Review Issues owned by Plugin Review Team [Type] Check Proposal A new check proposal

Comments

@ernilambar
Copy link
Member

parse_str() without second argument is restricted.

Using this function without the result parameter is highly DISCOURAGED and DEPRECATED as of PHP 7.2. As of PHP 8.0.0, the result parameter is mandatory.

Developing sniff using AbstractFunctionParameterSniff would be most feasible route I believe.
@ernilambar ernilambar added [Team] Plugin Review Issues owned by Plugin Review Team [Type] Check Proposal A new check proposal labels Oct 28, 2024
@swissspidy
Copy link
Member

The PHPCompatibility project has a dedicated PHPCompatibility.FunctionUse.OptionalToRequiredFunctionParameters sniff that catches exactly this.

No need to reinvent the wheel.

I would suggest addingPHPCompatibility as a whole to plugin-review.xml, as it helps plugins keep up to date with PHP compatibility

@ernilambar
Copy link
Member Author

Do you mean adding PHPCompatibility.FunctionUse.OptionalToRequiredFunctionParameters or whole PHPCompatibility? I will discuss it in the team meeting accordingly.

Regarding parse_str() particularly, currently it is enforced in internal scanner for any PHP version. Would it be possible to customize OptionalToRequiredFunctionParametersSniff class?

@swissspidy
Copy link
Member

I don‘t know, you will have to check

@ernilambar ernilambar mentioned this issue Oct 31, 2024
Closed
@chriscct7
Copy link
Member

It seems like it'd be better to copy the one test as opposed to including the entire library since we'd have to do that as a non-stable (dev branch)

@ernilambar ernilambar linked a pull request Nov 12, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Team] Plugin Review Issues owned by Plugin Review Team [Type] Check Proposal A new check proposal
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add sniff for required function parameter WordPress/plugin-check
3 participants
@swissspidy @chriscct7 @ernilambar