Squiz.WhiteSpace.ScopeClosingBrace fails to fix closing brace within indented PHP tags

[rezashamdani]

Good Monday Morning,

I just using the phpcs & squzlabs fot the past 2 days, so far it help me a lot with my ugly writen codes.

I have problem with large files with the last log shown below; Reached maximum number of loops with 2 violations left unfixed END FILE FIXING

If there any change for me to add more than 50 loops maximum ?

Big Thanks.

[gsherwood]

If it hit 50 loops, it is never going to finish. The most likely issue is that two of the fixers are conflicting, but I can only figure that out if you're able to get me some code you can reproduce the issue on, and let me know what coding standard you are using. Is that possible?

[rezashamdani]

attached is my ruleset.xml ruleset.xml.zip

let me know what else do you need 🤝

[gsherwood]

let me know what else do you need

I'd need a sample file you are using to reproduce the problem please.

[rezashamdani]

here is the file, thanks. sample.php.zip

my phpcs version: PHP_CodeSniffer version 2.7.1 (stable) by Squiz (http://www.squiz.net)

[gsherwood]

Perfect, thanks. I'll take a look into it.

[gsherwood]

The smallest piece of code I can reproduce with is:

<?php
if ($foo === $bar)
    $foo = true;
else
    $foo = false;

  if ($foo) {
    ?>
    <?php } ?>

[rezashamdani]

is this the problem ? ?> <?php

[gsherwood]

Smaller bit of code:

<?php
  if ($foo) {
    ?>
    <?php         } ?>

[gsherwood]

The problem was with the Squiz.WhiteSpace.ScopeClosingBrace sniff thinking that the <?php } ?> line was not indented because there was no PHP whitespace (T_WHITESPACE) at the start. I've changed the sniff to make sure it also looks for T_INLINE_HTML so it knows how far indented the PHP code actually is.

The fix has been committed and will be in the next release. Thanks for providing those test files.

[rezashamdani]

thanks, i'll be waiting the next release

[gsherwood]

I checked your sample file with your ruleset before committing.

When I do, I get this output from PHPCS:

FILE: sample.php
------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 17 ERRORS AND 1 WARNING AFFECTING 16 LINES
------------------------------------------------------------------------------------------------------------------------------------------------------
   1 | WARNING | [ ] A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it should execute
     |         |     logic with side effects, but should not do both. The first symbol is defined on line 23 and the first side effect is on line 2.
  23 | ERROR   | [x] Opening brace should be on a new line
  39 | ERROR   | [x] Inline control structures are not allowed
  41 | ERROR   | [x] Expected 1 space after ELSE keyword; newline found
  41 | ERROR   | [x] Inline control structures are not allowed
  57 | ERROR   | [x] Inline control structures are not allowed
  59 | ERROR   | [x] Expected 1 space after ELSE keyword; newline found
  59 | ERROR   | [x] Inline control structures are not allowed
 223 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 249 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 320 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 324 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 331 | ERROR   | [x] Inline control structures are not allowed
 351 | ERROR   | [x] Inline control structures are not allowed
 410 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 420 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 430 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
 561 | ERROR   | [x] Short PHP opening tag used; expected "<?php" but found "<?"
------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 47ms; Memory: 6Mb

Running PHPCBF over it produces no errors any more:

Processing sample.php [PHP => 2004 tokens in 612 lines]... DONE in 13ms (17 fixable violations)
        => Fixing file: 0/17 violations remaining [made 4 passes]... DONE in 53ms
Patched 1 file
Time: 100ms; Memory: 6Mb

And running a diff report with PHPCS shows:

115:PHP_CodeSniffer gsherwood$ php scripts/phpcs ~/Desktop/sample.php --standard=~/Desktop/ruleset.xml --report=diff
--- /Users/gsherwood/Desktop/sample.php
+++ PHP_CodeSniffer
@@ -20,7 +20,8 @@
 //$_SESSION['test'] = 123;
 // <editor-fold defaultstate="collapsed" desc="Functions">

-function createCombo($sql, $setvalue = "", $disabled = "", $id = "", $valuekey = "", $value = "", $uniq = "", $tabindex = "", $class = "", $empty = 0) {
+function createCombo($sql, $setvalue = "", $disabled = "", $id = "", $valuekey = "", $value = "", $uniq = "", $tabindex = "", $class = "", $empty = 0)
+{
   global $myDatabase;
   ini_set('default_charset', 'utf-8');
   $result2 = $myDatabase->query("SET NAMES 'UTF8'", MYSQLI_STORE_RESULT);
@@ -36,10 +37,11 @@
     }

     while ($combo_row = $result->fetch_object()) {
-      if (strtoupper($combo_row->$valuekey) == strtoupper($setvalue))
+      if (strtoupper($combo_row->$valuekey) == strtoupper($setvalue)) {
         $prop = "selected";
-      else
-        $prop = "";
+      } else {
+$prop = "";
+      }

       echo "<OPTION value=\"" . $combo_row->$valuekey . "\" " . $prop . ">" . $combo_row->$value . "</OPTION>";
     }
@@ -54,10 +56,11 @@

 // </editor-fold>

-if ($_SESSION['userCategory'] != 1)
+if ($_SESSION['userCategory'] != 1) {
   $using_cop = true;
-else
-  $using_cop = false;
+} else {
+$using_cop = false;
+}

 $qry = "SELECT concat(case when title = 1 then 'Mr.' when title = 2 then 'Ms.'  when title = 3 then 'Mrs.' when title = 4 then 'PT.'  when title = 5 then 'CV.' else '' end ,' ' , `client_name` ) as client_name " .
     "FROM client where client_id = " . $_SESSION['openPolicyID_client'];
@@ -220,7 +223,7 @@
               $("#prs__ff_onSUBMIT_FILTER").trigger("click");
           }
         });
-<?
+<?php
 if ($_SESSION['openPolicyID_client_messageboard'] > 0) {
   ?>
           $.fancybox({
@@ -246,7 +249,7 @@
               $("body").css("overflow", "auto");
             }
           });
-  <?
+  <?php
   $_SESSION['openPolicyID_client_messageboard'] --;
 }
 ?>
@@ -317,18 +320,20 @@
         <div class="container">
           <div class="row">
             <div class="span4 logo">
-              <?
+              <?php
               if (file_exists("./_images/" . ($_SESSION['applicationType'] == "1" ? 'client/' : 'client_training/') . $_SESSION['openPolicyID_client'] . ".png")) {
                 ?>
                 <img alt="logo" title="" width="123px" src="./_images/<?php echo ($_SESSION['applicationType'] == "1" ? 'client/' : 'client_training/') . $_SESSION['openPolicyID_client']; ?>.png">
-                <?
+                <?php
               } else {
                 ?>
                 <img alt="logo" title="" width="123px" src="./_images/<?php echo ($_SESSION['applicationType'] == "1" ? 'client/' : 'client_training/') . $_SESSION['openPolicyID_client']; ?>.jpg">
               <?php } ?>
               <br/><?php echo $clientName; ?>
             </div>
-            <div class="span4"><?php if ($_SESSION['applicationType'] == 2) echo '<div style="font-size:25px; text-align:center;margin-top:40px;">TRAINING VERSION</div>'; ?></div>
+            <div class="span4"><?php if ($_SESSION['applicationType'] == 2) {
+echo '<div style="font-size:25px; text-align:center;margin-top:40px;">TRAINING VERSION</div>';
+                               } ?></div>
             <div class="span4 logo">
               <!-- HEADER:  LOGO -->
               <a class="logo">
@@ -348,8 +353,9 @@
                   <ul class="nav nav-pills">
                     <li class="single">
                       <a href="<?php
-                      if (isset($_SESSION['originWebsite']))
+                      if (isset($_SESSION['originWebsite'])) {
                         echo $_SESSION['originWebsite'] . '/';
+                      }
                       ?>corporate.php?from=client">
                         HOME
                         <i>s</i>
@@ -407,7 +413,7 @@
                           <li><a href="procedures.php">Terms & Conditions</a></li>
                         </ul>
                       </li>
-                      <?
+                      <?php
                     }
                     if ($right_ViewManagementReports) {
                       ?>
@@ -417,7 +423,7 @@
                           <i>dashboards</i>
                         </a>
                       </li>
-                      <?
+                      <?php
                     }
                     if ($right_ViewGuidelines) {
                       ?>
@@ -427,7 +433,7 @@
                           <i>list of hospitals</i>
                         </a>
                       </li>
-                      <?
+                      <?php
                     }
                     ?>
                   </ul>
@@ -558,7 +564,7 @@
             <div class="row show-grid">
               <!-- FOOTER: COPYRIGHT TEXT -->
               <div class="span12">
-                <p><?
+                <p><?php
                   if ($_SESSION['userID'] == 3) {
                     echo "<br/>";
                     echo "HTTP_HOST:" . $_SERVER["HTTP_HOST"] . "<br/>";

So it looks fine to me. Maybe you are missing other fixes that have since been released. You could always clone the report directly and run PHPCS from there to test:

git clone https://github.com/squizlabs/PHP_CodeSniffer.git
cd PHP_CodeSniffer
php scripts/phpcs sample.php --standard=ruleset.xml

[rezashamdani]

sorry, still having trouble

[gsherwood]

Your output shows you must be using a different ruleset than the one you posted here. The one you are using has, for example, the Squiz.Strings.ConcatenationSpacing sniff included, but the one you posted doesn't.

Are you sure you gave me the right ruleset?

Note that I've tried fixing your file using the entire Squiz standard and I can't, so there is probably a second issue somewhere in this file. So I'll try and find and fix that, but it would be good to know wat standard you are using so I can make sure your specific case is fixed this time.

[rezashamdani]

sorry, i was reupdating my codesniffer version to PHP_CodeSniffer version 2.8.1 (stable) by Squiz (http://www.squiz.net) and apparently, it replace my previous ruleset.xml

so here is my ruleset.xml on my /home atm /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/Generic/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/MySource/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/PEAR/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/PHPCS/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/PSR1/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/PSR2/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/Squiz/ruleset.xml /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/Zend/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/Generic/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/MySource/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/PEAR/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/PHPCS/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/PSR1/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/PSR2/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/Squiz/ruleset.xml /Users/rezash//vendor/squizlabs/php_codesniffer/CodeSniffer/Standards/Zend/ruleset.xml

it look different from before, now i'm confuse which ruleset should i've change.

[rezashamdani]

Morning,

Today i've edit the file under /Users/rezash//pear/share/pear/PHP/CodeSniffer/Standards/Squiz/Sniffs/WhiteSpace/ScopeClosingBraceSniff.php

and write the changes as declared by you in bug #1405, now the problem is solved. i've checked it with this command phpcs --standard=~/Sites/ruleset.xml ~/Sites/howdenbenefitsasia.local/benefit-summary.php --report=diff -vv

Because i cannot choose which one is the default ruleset, so i set the standard like above.

Can you please point out which one is the default ruleset? or i can add my ruleset to the default so globally my phpcs will work like this one. the list is in above post.

Gracias.

[gsherwood]

I found the other issue with the Squiz standard unable to fix your file, but it was completely unrelated to the original issue here. It has been fixed though.

[gsherwood]

@rezashamdani You've obviously got PHPCS installed via composer and via PEAR. The fact you've had to change the PEAR one tells me that your phpcs command is obviously pointing to the PEAR installed version and not a globally installed composer version. That's fine.

When you run the phpcs command, the default standard is the PEAR standard. You can change this using a setting if you want, or you can specify the standard to use on the command line, as you are doing. You can also your standard into the root directory of your project and name it phpcs.xml to have PHPCS use it automatically when checking your project.

You might want to look at the following wiki sections: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Usage#specifying-a-coding-standard https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#setting-the-default-coding-standard https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#using-a-default-configuration-file