[Template] AlternativeControlStructure and ShortEchoTag sniffs

[shvlv]

Please check if the PR fulfills these requirements

• The commit message follows our guidelines
• Tests for the changes have been added (for bug fixes/features)
• Docs have been added/updated (for bug fixes/features)

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:

if ($flag) { ?>
<p>Flag</p>
<?php }

Correct:

if ($flag): ?>
<p>Flag</p>
<?php endif

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 handle else 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:

<?php echo 'content' ?>

Correct:

<?= 'content' ?>

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.

[2ndkauboy]

@shvlv I was under the impression that no one in the thread was really in favor of enforcing the short open echo tag.

[shvlv]

@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:

• it's a pretty common template standard in the PHP community
• it might improve consistency in our codebase

Cons:

• WP/VIP sniffs disallow short tags, including short echo tag
• it would probably cause a lot of issues in the existing code

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 WordPress/WordPress-Coding-Standards#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.

[tfrommen]

Thanks for this, @shvlv!

I left a handful of comments about different things we could change, improve, add.

[tfrommen]

👍