-
Notifications
You must be signed in to change notification settings - Fork 178
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
Increment decrement operators #112
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.
I think the "spacing" part should be moved to the "Space usage" section. (including the code samples)
For the recommended use of pre vs post, I do think having a separate subsection is the right way to go and the subsection is in the right place.
db21f68
to
60ba742
Compare
Split the parts that refered to spacing to the spacing section and code examples. Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
b6a4283
to
8637060
Compare
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.
LGTM
Merging this now as @GaryJones is AFK for the next few weeks. If needs be, additional textual tweaks can still be made in a follow up commit.
// Correct: Pre-decrement. | ||
--$a; | ||
|
||
// Incorrect: Post-decrement. |
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.
Rather than "Correct" and "Incorrect", I might have gone with "Recommended" and "Discouraged" since the overall rule seems less strict ("favoured").
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.
Everywhere else we are using the Correct/Incorrect
syntax, and I think that Juliette mentioned this will get reported as incorrect usage in WPCS 3.0 (if I recall correctly).
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.
Yes, there is a sniff which will start flagging this, but as the original text used "favoured" (strong recommendation), in my WIP branch, I've got that sniff throwing a warning
for this code, not an error
.
In that sense, we could choose to (consistently) used "Correct/Incorrect" for everything that will be throwing errors and "Recommended/Discouraged" for everything that will be throwing warnings ?
The Core
ruleset is mostly errors, but there are some things set to warning. We could cross-check the "final" settings in WPCS with the docs before publishing WPCS 3.0.0 ?
@@ -600,6 +611,21 @@ While this operator does exist in Core, it is often used lazily instead of doing | |||
|
|||
> Warning: Prior to PHP 8.0.0, it was possible for the @ operator to disable critical errors that will terminate script execution. For example, prepending @ to a call of a function that did not exist, by being unavailable or mistyped, would cause the script to terminate with no indication as to why. | |||
|
|||
### Increment/decrement operators | |||
|
|||
Pre-increment/decrement should be favoured over post-increment/decrement for stand-alone statements. |
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.
Instead of "favoured", I would have gone with "preferred", since that avoids the en_US / en_GB (and others) difference in the spelling of "favo[u]red".
This PR depends on #111. Once that is merged, this PR should be rebased to the master branch.
The PR adds rules about the increment/decrement operators and is the continuation of the additions of 'modern' PHP code in the WordPress PHP Coding Standards handbook based on the make post by Juliette Reinders Folmer.