Description
Rule
The following should be discouraged:
<?php if ($block->hasThings()) { ?>
<div>
<h1>Things</h1>
<?php foreach ($block->getThings() as $thing) { ?>
<h2><?= $block->escapeHtml($thing->name()) ?></h2>
<div>
<div>Bunch of HTML</div>
</div>
<?php } ?>
</div>
<?php } ?>
Instead, use alternative control structure syntax, i.e. endif
, endforeach
etc., if HTML is embedded within the block:
<?php if ($block->hasThings()): ?>
<div>
<h1>Things</h1>
<?php foreach ($block->getThings() as $thing): ?>
<h2><?= $block->escapeHtml($thing->name()) ?></h2>
<div>
<div>Bunch of HTML</div>
</div>
<?php endforeach ?>
</div>
<?php endif ?>
Reason
A mix of HTML and PHP in templates with different indentations is already hard to read. Lonely closing braces make it even harder to reasonate about the code, and easier to introduce bugs. With the alternative control structure syntax, it becomes a bit clearer, which block becomes closed, improving readability and maintainability.
We do not want to disallow the normal PHP syntax altogether in templates, because as long as it is within one PHP block, it not harder to read and has the advantage of familiarity:
<?php
if ($block->hasThings()) {
$headline = 'A bucket of things';
} else {
$headline = 'An empty bucket';
}
is not worse than
<?php
if ($block->hasThings()):
$headline = 'A bucket of things';
else:
$headline = 'An empty bucket';
endif;
and certainly better than
<?php if ($block->hasThings()): ?>
<?php $headline = 'A bucket of things'; ?>
<?php else: ?>
<?php $headline = 'An empty bucket'; ?>
<?php endif; ?>
Previous discussion: #16
Implementation
Trace opening and closing braces as well as opening and closing <?php
tags, alert if there is a mismatch