-
-
Notifications
You must be signed in to change notification settings - Fork 488
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
base: develop
Are you sure you want to change the base?
Add documentation for WordPress.PHP.RestrictedPHPFunctions #2491
Conversation
add_action( 'init', function() { | ||
return foo( 'bar' ); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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() { |
There was a problem hiding this comment.
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)."> |
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
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?
Related to #1722