Closed shvlv closed 1 week ago
@shvlv I was under the impression that no one in the thread was really in favor of enforcing the short open echo tag.
@shvlv I was under the impression that no one in the thread was really in favor of enforcing the short open echo tag.
Thanks, as an initial discussion happened internally, I will provide a short summary. For AlternativeControlStructure.Encouraged
, we agreed it makes templates more legible, and it's pretty common in WordPress templates as well.
For ShortEchoTag.Encouraged
, we discussed the following points.
Pros:
Cons:
I personally found the cons not very robust for the several reasons.
WP/VIP sniffs disallow short tags, including short echo tag
It's true, but it's also in the WordPress community discussion. See https://github.com/WordPress/WordPress-Coding-Standards/issues/1642. The problem is that WordPress is a very big open-source project, and it should care a lot about backward compatibility and has a pretty long decision-making process. In Syde, we definitely target PHP 8.0+ environments and we should not care about problems with PHP <= 5.6. We already have sniffs helpful for modern PHP/OOP which is not applicable for WordPress core development.
it would probably cause a lot of issues in the existing code
We add the sniff to the extra InpsydeTemplates
ruleset. It's optional, and as it's suggested in the documentation, it should be applied only to template files like:
<rule ref="InpsydeTemplates">
<include-pattern>*/templates/*</include-pattern>
<include-pattern>*/views/*</include-pattern>
<include-pattern>*/Blocks/*/render.php</include-pattern>
</rule>
So, the new sniff will not be applied to the whole codebase. To apply it, you should change the PHPCS configuration intentionally.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) Feature.
What is the current behavior? (You can also link to an open issue here) We have the single template-specific sniff and want to go ahead.
What is the new behavior (if this is a feature change)? Two new sniffs are added to follow https://platesphp.com/templates/syntax/.
AlternativeControlStructure
If the body contains inline HTML, it encourages the usage of alternative syntax for control structures.
Wrong:
Correct:
The implementation was inspired by
Universal.DisallowAlternativeSyntaxSniff
. Still, the logic was simplified (closed scopes is not needed for templates https://github.com/PHPCSStandards/PHPCSExtra/blob/ed86bb117c340f654eab603a06b95a437ac619c9/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.1.inc.fixed#L234-L272). Also I decided to make the sniff not fixable because otherwise, it would add the complexity a lot (we need to handleelse if
also to detect closer based on opener and so on).ShortEchoTag
If encourages the usage of the short echo tag for one-line echoing.
Wrong:
Correct:
It's autofixable.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?) No.
Other information: We should consider adding tests for the fixing, but I think it should be additional discussion and PR.