Home » PHP » PHP_CodeSniffer » Bug #9411
too few pattern characters cause incorrect error report
Details
| Submitted | 2006-11-21 21:32 UTC |
|---|---|
| From | kwilliams at erepublic dot com |
| Assigned | squiz |
| Status | Closed |
| Package | PHP_CodeSniffer |
| PHP Version | 5.1.6 |
| OS | Windows XP Pro SP2 |
| Roadmaps | (Not assigned) |
Comments
[2006-11-21 21:32 UTC] kwilliams at erepublic dot com
Description:
------------
I was using ControlSignatureSniff class to parse my PHP control structures. Everything worked against my code (as far as whether the reported errors were correct or not), until I was testing against this pattern:
'if (...)\r\n{\r\n'
which was originally (from source code):
'if (...) {\n'
The bug was causing a correct if block to still trigger an error. I dug through the AbstractPatternSniff class and found the problem (a few hours later). The bug was in the last sub-string sub-pattern the original code creates. The original code was making a call to substr() with a length parameter passed in, even though the last sub-string sub-pattern to get was the rest of the pattern anyways. In order to fix it, I only allowed the length parameter to be passed when the current sub-pattern you want is not the last part of the entire pattern's string.
Test script:
---------------
// Use this as the patterns array in
// ControlSignatureSniff::getPatterns
// array(
// "do\r\n{...} while (...);\r\n",
// "while (...)\r\n{\r\n",
// "for (...)\r\n{\r\n",
// "if (...)\r\n{\r\n",
// "foreach (...)\r\n{\r\n",
// "}\r\nelseif (...)\r\n{\r\n",
// "}\r\nelse\r\n{\r\n",
// "do\r\n{\r\n",
// );
// Use this code as the script to test against.
// Make sure the newlines are windows (\r\n) style in
// Whatever IDE you are working in.
if ($foo)
{
echo "here";
}
// Problem code:
// Line: 530
// In file: AbstractPatternSniff.php
// -----------------------------------
// if ($i !== ($length - 1) || $patternCount === 1) {
// $str = substr($pattern, $firstToken, $lastToken);
// } else {
// $str = substr($pattern, $firstToken);
// }
// -----------------------------------
//
// My fixed code:
// -----------------------------------
// if ($i === ($length - 1)) {
// $str = substr($pattern, $firstToken);
// } else {
// $str = substr($pattern, $firstToken, $lastToken);
// }
// -----------------------------------
Expected result:
----------------
I expected to see no triggered errors with the test script.
Actual result:
--------------
FILE: test.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 0 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
[LINE 2] ERROR: Expected "if (...)\r\n{\r\n" found "if (...)\r\n{\r\n".
--------------------------------------------------------------------------------
[2006-11-21 21:37 UTC] kwilliams at erepublic dot com
The actual results should have a space character after the 'if' clause of the second double quoted string. Essentially, I'm just saying that the first and second double quoted strings are identical.
[2006-11-23 03:56 UTC] squiz at php dot net
This bug has been fixed in CVS.
If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET).
If this was a problem with the pear.php.net website, the change should be live shortly.
Otherwise, the fix will appear in the package's next release.
Thank you for the report and for helping us make PEAR better.