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

Add documentation for WordPress.PHP.DevelopmentFunctions #2490

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

Add documentation for WordPress.PHP.DevelopmentFunctions #2490

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c746bf3
Add documentation for WordPress.PHP.DevelopmentFunctions
gogdzl Sep 17, 2024
28f5a16
Add em tag
gogdzl Sep 17, 2024
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
44 changes: 44 additions & 0 deletions WordPress/Docs/PHP/DevelopmentFunctionsStandard.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?xml version="1.0"?>
<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
title="Development Functions"
>
<standard>
<![CDATA[
Debug code should not normally be used in production.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that this is the warning message displayed by the sniff, but I believe that the documentation (and maybe also the sniff messages) usually only uses should/should not when describing an error. Since this is a warning, maybe we could say that it is recommended or something like that?


Typically, this rule verifies if function calls to the PHP native `error_log()`, `var_dump()`, `var_export()`, `print_r()`, `trigger_error()`, `set_error_handler()`, `debug_backtrace`, `debug_print_backtrace` and `wp_debug_backtrace_summary()` functions are present in the code.
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
Typically, this rule verifies if function calls to the PHP native `error_log()`, `var_dump()`, `var_export()`, `print_r()`, `trigger_error()`, `set_error_handler()`, `debug_backtrace`, `debug_print_backtrace` and `wp_debug_backtrace_summary()` functions are present in the code.
This rule verifies if calls to the `error_log()`, `var_dump()`, `var_export()`, `print_r()`, `trigger_error()`, `set_error_handler()`, `debug_backtrace()`, `debug_print_backtrace()` and `wp_debug_backtrace_summary()` functions are present in the code.

I'm suggesting the removal of the PHP native part as wp_debug_backtrace_summary() is not a PHP native function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe instead of verifies, we could use warns to more accurately describe what the rule does? Just thinking out loud here.

]]>
</standard>
<code_comparison>
<code title="Valid: (Not) using var_dump().">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not is in between parenthesis?

Also, I wonder if the message should be more generic and mention debug code/functions instead of singling out var_dump()? I'm ok with a single example with just var_dump().

<![CDATA[
// var_dump() should not be used.
]]>
</code>
<code title="Invalid: Calling the PHP native `var_dump()` function.">
<![CDATA[
<em>var_dump( $bar );</em>
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
Please refrain from using functions that can lead to full path disclosure.

Typically, this rule verifies if function calls to the PHP native `error_reporting()` and `phpinfo()` functions are present in the code.
]]>
</standard>
<code_comparison>
<code title="Valid: (Not) using `phpinfo()`.">
<![CDATA[
// php_info() should not be used.
]]>
</code>
<code title="Invalid: Calling the PHP native `phpinfo()` function.">
<![CDATA[
<em>phpinfo();</em>
]]>
</code>
</code_comparison>
</documentation>
Loading