Skip to content

Commit

Permalink
Added PSR12.ControlStructures.BooleanOperatorPlacement sniff to enfor…
Browse files Browse the repository at this point in the history
…ce that boolean operators between conditions are consistently at the start or end of the line (ref squizlabs#750)
  • Loading branch information
gsherwood committed Sep 4, 2019
1 parent e201da5 commit 6f4ac49
Show file tree
Hide file tree
Showing 5 changed files with 353 additions and 0 deletions.
10 changes: 10 additions & 0 deletions package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
-- Thanks to Mponos George for the contribution
- Added Generic.PHP.RequireStrictTypes sniff
-- Enforce the use of a strict types declaration in PHP files
- Added PSR12.ControlStructures.BooleanOperatorPlacement sniff
-- Enforces that boolean operators between conditions are consistently at the start or end of the line
- Added PSR12.Files.DeclareStatement sniff
-- Enforces the formatting of declare statements within a file
- Added PSR12.Files.FileHeader sniff
Expand Down Expand Up @@ -1116,6 +1118,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
<dir name="Classes">
<file baseinstalldir="PHP/CodeSniffer" name="ClassInstantiationSniff.php" role="php" />
</dir>
<dir name="ControlStructures">
<file baseinstalldir="PHP/CodeSniffer" name="BooleanOperatorPlacementSniff.php" role="php" />
</dir>
<dir name="Files">
<file baseinstalldir="PHP/CodeSniffer" name="DeclareStatementSniff.php" role="php" />
<file baseinstalldir="PHP/CodeSniffer" name="FileHeaderSniff.php" role="php" />
Expand Down Expand Up @@ -1148,6 +1153,11 @@ http://pear.php.net/dtd/package-2.0.xsd">
<file baseinstalldir="PHP/CodeSniffer" name="ClassInstantiationUnitTest.inc.fixed" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="ClassInstantiationUnitTest.php" role="test" />
</dir>
<dir name="ControlStructures">
<file baseinstalldir="PHP/CodeSniffer" name="BooleanOperatorPlacementUnitTest.inc" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="BooleanOperatorPlacementUnitTest.inc.fixed" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="BooleanOperatorPlacementUnitTest.php" role="test" />
</dir>
<dir name="Files">
<file baseinstalldir="PHP/CodeSniffer" name="DeclareStatementUnitTest.inc" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="DeclareStatementUnitTest.inc.fixed" role="test" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
<?php
/**
* Checks that control structures have boolean operators in the correct place.
*
* @author Greg Sherwood <gsherwood@squiz.net>
* @copyright 2006-2019 Squiz Pty Ltd (ABN 77 084 670 600)
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Standards\PSR12\Sniffs\ControlStructures;

use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;

class BooleanOperatorPlacementSniff implements Sniff
{


/**
* Returns an array of tokens this test wants to listen for.
*
* @return array
*/
public function register()
{
return [
T_IF,
T_WHILE,
T_SWITCH,
T_ELSEIF,
];

}//end register()


/**
* Processes this test, when one of its tokens is encountered.
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the current token
* in the stack passed in $tokens.
*
* @return void
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

if (isset($tokens[$stackPtr]['parenthesis_opener']) === false
|| isset($tokens[$stackPtr]['parenthesis_closer']) === false
) {
return;
}

$parenOpener = $tokens[$stackPtr]['parenthesis_opener'];
$parenCloser = $tokens[$stackPtr]['parenthesis_closer'];

if ($tokens[$parenOpener]['line'] === $tokens[$parenCloser]['line']) {
// Conditions are all on the same line.
return;
}

$find = [
T_BOOLEAN_AND,
T_BOOLEAN_OR,
];

$operator = $parenOpener;
$position = null;
$error = false;
$operators = [];

do {
$operator = $phpcsFile->findNext($find, ($operator + 1), $parenCloser);
if ($operator === false) {
break;
}

$operators[] = $operator;

$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($operator - 1), $parenOpener, true);
if ($prev === false) {
// Parse error.
return;
}

if ($tokens[$prev]['line'] < $tokens[$operator]['line']) {
// The boolean operator is the first content on the line.
if ($position === null) {
$position = 'first';
}

if ($position !== 'first') {
$error = true;
}

continue;
}

$next = $phpcsFile->findNext(T_WHITESPACE, ($operator + 1), $parenCloser, true);
if ($next === false) {
// Parse error.
return;
}

if ($tokens[$next]['line'] > $tokens[$operator]['line']) {
// The boolean operator is the last content on the line.
if ($position === null) {
$position = 'last';
}

if ($position !== 'last') {
$error = true;
}

continue;
}

if ($position === null) {
$position = 'middle';
}

// Error here regardless as boolean operators need to be at start/end of line.
$msg = 'Boolean operators between conditions must be at the beginning or end of the line';
$phpcsFile->addError($msg, $next, 'FoundMiddle');

if ($position !== 'middle') {
$error = true;
}
} while ($operator !== false);

if ($error === false) {
return;
}

$error = 'Boolean operators between conditions must be at the beginning or end of the line, but not both';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'FoundMixed');
if ($fix === false) {
return;
}

$phpcsFile->fixer->beginChangeset();
foreach ($operators as $operator) {
$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($operator - 1), $parenOpener, true);
$next = $phpcsFile->findNext(T_WHITESPACE, ($operator + 1), $parenCloser, true);

if ($position === 'last') {
if ($tokens[$next]['line'] === $tokens[$operator]['line']) {
if ($tokens[$prev]['line'] === $tokens[$operator]['line']) {
// Move the content after the operator to the next line.
if ($tokens[($operator + 1)]['code'] === T_WHITESPACE) {
$phpcsFile->fixer->replaceToken(($operator + 1), '');
}

$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $operator, true);
$padding = str_repeat(' ', ($tokens[$first]['column'] - 1));
$phpcsFile->fixer->addContent($operator, $phpcsFile->eolChar.$padding);
} else {
// Move the operator to the end of the previous line.
if ($tokens[($operator + 1)]['code'] === T_WHITESPACE) {
$phpcsFile->fixer->replaceToken(($operator + 1), '');
}

$phpcsFile->fixer->addContent($prev, ' '.$tokens[$operator]['content']);
$phpcsFile->fixer->replaceToken($operator, '');
}
}//end if
} else {
if ($tokens[$prev]['line'] === $tokens[$operator]['line']) {
if ($tokens[$next]['line'] === $tokens[$operator]['line']) {
// Move the operator, and the rest of the expression, to the next line.
if ($tokens[($operator - 1)]['code'] === T_WHITESPACE) {
$phpcsFile->fixer->replaceToken(($operator - 1), '');
}

$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $operator, true);
$padding = str_repeat(' ', ($tokens[$first]['column'] - 1));
$phpcsFile->fixer->addContentBefore($operator, $phpcsFile->eolChar.$padding);
} else {
// Move the operator to the start of the next line.
if ($tokens[($operator - 1)]['code'] === T_WHITESPACE) {
$phpcsFile->fixer->replaceToken(($operator - 1), '');
}

$phpcsFile->fixer->addContentBefore($next, $tokens[$operator]['content'].' ');
$phpcsFile->fixer->replaceToken($operator, '');
}
}//end if
}//end if
}//end foreach

$phpcsFile->fixer->endChangeset();

}//end process()


}//end class
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php
if (
$expr1
&& $expr2
&& ($expr3
|| $expr4)
&& $expr5
) {
// if body
} elseif (
$expr1 &&
($expr3 || $expr4)
&& $expr5
) {
// elseif body
} elseif (
$expr1
&& ($expr3 || $expr4) &&
$expr5
) {
// elseif body
}

if ($expr1 || $expr2) {
}

do {
} while (
$expr1 || $expr2
|| $expr3 ||
$expr4
);

switch (
$expr1
&& $expr2 &&
$expr3 || $expr4 || $expr5 && $expr6 &&
$expr7
) {
// structure body
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
if (
$expr1
&& $expr2
&& ($expr3
|| $expr4)
&& $expr5
) {
// if body
} elseif (
$expr1 &&
($expr3 ||
$expr4) &&
$expr5
) {
// elseif body
} elseif (
$expr1
&& ($expr3
|| $expr4)
&& $expr5
) {
// elseif body
}

if ($expr1 || $expr2) {
}

do {
} while (
$expr1
|| $expr2
|| $expr3
|| $expr4
);

switch (
$expr1
&& $expr2
&& $expr3
|| $expr4
|| $expr5
&& $expr6
&& $expr7
) {
// structure body
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php
/**
* Unit test class for the BooleanOperatorPlacement sniff.
*
* @author Greg Sherwood <gsherwood@squiz.net>
* @copyright 2006-2019 Squiz Pty Ltd (ABN 77 084 670 600)
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Standards\PSR12\Tests\ControlStructures;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

class BooleanOperatorPlacementUnitTest extends AbstractSniffUnitTest
{


/**
* Returns the lines where errors should occur.
*
* The key of the array should represent the line number and the value
* should represent the number of errors that should occur on that line.
*
* @return array<int, int>
*/
public function getErrorList()
{
return [
10 => 1,
12 => 1,
16 => 1,
18 => 1,
28 => 1,
29 => 1,
34 => 1,
37 => 3,
];

}//end getErrorList()


/**
* Returns the lines where warnings should occur.
*
* The key of the array should represent the line number and the value
* should represent the number of warnings that should occur on that line.
*
* @return array<int, int>
*/
public function getWarningList()
{
return [];

}//end getWarningList()


}//end class

0 comments on commit 6f4ac49

Please sign in to comment.