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

Add documentation for WordPress.PHP.RestrictedPHPFunctions #2491

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

gogdzl
Copy link

@gogdzl gogdzl commented Sep 17, 2024

Related to #1722

@jrfnl jrfnl mentioned this pull request Sep 17, 2024
61 tasks
Comment on lines +16 to +18
add_action( 'init', function() {
return foo( 'bar' );
} );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
add_action( 'init', function() {
return foo( 'bar' );
} );
add_action( 'init', <em>function() {
return foo( 'bar' );
}</em> );

We can highlight the good bit of this snippet to focus on.

Copy link
Collaborator

@rodrigoprimo rodrigoprimo left a comment

Choose a reason for hiding this comment

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

Thanks for another PR adding user documentation for a sniff, @gogdzl! I left some comments with suggestions for us to discuss.

>
<standard>
<![CDATA[
The restricted function create_function() should not be used.
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
The restricted function create_function() should not be used.
The function create_function() should not be used.

<code_comparison>
<code title="Valid: Using anonymous function (closure).">
<![CDATA[
add_action( 'init', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it is a good idea to include an example with a declared named function as well (and then also update the title attribute)? Maybe it is just me, but it took me a second to properly read the sentence Please use declared named or anonymous functions instead. and to realize that functions applies to both declared named and anonymous. My first thought was "what is a declared named"?

]]>
</standard>
<code_comparison>
<code title="Valid: Using anonymous function (closure).">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not against mentioning here that anonymous functions are also known as closure. But if it is important to provide this information, I believe it should also be present in the description of the <standard> block.

<code_comparison>
<code title="Valid: Using anonymous function (closure).">
<![CDATA[
add_action( 'init', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm unsure if including a call to add_action() here helps understand what the sniff does or if it just adds noise. Usually, the examples are minimalistic, and just calling calling create_function() or calling it and assigning the returned value to a variable is enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants