-
-
Notifications
You must be signed in to change notification settings - Fork 89
-
Hi, I'd like to include a rule to add newlines around control structures like so:
$customFieldsDeleted = [];
foreach ($deleteCustomFields as $customFieldToDelete) {
$customFieldIdToDelete = $customFieldToDelete['id'];
if (empty($customFieldIdToDelete)) {
continue;
}
$result = $this->client->deleteCustomField($productId, $customFieldIdToDelete);
if ($result) {
array_push($customFieldsDeleted, $customFieldIdToDelete);
}
}
return $customFieldsDeleted;
becomes this:
$customFieldsDeleted = [];
foreach ($deleteCustomFields as $customFieldToDelete) {
$customFieldIdToDelete = $customFieldToDelete['id'];
if (empty($customFieldIdToDelete)) {
continue;
}
$result = $this->client->deleteCustomField($productId, $customFieldIdToDelete);
if ($result) {
array_push($customFieldsDeleted, $customFieldIdToDelete);
}
}
return $customFieldsDeleted;
Essentially, adding a newline both before and after a control structure (and ensuring it's only one line total in the case of back-to-back control structures).
First, I can't seem to find a rule that works for this. When I include <rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/>, it almost gets me there, because it seems to be adding newlines after the control structure, but not before:
$customFieldsDeleted = [];
foreach ($deleteCustomFields as $customFieldToDelete) {
$customFieldIdToDelete = $customFieldToDelete['id'];
if (empty($customFieldIdToDelete)) {
continue;
}
$result = $this->client->deleteCustomField($productId, $customFieldIdToDelete);
if ($result) {
array_push($customFieldsDeleted, $customFieldIdToDelete);
}
}
return $customFieldsDeleted;
Even more confusing, because we want to lint against the PSR-12 standard, when we add <rule ref="PSR12"/>, it seems to overwrite the above rule and phpcbf will not add any spacing around the control structures when I run it. When I dive into the PSR12 ruleset here, and pulled all the rules into my own phpcs.xml and started removing them one-by-one until the spacing worked to try to figure out potential conflicts, it seemed like <rule ref="Squiz.WhiteSpace.ControlStructureSpacing.SpacingAfterOpen"/> was the culprit, because when that is in the config along with <rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/>, the spacing is not added, but when I remove the former rule, the latter rule finally works.
I tried excluding this from the PSR-12 config block in our file and I also tried
<rule ref="PSR12">
<exclude name="PSR12.WhiteSpace.ControlStructureSpacing"/>
</rule>
But none of this seems to work.
Any guidance on how to get this spacing rule working?
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 2 comments
-
Hey @jrfnl -- thank you for the incredible amount of hard work on this project!
I wanted to follow up on this because I spent a good chunk of the weekend trying to get this to work but just keep hitting walls.
1. Bug Behavior
At the simplest level, the following two rules simply do not work together:
<rule ref="PSR12"/>
<rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/>
Including PSR12 always disables that Squiz rule. I cannot figure out why. There do not seem to be any conflicts when I read through the rulesets.
<rule ref="PSR12">
<exclude name="PSR12.ControlStructures.ControlStructureSpacing"/>
</rule>
<rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/>
Does not work either. I tried many different exclusion rules, none worked.
This definitely feels like it is not intended?
2. Guidance on Before/After Spacing
If I could just get the above combo working in Section 1, I'd be a happy camper and can settle for that. Additionally though I'm having a hard time believing I can't find a standard ruleset for this (spacing both after and before)? I actually spent an hour or two trying to simply write my own Sniff, and it works on its own, but when I run it with PSR12 enabled, it just results in some sort of infinite loop and won't fix the file at all. So I'm back to Square 1. I'm basically just hitting wall after wall at this point which is surprising me as I thought this would be quite the simple/standard rule to implement going into all this!
Beta Was this translation helpful? Give feedback.
All reactions
-
Hi, @ShaneOH! I know this was asked a long time ago, and you might have solved it already or found a workaround. I will answer to the best of my knowledge in case it helps other users who might be facing the same problem.
Any guidance on how to get this spacing rule working?
Adding the following to your ruleset should work:
<rule ref="PSR12"/>
<rule ref="Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose"/>
As you pointed out, the PSR12 standard enables two specific error codes from the Squiz.WhiteSpace.ControlStructureSpacing sniff:
PHP_CodeSniffer/src/Standards/PSR12/ruleset.xml
Lines 225 to 226 in 79238b1
When reading the ruleset, PHPCS turns off the sniff, except for the two error codes listed in the PSR12 ruleset. Behind the scenes, it does that by setting the severity of the sniff to 0 and the severity of the enabled error codes to 5:
PHP_CodeSniffer/src/Ruleset.php
Lines 645 to 646 in 6f33e27
The error code related to the behavior that you are looking for is Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose. Adding <rule ref="Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose"/> to your ruleset enables it.
I'm not sure if not being able to enable all the error codes of a particular sniff by adding <rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/> to the ruleset, should be considered a bug or not. I also expected it to work, but I can confirm that it doesn't. Explicitly setting the severity of this sniff does work (and will turn on all of its error codes):
<rule ref="PSR12"/>
<rule ref="Squiz.WhiteSpace.ControlStructureSpacing">
<severity>5</severity>
</rule>
Beta Was this translation helpful? Give feedback.