Add Core and Extra sniffs #382
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| <rule ref="Generic.PHP.ForbiddenFunctions"/> | ||
| <rule ref="Generic.Functions.CallTimePassByReference"/> | ||
| <rule ref="Generic.Formatting.DisallowMultipleStatements"/> | ||
| <rule ref="Generic.Formatting.MultipleStatementAlignment"/> |
There was a problem hiding this comment.
I really don't think we should include MultipleStatementAlignment. It's really annoying, for instance:
There was a problem hiding this comment.
I removed it previously: 9f6854d
There was a problem hiding this comment.
I agree, MultipleStatementAlignment is a good practice within reason, but the sniff isn't refined enough.
There was a problem hiding this comment.
In this case, I'd optionally put $redirect_vars_pairs above line 129, and align the = out.
In other cases, just adding a clear line above 134 would do - that the separates the predefined data in $redirect_vars, and the variable that's only populated from the foreach.
It is part of the standard.
However, I'm open to removing it if you both still insist.
There was a problem hiding this comment.
It is part of the standard.
Kinda. It says “spaces can be used mid-line for alignment”. It doesn't say that that alignment is required.
There was a problem hiding this comment.
We should probably have in the README a list of other sniffs that users may like to opt-in to, such as this, but which are perhaps even more opinionated than Extra already is.
Left in and commented out with a note so that future us will know why it's been excluded.
|
Updated. |
JDGrimes
added a commit
that referenced
this pull request
May 16, 2015
…sniffs Add Core and Extra sniffs
jrfnl
added a commit
that referenced
this pull request
Jan 28, 2017
The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment) To this end, I've send in a PR upstream to address these concerns, which has subsequently been merged. The adjusted version of the sniff will be released in PHPCS 2.7.2.
jrfnl
added a commit
that referenced
this pull request
Feb 1, 2017
The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment) To this end, I've send in a PR upstream to address these concerns, which has subsequently been merged. The adjusted version of the sniff will be released in PHPCS 2.7.2.
jrfnl
added a commit
that referenced
this pull request
Feb 25, 2017
The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment) To this end, I've send in a PR upstream to address these concerns, which has subsequently been merged. The adjusted version of the sniff will be released in PHPCS 2.7.2.
jrfnl
added a commit
that referenced
this pull request
Feb 25, 2017
The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment) To this end, I've send in a PR upstream to address these concerns, which has subsequently been merged. The adjusted version of the sniff will be released in PHPCS 2.7.2.
Closed
3 tasks
jrfnl
added a commit
that referenced
this pull request
Oct 27, 2022
… sniff. The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment), also see 522b35f To this end, I've send in a [PR upstream](squizlabs/PHP_CodeSniffer#1318) to address these concerns, which has subsequently been merged. The adjusted version of the sniff was released in PHPCS 3.4.0, which means the sniff can now be added. Note: the sniff has since then also been adjusted to handle arrow functions correctly, as well as constructor property promotion. Fixes 1510
jrfnl
added a commit
that referenced
this pull request
Oct 27, 2022
… sniff The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment), also see 522b35f To this end, I've send in a [PR upstream](squizlabs/PHP_CodeSniffer#1318) to address these concerns, which has subsequently been merged. The adjusted version of the sniff was released in PHPCS 3.4.0, which means the sniff can now be added. Note: the sniff has since then also been adjusted to handle arrow functions correctly, as well as constructor property promotion. Fixes 1510
jrfnl
added a commit
that referenced
this pull request
Oct 27, 2022
… sniff The reason why the sniff was disabled up to now was that WP uses callbacks a lot and the hooked in functions may not use all parameters passed to them. This concern was previously raised in #382 (comment), also see 522b35f To this end, I've send in a [PR upstream](squizlabs/PHP_CodeSniffer#1318) to address these concerns, which has subsequently been merged. The adjusted version of the sniff was released in PHPCS 3.4.0, which means the sniff can now be added. Note: the sniff has since then also been adjusted to handle arrow functions correctly, as well as constructor property promotion. Fixes 1510
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a few Generic sniffs for Core, and a whole bunch more for Extra. See the diff for which sniffs.