Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Squiz.WhiteSpace.OperatorSpacing false positives for some negation operators #2640

Merged
merged 1 commit into from Nov 12, 2019
Merged

Squiz.WhiteSpace.OperatorSpacing false positives for some negation operators #2640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
89 changes: 46 additions & 43 deletions src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php
View file
Open in desktop
Expand Up @@ -9,8 +9,8 @@

namespace PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace;

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

class OperatorSpacingSniff implements Sniff
Expand Down Expand Up @@ -42,6 +42,13 @@ class OperatorSpacingSniff implements Sniff
*/
public $ignoreSpacingBeforeAssignments = true;

/**
* A list of tokens that aren't considered as operands.
*
* @var string[]
*/
private $nonOperandTokens = [];


/**
* Returns an array of tokens this test wants to listen for.
Expand All @@ -57,6 +64,43 @@ public function register()
$targets[] = T_INLINE_ELSE;
$targets[] = T_INSTANCEOF;

// Trying to operate on a negative value; eg. ($var * -1).
$this->nonOperandTokens = Tokens::$operators;
// Trying to compare a negative value; eg. ($var === -1).
$this->nonOperandTokens += Tokens::$comparisonTokens;
// Trying to compare a negative value; eg. ($var || -1 === $b).
$this->nonOperandTokens += Tokens::$booleanOperators;
// Trying to assign a negative value; eg. ($var = -1).
$this->nonOperandTokens += Tokens::$assignmentTokens;
$this->nonOperandTokens += [
// Returning/printing a negative value; eg. (return -1).
T_RETURN => T_RETURN,
T_ECHO => T_ECHO,
T_PRINT => T_PRINT,
T_YIELD => T_YIELD,

// Trying to use a negative value; eg. myFunction($var, -2).
T_COMMA => T_COMMA,
T_OPEN_PARENTHESIS => T_OPEN_PARENTHESIS,
T_OPEN_SQUARE_BRACKET => T_OPEN_SQUARE_BRACKET,
T_OPEN_SHORT_ARRAY => T_OPEN_SHORT_ARRAY,
T_DOUBLE_ARROW => T_DOUBLE_ARROW,
T_COLON => T_COLON,
T_INLINE_THEN => T_INLINE_THEN,
T_INLINE_ELSE => T_INLINE_ELSE,
T_CASE => T_CASE,
T_OPEN_CURLY_BRACKET => T_OPEN_CURLY_BRACKET,

// Casting a negative value; eg. (array) -$a.
T_ARRAY_CAST => T_ARRAY_CAST,
T_BOOL_CAST => T_BOOL_CAST,
T_DOUBLE_CAST => T_DOUBLE_CAST,
T_INT_CAST => T_INT_CAST,
T_OBJECT_CAST => T_OBJECT_CAST,
T_STRING_CAST => T_STRING_CAST,
T_UNSET_CAST => T_UNSET_CAST,
];

return $targets;

}//end register()
Expand Down Expand Up @@ -311,48 +355,7 @@ protected function isOperator(File $phpcsFile, $stackPtr)
// Check minus spacing, but make sure we aren't just assigning
// a minus value or returning one.
$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
if ($tokens[$prev]['code'] === T_RETURN) {
// Just returning a negative value; eg. (return -1).
return false;
}

if (isset(Tokens::$operators[$tokens[$prev]['code']]) === true) {
// Just trying to operate on a negative value; eg. ($var * -1).
return false;
}

if (isset(Tokens::$comparisonTokens[$tokens[$prev]['code']]) === true) {
// Just trying to compare a negative value; eg. ($var === -1).
return false;
}

if (isset(Tokens::$booleanOperators[$tokens[$prev]['code']]) === true) {
// Just trying to compare a negative value; eg. ($var || -1 === $b).
return false;
}

if (isset(Tokens::$assignmentTokens[$tokens[$prev]['code']]) === true) {
// Just trying to assign a negative value; eg. ($var = -1).
return false;
}

// A list of tokens that indicate that the token is not
// part of an arithmetic operation.
$invalidTokens = [
T_COMMA => true,
T_OPEN_PARENTHESIS => true,
T_OPEN_SQUARE_BRACKET => true,
T_OPEN_SHORT_ARRAY => true,
T_DOUBLE_ARROW => true,
T_COLON => true,
T_INLINE_THEN => true,
T_INLINE_ELSE => true,
T_CASE => true,
T_OPEN_CURLY_BRACKET => true,
];

if (isset($invalidTokens[$tokens[$prev]['code']]) === true) {
// Just trying to use a negative value; eg. myFunction($var, -2).
if (isset($this->nonOperandTokens[$tokens[$prev]['code']]) === true) {
return false;
}
}//end if
Expand Down
193 changes: 193 additions & 0 deletions src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc
View file
Open in desktop
Expand Up @@ -268,5 +268,198 @@ $a = ($a)instanceof$b;
// phpcs:set Squiz.WhiteSpace.OperatorSpacing ignoreSpacingBeforeAssignments false
$a = 3;

yield -1;
echo -1;
$a = -1;
func(-1);
$a = [-1];
return -1;
print -1;
$a &= -1;
switch ($a) {
case -1:
}
$a = $a ?? -1;
$a .= -1;
$a /= -1;
$a = [1 => -1];
$a = $a == -1;
$a = $a >= -1;
$a = $a === -1;
$a = $a != -1;
$a = $a !== -1;
$a = $a <= -1;
$a = $a <=> -1;
$a = $a and -1;
$a = $a or -1;
$a = $a xor -1;
$a -= -1;
$a %= -1;
$a *= -1;
$a |= -1;
$a += -1;
$a = $a ** -1;
$a **= -1;
$a = $a << -1;
$a <<= -1;
$a = $a >> -1;
$a >>= -1;
$a = (string) -1;
$a = (array) -1;
$a = (bool) -1;
$a = (object) -1;
$a = (unset) -1;
$a = (float) -1;
$a = (int) -1;
$a ^= -1;
$a = [$a, -1];
$a = $a[-1];
$a = $a ? -1 : -1;

yield - 1;
echo - 1;
$a = - 1;
func(- 1);
$a = [- 1];
return - 1;
print - 1;
$a &= - 1;
switch ($a) {
case - 1:
}
$a = $a ?? - 1;
$a .= - 1;
$a /= - 1;
$a = [1 => - 1];
$a = $a == - 1;
$a = $a >= - 1;
$a = $a === - 1;
$a = $a != - 1;
$a = $a !== - 1;
$a = $a <= - 1;
$a = $a <=> - 1;
$a = $a and - 1;
$a = $a or - 1;
$a = $a xor - 1;
$a -= - 1;
$a %= - 1;
$a *= - 1;
$a |= - 1;
$a += - 1;
$a = $a ** - 1;
$a **= - 1;
$a = $a << - 1;
$a <<= - 1;
$a = $a >> - 1;
$a >>= - 1;
$a = (string) - 1;
$a = (array) - 1;
$a = (bool) - 1;
$a = (object) - 1;
$a = (unset) - 1;
$a = (float) - 1;
$a = (int) - 1;
$a ^= - 1;
$a = [$a, - 1];
$a = $a[- 1];
$a = $a ? - 1 : - 1;


yield -$b;
echo -$b;
$a = -$b;
func(-$b);
$a = [-$b];
return -$b;
print -$b;
$a &= -$b;
switch ($a) {
case -$b:
}
$a = $a ?? -$b;
$a .= -$b;
$a /= -$b;
$a = [1 => -$b];
$a = $a == -$b;
$a = $a >= -$b;
$a = $a === -$b;
$a = $a != -$b;
$a = $a !== -$b;
$a = $a <= -$b;
$a = $a <=> -$b;
$a = $a and -$b;
$a = $a or -$b;
$a = $a xor -$b;
$a -= -$b;
$a %= -$b;
$a *= -$b;
$a |= -$b;
$a += -$b;
$a = $a ** -$b;
$a **= -$b;
$a = $a << -$b;
$a <<= -$b;
$a = $a >> -$b;
$a >>= -$b;
$a = (string) -$b;
$a = (array) -$b;
$a = (bool) -$b;
$a = (object) -$b;
$a = (unset) -$b;
$a = (float) -$b;
$a = (int) -$b;
$a ^= -$b;
$a = [$a, -$b];
$a = $a[-$b];
$a = $a ? -$b : -$b;

yield - $b;
echo - $b;
$a = - $b;
func(- $b);
$a = [- $b];
return - $b;
print - $b;
$a &= - $b;
switch ($a) {
case - $b:
}
$a = $a ?? - $b;
$a .= - $b;
$a /= - $b;
$a = [1 => - $b];
$a = $a == - $b;
$a = $a >= - $b;
$a = $a === - $b;
$a = $a != - $b;
$a = $a !== - $b;
$a = $a <= - $b;
$a = $a <=> - $b;
$a = $a and - $b;
$a = $a or - $b;
$a = $a xor - $b;
$a -= - $b;
$a %= - $b;
$a *= - $b;
$a |= - $b;
$a += - $b;
$a = $a ** - $b;
$a **= - $b;
$a = $a << - $b;
$a <<= - $b;
$a = $a >> - $b;
$a >>= - $b;
$a = (string) - $b;
$a = (array) - $b;
$a = (bool) - $b;
$a = (object) - $b;
$a = (unset) - $b;
$a = (float) - $b;
$a = (int) - $b;
$a ^= - $b;
$a = [$a, - $b];
$a = $a[- $b];
$a = $a ? - $b : - $b;

/* Intentional parse error. This has to be the last test in the file. */
$a = 10 +