Skip to content

Docs: Add documentation for WordPress.PHP.TypeCasts #2591

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

Open
brentwilson-clariio wants to merge 3 commits into
base: develop
Choose a base branch
from
+56 −0
Open

Docs: Add documentation for WordPress.PHP.TypeCasts #2591

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f38caf
Docs: document (double|real) → (float) normalization for WordPress.PH…
brentwilson-clariio Aug 26, 2025
795e5ff
Docs: refine TypeCasts docs per review (double/real→float, forbid (un…
brentwilson-clariio Aug 26, 2025
6bf9ab7
Docs: reorder TypeCasts code examples to Valid (left) / Invalid (righ…
brentwilson-clariio Aug 26, 2025
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
56 changes: 56 additions & 0 deletions WordPress/Docs/PHP/TypeCastsStandard.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?xml version="1.0"?>
<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
title="Type Casts"
>
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

Suggested change
>
>

<standard>
<![CDATA[
Enforce normalized and safe type casts.
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

I'm unsure about a few things in this paragraph:

  • Mentioning that the casts that are not recommended are not safe. Are they all really not safe? I can see why using the casts that were removed is not safe, but not all casts discouraged by the sniff were removed.
  • Maybe we could explicitly mention the legacy float casts?
  • I don't think it is necessary to mention in parenthesis that (binary) is discouraged. If we want to mention that maybe we can include it in the main sentence?

Here is a suggestion of a possible alternative for description. Feel free to create your own version.

Type casts must use normalized keywords.

    • Use (float) not (double) or (real).
    • Do not use the (unset) cast. Use unset().
    • Do not use the (binary) cast or b"string" prefix. Use (string) casts or regular string literals instead.


• Use (float) instead of legacy float casts.
• Do not use the (unset) cast. Use unset().
• Avoid the (binary) cast (discouraged).
Comment on lines +8 to +12
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

Suggested change
Enforce normalized and safe type casts.
• Use (float) instead of legacy float casts.
• Do not use the (unset) cast. Use unset().
• Avoid the (binary) cast (discouraged).
Enforce normalized and safe type casts.
• Use (float) instead of legacy float casts.
• Do not use the (unset) cast. Use unset().
• Avoid the (binary) cast (discouraged).

]]>
</standard>

<code_comparison>
Copy link
Member

@jrfnl jrfnl Aug 26, 2025

Choose a reason for hiding this comment

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

The code comparison should have "valid" on the left (first code sample), "invalid" on the right (second code sample).

Copy link
Author

@brentwilson-clariio brentwilson-clariio Aug 26, 2025

Choose a reason for hiding this comment

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

Thanks for the feedback! I’ve reordered the code examples so that Valid is on the left and Invalid is on the right, as suggested.

<code title="Valid: Using normalized float casts.">
<![CDATA[
$price = <em>(float)</em> $value;
$size = <em>(float)</em> $value;
]]>
</code>
<code title="Invalid: Using legacy float casts.">
<![CDATA[
$price = <em>(double)</em> $value;
$size = <em>(real)</em> $value;
]]>
</code>
</code_comparison>

<code_comparison>
<code title="Valid: Use unset().">
<![CDATA[
unset( $value );
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

Suggested change
unset( $value );
<em>unset</em>( $value );

]]>
</code>
<code title="Invalid: Using the (unset) cast.">
<![CDATA[
result = <em>(unset)</em> $value;
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

Suggested change
result = <em>(unset)</em> $value;
$result = <em>(unset)</em> $value;

bytes below is also missing a $.

]]>
</code>
</code_comparison>

<code_comparison>
<code title="Valid: Prefer a clear alternative.">
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

Maybe "Valid: Using (string) cast instead."? This way the title clearly communicates what is the alternative like the other titles.

That being said, I noticed that the sniff also detects the binary string prefix notation b"string" which is also tokenized as T_BINARY_CAST, but in this case using (string) is not the correct alternative. I believe we should explicitly mention in the documentation that b"string" also trigger the sniff both here with an example and in the standard description above.

In that case, the title could be something like "Valid: (binary) is not used." or "Valid: Use (string) or regular strings."

Another option is to create a separate <code_comparison> block for the binary string. Tipically, sniff documentation uses a single <code_comparison> block per warning/error message that is triggered by the sniff. But in this case it might make sense to have a separate block.

What do you think?

<![CDATA[
bytes = (string) $value;
Copy link
Collaborator

@rodrigoprimo rodrigoprimo Oct 24, 2025

Choose a reason for hiding this comment

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

Suggested change
bytes = (string) $value;
bytes = <em>(string)</em> $value;

]]>
</code>
<code title="Invalid: Using the (binary) cast.">
<![CDATA[
bytes = <em>(binary)</em> $value;
]]>
</code>
</code_comparison>
</documentation>
Loading