-
-
Notifications
You must be signed in to change notification settings - Fork 518
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.
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
base: develop
Are you sure you want to change the base?
Docs: Add documentation for WordPress.PHP.TypeCasts #2591
Conversation
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.
@brentwilson-clariio Thanks for picking this up! I've just had a quick look and left some feedback for you to consider.
Note: Other type-cast rules in the WordPress standard | ||
are enforced by different sniffs: | ||
- Short forms like (int), (bool), (string): | ||
PSR12.Keywords.ShortFormTypeKeywords | ||
- No spaces inside parentheses: | ||
Squiz.WhiteSpace.CastSpacing | ||
- Exactly one space after the cast: | ||
Generic.Formatting.SpaceAfterCast |
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.
This whole block doesn't belong here. Each sniff functions independently of other sniffs. That also means that the docs should be independent of each other and should just focus on what this sniff does.
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! I’ve removed the paragraph referencing other sniffs. The docs now only describe what WordPress.PHP.TypeCasts enforces.
<code_comparison> | ||
<code title="Invalid: legacy float casts"> | ||
<![CDATA[ | ||
<?php |
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.
PHP open tag is not needed here. (Same for the "invalid" code sample)
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.
Removed <?php from all code samples as suggested.
<documentation title="Type Casts"> | ||
<standard> | ||
<![CDATA[ | ||
This sniff normalizes float type cast keywords. |
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.
This sentence seems redundant.
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.
Reworded the opener to be shorter and clearer. It now states the rules enforced without repeating itself.
@@ -0,0 +1,38 @@ | |||
<?xml version="1.0"?> | |||
<documentation title="Type Casts"> |
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.
<documentation title="Type Casts"> | |
<?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" | |
> |
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.
Added the schema header and xsi:noNamespaceSchemaLocation pointing to phpcsdocs.xsd.
Use (float) instead of legacy synonyms: | ||
- Disallowed: (double), (real) | ||
- Allowed: (float) |
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.
This is incorrect/incomplete.
The sniff:
- Forbids the use of non-standard casts for floats.
- Forbids the use of the
(unset)
cast. - Discourages the use of the
(binary)
cast.
Considering the above, I believe the code samples also need work.
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.
Updated the standard text and the code examples to cover all three behaviors:
• (double)/(real) → error (normalize to (float))
• (unset) → error (removed in PHP 8.0; suggest unset())
• (binary) → warning (discouraged)
</standard> | ||
|
||
<code_comparison> | ||
<code title="Invalid: legacy float casts"> |
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.
<code title="Invalid: legacy float casts"> | |
<code title="Invalid: Using legacy float casts."> |
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.
Adjusted the code block titles to your suggested wording.
$size = <em>(real)</em> $value; | ||
]]> | ||
</code> | ||
<code title="Valid: normalized float cast"> |
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.
<code title="Valid: normalized float cast"> | |
<code title="Valid: Using normalized float casts."> |
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.
Adjusted the code block titles to your suggested wording.
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.
Updated code block titles to “Valid: Using normalized float casts.” (and similar phrasing for the other examples), as suggested.
]]> | ||
</standard> | ||
|
||
<code_comparison> |
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 code comparison should have "valid" on the left (first code sample), "invalid" on the right (second code sample).
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 the feedback! I’ve reordered the code examples so that Valid is on the left and Invalid is on the right, as suggested.
…set), discourage (binary))
Related to #1722
This PR adds a new documentation file:
WordPress/Docs/PHP/TypeCastsStandard.xml
What it covers
Documents the rule enforced by the
WordPress.PHP.TypeCasts
sniff:(double)
,(real)
) to(float)
.Notes
Other type-cast related rules in the WordPress standard are enforced by separate sniffs:
(int)
,(bool)
,(string)
): PSR12.Keywords.ShortFormTypeKeywords