Skip to content

Commit

Permalink
Fixed bug #879 : Generic InlineControlStructureSniff can create parse…
Browse files Browse the repository at this point in the history 
… error when case/if/elseif/else have mixed brace and braceless definitions
  • Loading branch information
@gsherwood
gsherwood committed Jan 29, 2016
1 parent 9a70ae2 commit b498dbe
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 83 deletions.
16 changes: 15 additions & 1 deletion CodeSniffer/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1948,6 +1948,20 @@ private static function _recurseScopeMap(
return $i;
}

if ($opener === null
&& $ignore === 0
&& $tokenType === T_CLOSE_CURLY_BRACKET
&& isset($tokenizer->scopeOpeners[$currType]['end'][$tokenType]) === true
) {
if (PHP_CODESNIFFER_VERBOSITY > 1) {
$type = $tokens[$stackPtr]['type'];
echo str_repeat("\t", $depth);
echo "=> Found curly brace closer before scope opener for $stackPtr:$type, bailing".PHP_EOL;
}

return ($i - 1);
}

if ($opener !== null
&& (isset($tokens[$i]['scope_opener']) === false
|| $tokenizer->scopeOpeners[$tokens[$stackPtr]['code']]['shared'] === true)
Expand Down Expand Up @@ -2116,7 +2130,7 @@ private static function _recurseScopeMap(
echo "=> Found new opening condition before scope opener for $stackPtr:$type, bailing".PHP_EOL;
}

return $stackPtr;
return ($i - 1);
}//end if

if (PHP_CODESNIFFER_VERBOSITY > 1) {
Expand Down
167 changes: 85 additions & 82 deletions CodeSniffer/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,107 +134,110 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)

$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'yes');

if ($fix === true) {
$phpcsFile->fixer->beginChangeset();
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
$closer = $tokens[$stackPtr]['parenthesis_closer'];
} else {
$closer = $stackPtr;
// Stop here if we are not fixing the error.
if ($fix !== true) {
return;
}

$phpcsFile->fixer->beginChangeset();
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
$closer = $tokens[$stackPtr]['parenthesis_closer'];
} else {
$closer = $stackPtr;
}

if ($tokens[($closer + 1)]['code'] === T_WHITESPACE
|| $tokens[($closer + 1)]['code'] === T_SEMICOLON
) {
$phpcsFile->fixer->addContent($closer, ' {');
} else {
$phpcsFile->fixer->addContent($closer, ' { ');
}

$lastNonEmpty = $closer;
for ($end = ($closer + 1); $end < $phpcsFile->numTokens; $end++) {
if ($tokens[$end]['code'] === T_SEMICOLON) {
break;
}

if ($tokens[($closer + 1)]['code'] === T_WHITESPACE
|| $tokens[($closer + 1)]['code'] === T_SEMICOLON
) {
$phpcsFile->fixer->addContent($closer, ' {');
} else {
$phpcsFile->fixer->addContent($closer, ' { ');
if ($tokens[$end]['code'] === T_CLOSE_TAG) {
$end = $lastNonEmpty;
break;
}

$lastNonEmpty = $closer;
for ($end = ($closer + 1); $end < $phpcsFile->numTokens; $end++) {
if ($tokens[$end]['code'] === T_SEMICOLON) {
break;
}
if (isset($tokens[$end]['scope_opener']) === true) {
$type = $tokens[$end]['code'];
$end = $tokens[$end]['scope_closer'];
if ($type === T_DO || $type === T_IF || $type === T_ELSEIF) {
$next = $phpcsFile->findNext(PHP_CodeSniffer_Tokens::$emptyTokens, ($end + 1), null, true);
if ($next === false) {
break;
}

if ($tokens[$end]['code'] === T_CLOSE_TAG) {
$end = $lastNonEmpty;
break;
}
$nextType = $tokens[$next]['code'];

if (isset($tokens[$end]['scope_opener']) === true) {
$type = $tokens[$end]['code'];
$end = $tokens[$end]['scope_closer'];
if ($type === T_DO || $type === T_IF || $type === T_ELSEIF) {
$next = $phpcsFile->findNext(PHP_CodeSniffer_Tokens::$emptyTokens, ($end + 1), null, true);
if ($next === false) {
break;
}

$nextType = $tokens[$next]['code'];

// Let additional conditions loop and find their ending.
if (($type === T_IF
|| $type === T_ELSEIF)
&& ($nextType === T_ELSEIF
|| $nextType === T_ELSE)
) {
continue;
}

// Account for DO... WHILE conditions.
if ($type === T_DO && $nextType === T_WHILE) {
$end = $phpcsFile->findNext(T_SEMICOLON, ($next + 1));
}
}//end if

break;
}//end if
// Let additional conditions loop and find their ending.
if (($type === T_IF
|| $type === T_ELSEIF)
&& ($nextType === T_ELSEIF
|| $nextType === T_ELSE)
) {
continue;
}

if ($tokens[$end]['code'] !== T_WHITESPACE) {
$lastNonEmpty = $end;
}
}//end for
// Account for DO... WHILE conditions.
if ($type === T_DO && $nextType === T_WHILE) {
$end = $phpcsFile->findNext(T_SEMICOLON, ($next + 1));
}
}//end if

$next = $phpcsFile->findNext(T_WHITESPACE, ($closer + 1), ($end + 1), true);
break;
}//end if

// Account for a comment on the end of the line.
for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
if (isset($tokens[($endLine + 1)]) === false
|| $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
) {
break;
}
if ($tokens[$end]['code'] !== T_WHITESPACE) {
$lastNonEmpty = $end;
}
}//end for

if ($tokens[$endLine]['code'] !== T_COMMENT) {
$endLine = $end;
$next = $phpcsFile->findNext(T_WHITESPACE, ($closer + 1), ($end + 1), true);

// Account for a comment on the end of the line.
for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
if (isset($tokens[($endLine + 1)]) === false
|| $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
) {
break;
}
}

if ($next !== $end) {
if ($endLine !== $end) {
$phpcsFile->fixer->addContent($endLine, '}');
} else {
if ($tokens[$end]['code'] !== T_SEMICOLON
&& $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
) {
$phpcsFile->fixer->addContent($end, ';');
}
if ($tokens[$endLine]['code'] !== T_COMMENT) {
$endLine = $end;
}

$phpcsFile->fixer->addContent($end, ' }');
}
if ($next !== $end) {
if ($endLine !== $end) {
$phpcsFile->fixer->addContent($endLine, '}');
} else {
if ($endLine !== $end) {
$phpcsFile->fixer->replaceToken($end, '');
$phpcsFile->fixer->addNewlineBefore($endLine);
$phpcsFile->fixer->addContent($endLine, '}');
} else {
$phpcsFile->fixer->replaceToken($end, '}');
if ($tokens[$end]['code'] !== T_SEMICOLON
&& $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
) {
$phpcsFile->fixer->addContent($end, ';');
}
}//end if

$phpcsFile->fixer->endChangeset();
$phpcsFile->fixer->addContent($end, ' }');
}
} else {
if ($endLine !== $end) {
$phpcsFile->fixer->replaceToken($end, '');
$phpcsFile->fixer->addNewlineBefore($endLine);
$phpcsFile->fixer->addContent($endLine, '}');
} else {
$phpcsFile->fixer->replaceToken($end, '}');
}
}//end if

$phpcsFile->fixer->endChangeset();

}//end process()


Expand Down
12 changes: 12 additions & 0 deletions CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,15 @@ foreach($stuff as $num)
if ($foo) echo 'foo';
elseif ($bar) echo 'bar';
else echo 'baz';

switch ($type) {
case 1:
if ($foo) {
return true;
} elseif ($baz)
return true;
else {
echo 'else';
}
break;
}
12 changes: 12 additions & 0 deletions ...niffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,15 @@ foreach($stuff as $num) {
if ($foo) { echo 'foo'; }
elseif ($bar) { echo 'bar'; }
else { echo 'baz'; }

switch ($type) {
case 1:
if ($foo) {
return true;
} elseif ($baz) {
return true; }
else {
echo 'else';
}
break;
}
1 change: 1 addition & 0 deletions CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public function getErrorList($testFile='InlineControlStructureUnitTest.inc')
142 => 1,
143 => 1,
144 => 1,
150 => 1,
);
break;
case 'InlineControlStructureUnitTest.js':
Expand Down
1 change: 1 addition & 0 deletions package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
- Squiz FunctionCommentThrowTagSniff now ignores non-docblock comments
- Squiz ComparisonOperatorUsageSniff now allows conditions like while(true)
- Fixed bug #872 : Incorrect detection of blank lines between CSS class names
- Fixed bug #879 : Generic InlineControlStructureSniff can create parse error when case/if/elseif/else have mixed brace and braceless definitions
</notes>
<contents>
<dir name="/">
Expand Down

0 comments on commit b498dbe

Please sign in to comment.