Priority: braces should run before no_trailing_whitespace_in_comment

[dmvdbrugge]

Warning: mixed php and html 😉

The PHP version you are using ($ php -v):

PHP 7.1.18 (cli) (built: May 25 2018 19:18:59) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.1.18, Copyright (c) 1999-2018, by Zend Technologies

PHP CS Fixer version you are using ($ php-cs-fixer -V):

PHP CS Fixer 2.12.1 Long Journey by Fabien Potencier and Dariusz Ruminski

The command you use to run PHP CS Fixer:

vendor/bin/php-cs-fixer fix -v --using-cache=no

The configuration file you are using, if any:

Basically the same as dmvdbrugge/dynamic-components, except explicit_string_variable, header_comment, no_alternative_syntax, and no_unset_on_property

If applicable, please provide minimum samples of PHP code (as plain text, not screenshots):

• before running PHP CS Fixer (no changes):
  (This was the minimal sample I could produce)

<?php
// Sorry
?>
<h1>Test</h1>
<div>
    <?php

    $a = true;

    if ($a) { // This will always run
        ?>
        <p>First!</p>
        <?php
        $b = true;

        if ($b) { // And so will this
            ?>
            <p>Hello world</p>
            <?php
        }
    } ?>
</div>

• after first run:

--- Original
+++ New
@@ @@
-    if ($a) { // This will always run
-        ?>
+    if ($a) { // This will always run ?>
         <p>First!</p>
         <?php

• after second run:

--- Original
+++ New
@@ @@
-    if ($a) { // This will always run ?>
+    if ($a) { // This will always run?>
         <p>First!</p>
         <?php

• expected:
  Immediately arrive at the last step

Meta: this is case 72 of #3844

[dmvdbrugge]

When #4085 is merged, braces will run before no_trailing_whitespace_in_comment so after that, all that's needed for this issue is to actually link the priority.

[dmvdbrugge]

Separated the PR from #4085, I can haz [bug][has PR] labels? 😜

[SpacePossum]

I don't like the ?> being pulled on the same line of the comment... but that seems to be expected.
I don't like the space being removed before the ?> either, so if it is pulled on that line I would expect a single space before it and that after any more runs it stays like that...

So while this issue indeed correctly reports a priority issue, I do wander if the solution should be changing the priorities or if it would be better to change the behavior of the rules.

[dmvdbrugge]

I don't like the comment being there, the ?> is indeed expected to be pulled up to that line. I would agree with the space staying there.

I made this a priority issue because from my experience, those PRs get merged faster and I just want these issues fixed one way or the other.

If we would fix the fixers later on to keep the space that's 👍 from me, however that could possibly be a lot more (difficult) work.

[dmvdbrugge]

As the close tag is sadly no longer pulled up, this issue will not occur anymore.