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
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions WordPress/Docs/PHP/RestrictedPHPFunctionsStandard.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0"?>
<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
title="Restricted PHP Functions"
>
<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.


create_function() is deprecated as of PHP 7.2 and removed in PHP 8.0. Please use declared named or anonymous functions instead.
]]>
</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.

<![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"?

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?

return foo( 'bar' );
} );
Comment on lines +16 to +18
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.

]]>
</code>
<code title="Invalid: Using create_function().">
<![CDATA[
add_action( 'init', <em>create_function( '',
'return foo( "bar" );'
)</em> );
]]>
</code>
</code_comparison>
</documentation>
Loading