Squiz.WhiteSpace.ScopeClosingBrace fixer removes HTML content when fixing closing brace alignment

[GlennBags]

Describe the bug
Div end tag removed when reformatting using phpcbf to reformat code to PSR-12 standard

Code sample

// before reformatter
    </div> <?php endif; ?>

// after reformatter
    <?php endif; ?>

Custom ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
  <description>If you are using a custom ruleset, please enter it here.</description>

  <description>Besides the below, have some exclude patterns that shouldn't affect the outcode.</description>

    <rule ref="PSR12"/>
    <arg name="extensions" value="php" />

</ruleset>

To reproduce
Steps to reproduce the behavior:

1. Create a file called test.php with the code sample above...
2. Run php phpcbf.phar --standard=ruleset.xml test.php -v
3. See code reformatted with the end div removed

Expected behavior
Should not remove HTML

Versions (please complete the following information):

• OS: MacOS 11.5.2
• PHP: 8.0.2
• $ php phpcbf.phar --version
  PHP_CodeSniffer version 3.6.0 (stable) by Squiz (http://www.squiz.net)
• Standard: PSR12

[jrfnl]

@GlennBags The issue cannot be reproduced with the code sample you provided.

[GlennBags]

OK, apologies, the code sample was incomplete. Please try:

<?php if ($someConditional) : ?>
<div class="container">

</div> <?php endif; ?>

Using the above file as the test.php, I am able to replicate using the following ruleset:

<?xml version="1.0"?>
<ruleset name="Selective Standard">

    <rule ref="PSR12"/>

    <arg name="extensions" value="php" />

</ruleset>

[jrfnl]

@GlennBags Thanks, that's better. Reproduced the issue. Thanks for reporting it.

The problem is in the Squiz.WhiteSpace.ScopeClosingBrace sniff which doesn't handle non-whitespace inline HTML tokens on the same line as the scope closer correctly.

This will need looking into.

[jrfnl]

@GlennBags I've prepared a fix for this issue. See PR #3447. Testing appreciated.

[GlennBags]

@GlennBags I've prepared a fix for this issue. See PR #3447. Testing appreciated.

Am I able to check this out to test using composer?

[jrfnl]

Am I able to check this out to test using composer?

@GlennBags You should be able to using squizlabs/php_codesniffer:"dev-feature/3422-squiz-scopeclosingbrace-bug-fix", but you will probably have to set a VCS to point to my fork too. 😒

Once PR #3442 has been merged, testing PRs should become easier as you can then download a PHAR file for each PR to use for testing.

[jrfnl]

@GlennBags Just thinking - if it helps, I could generate the PHAR file for the PR for you. Let me know if you want that.

[GlennBags]

@jrfnl Yes, apparently I can't get it from composer command line, or am I doing something wrong?

bash-4.2# composer  squizlabs/php_codesniffer:"dev-feature/3422-squiz-scopeclosingbrace-bug-fix" --require-dev

  [Symfony\Component\Console\Exception\CommandNotFoundException]
  There are no commands defined in the "squizlabs/php_codesniffer" namespace.

[GlennBags]

Well, duh, I'm testing using the phar, so, yeah, a phar file would help.
I haven't finished my first cup of coffee... that's my excuse and I'm sticking to it! heh

[jrfnl]

@GlennBags 😂 you were also missing the require keyword in Composer. The things lack of coffee makes us do ;-)

You can download zips containing the phpcs and phpcbf PHAR files for this PR here: https://github.com/jrfnl/PHP_CodeSniffer/actions/runs/1311950956

[GlennBags]

Awesome, thank you. Works like a charm.
Pre:

<?php if ($someConditional) : ?>
    <div class="container">

    </div> <?php endif; ?>
<?php if ($someConditional) { ?>
    <div class="container">

    </div> <?php } ?>

Post (after running the phpcbf):

<?php if ($someConditional) : ?>
    <div class="container">

    </div> 
<?php endif; ?>
<?php if ($someConditional) { ?>
    <div class="container">

    </div> 
<?php } ?>

[jrfnl]

Thanks for testing @GlennBags !