From e7563e75dca7bd61bd12b8a1a901ecb878c61234 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 6 Aug 2026 15:12:27 +0200 Subject: [PATCH] Remove duplicate rule coverage and add unary minus spacing Three constructs were reported by more than one rule at once. In each case the narrower rule is silenced and the one with the widest coverage kept, so every input still gets flagged, just once: - long casts: SlevomatCodingStandard.PHP.TypeCast survives. It is the only one of the three that rewrites (double) to (float) and copes with inner spaces like ( boolean ). PSR12.Keywords.ShortFormTypeKeywords is excluded, and so is the cast half of PhpCollective.PHP.ShortCast, whose DoubleNotInvalid check stays because nothing else flags a double negation used as a bool cast. - incrementer spacing: Generic.WhiteSpace.IncrementDecrementSpacing survives, since it also handles targets like $b[0] ++. The matching pair of codes on PhpCollective.WhiteSpace.ImplicitCastSpacing is excluded. - sizeof(): PhpCollective.PHP.RemoveFunctionAlias survives, because unlike Generic.PHP.ForbiddenFunctions it can fix the call rather than only report it. That last one only holds once the alias sniff stops skipping a call that opens a file. Its guard read `if (!$previous)`, and findPrevious() returns index 0 for the open tag, so `findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if (!$previous || in_array($tokens[$previous]['code'], static::$wrongTokens, true)) { + if ($previous === false || in_array($tokens[$previous]['code'], static::$wrongTokens, true)) { return; } diff --git a/PhpCollective/Sniffs/PHP/DisallowFunctionsSniff.php b/PhpCollective/Sniffs/PHP/DisallowFunctionsSniff.php index e0248b5..cb0906b 100644 --- a/PhpCollective/Sniffs/PHP/DisallowFunctionsSniff.php +++ b/PhpCollective/Sniffs/PHP/DisallowFunctionsSniff.php @@ -59,7 +59,7 @@ protected function checkForbiddenFunctions(File $phpcsFile, int $stackPtr): void $tokenContent = $tokens[$stackPtr]['content']; $previous = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if (!$previous || in_array($tokens[$previous]['code'], static::$wrongTokens)) { + if ($previous === false || in_array($tokens[$previous]['code'], static::$wrongTokens)) { return; } @@ -88,7 +88,7 @@ protected function checkImplodeUsage(File $phpcsFile, int $stackPtr): void } $previous = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if (!$previous || in_array($tokens[$previous]['code'], static::$wrongTokens)) { + if ($previous === false || in_array($tokens[$previous]['code'], static::$wrongTokens)) { return; } diff --git a/PhpCollective/Sniffs/PHP/NoIsNullSniff.php b/PhpCollective/Sniffs/PHP/NoIsNullSniff.php index 618548e..8e1afbc 100644 --- a/PhpCollective/Sniffs/PHP/NoIsNullSniff.php +++ b/PhpCollective/Sniffs/PHP/NoIsNullSniff.php @@ -40,7 +40,7 @@ public function process(File $phpcsFile, $stackPtr): void $tokenContent = $tokens[$stackPtr]['content']; $previous = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if (!$previous || in_array($tokens[$previous]['code'], $wrongTokens)) { + if ($previous === false || in_array($tokens[$previous]['code'], $wrongTokens)) { return; } diff --git a/PhpCollective/Sniffs/PHP/RemoveFunctionAliasSniff.php b/PhpCollective/Sniffs/PHP/RemoveFunctionAliasSniff.php index 8a14415..5cdc493 100644 --- a/PhpCollective/Sniffs/PHP/RemoveFunctionAliasSniff.php +++ b/PhpCollective/Sniffs/PHP/RemoveFunctionAliasSniff.php @@ -74,7 +74,7 @@ protected function checkFixableAliases(File $phpcsFile, int $stackPtr): void $tokenContent = $tokens[$stackPtr]['content']; $previous = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if (!$previous || in_array($tokens[$previous]['code'], $wrongTokens)) { + if ($previous === false || in_array($tokens[$previous]['code'], $wrongTokens)) { return; } diff --git a/PhpCollective/Sniffs/PHP/ShortCastSniff.php b/PhpCollective/Sniffs/PHP/ShortCastSniff.php index 628a6fe..c414ece 100644 --- a/PhpCollective/Sniffs/PHP/ShortCastSniff.php +++ b/PhpCollective/Sniffs/PHP/ShortCastSniff.php @@ -40,7 +40,7 @@ public function process(File $phpcsFile, $stackPtr): void if ($tokens[$stackPtr]['content'] === '!') { $prevIndex = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if (!$prevIndex || $tokens[$prevIndex]['content'] !== '!') { + if ($prevIndex === false || $tokens[$prevIndex]['content'] !== '!') { return; } diff --git a/PhpCollective/Sniffs/WhiteSpace/ImplicitCastSpacingSniff.php b/PhpCollective/Sniffs/WhiteSpace/ImplicitCastSpacingSniff.php index d3541d6..32d1b74 100644 --- a/PhpCollective/Sniffs/WhiteSpace/ImplicitCastSpacingSniff.php +++ b/PhpCollective/Sniffs/WhiteSpace/ImplicitCastSpacingSniff.php @@ -9,6 +9,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Util\Tokens; /** * No whitespace should be between implicit cast and variable, the same as with other casts. @@ -21,7 +22,7 @@ class ImplicitCastSpacingSniff implements Sniff */ public function register(): array { - return [T_BOOLEAN_NOT, T_NONE, T_ASPERAND, T_INC, T_DEC]; + return [T_BOOLEAN_NOT, T_NONE, T_ASPERAND, T_INC, T_DEC, T_MINUS]; } /** @@ -37,6 +38,19 @@ public function process(File $phpcsFile, $stackPtr): void return; } + // A minus is only an implicit cast when it negates; as a subtraction it wants its spaces. + if ($tokens[$stackPtr]['code'] === T_MINUS) { + if (!$this->isUnaryOperator($phpcsFile, $stackPtr)) { + return; + } + + // `- -$i` must keep its space: closing it up would produce `--$i`, a decrement. + $followingIndex = $phpcsFile->findNext(T_WHITESPACE, $stackPtr + 1, null, true); + if ($followingIndex !== false && in_array($tokens[$followingIndex]['code'], [T_MINUS, T_DEC], true)) { + return; + } + } + $nextIndex = $phpcsFile->findNext(T_WHITESPACE, $stackPtr + 1, null, true); if ($nextIndex === false || $nextIndex - $stackPtr === 1) { @@ -51,6 +65,62 @@ public function process(File $phpcsFile, $stackPtr): void } } + /** + * A minus is unary only when what precedes it cannot end a value - an operator, an opening + * bracket, a comma, `return` and so on. + * + * The test is deliberately this way round. Listing what may PRECEDE a subtraction instead + * would have to enumerate every value-producing token, and anything forgotten (`__LINE__`, + * `true`, `null`, a qualified constant) would be read as a negation and "fixed" into + * `__LINE__ -1`. Defaulting to binary keeps an unknown predecessor harmless. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * @param int $stackPtr + * + * @return bool + */ + protected function isUnaryOperator(File $phpcsFile, int $stackPtr): bool + { + $tokens = $phpcsFile->getTokens(); + + $previousIndex = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true); + if ($previousIndex === false) { + return true; + } + + $unaryPrefixes = Tokens::$operators + + Tokens::$assignmentTokens + + Tokens::$comparisonTokens + + Tokens::$booleanOperators + + Tokens::$castTokens + + [ + T_OPEN_PARENTHESIS => T_OPEN_PARENTHESIS, + T_OPEN_SQUARE_BRACKET => T_OPEN_SQUARE_BRACKET, + T_OPEN_SHORT_ARRAY => T_OPEN_SHORT_ARRAY, + T_OPEN_CURLY_BRACKET => T_OPEN_CURLY_BRACKET, + T_COMMA => T_COMMA, + T_SEMICOLON => T_SEMICOLON, + T_COLON => T_COLON, + T_DOUBLE_ARROW => T_DOUBLE_ARROW, + T_INLINE_THEN => T_INLINE_THEN, + T_INLINE_ELSE => T_INLINE_ELSE, + T_RETURN => T_RETURN, + T_ECHO => T_ECHO, + T_PRINT => T_PRINT, + T_CASE => T_CASE, + T_BOOLEAN_NOT => T_BOOLEAN_NOT, + T_YIELD => T_YIELD, + T_YIELD_FROM => T_YIELD_FROM, + T_THROW => T_THROW, + T_FN_ARROW => T_FN_ARROW, + T_MATCH_ARROW => T_MATCH_ARROW, + T_OPEN_TAG => T_OPEN_TAG, + T_OPEN_TAG_WITH_ECHO => T_OPEN_TAG_WITH_ECHO, + ]; + + return isset($unaryPrefixes[$tokens[$previousIndex]['code']]); + } + /** * @param \PHP_CodeSniffer\Files\File $phpcsFile * @param int $stackPtr diff --git a/PhpCollective/ruleset.xml b/PhpCollective/ruleset.xml index a6d9f9b..e5a84ee 100644 --- a/PhpCollective/ruleset.xml +++ b/PhpCollective/ruleset.xml @@ -207,15 +207,16 @@ - @@ -234,6 +235,30 @@ + + + + + + + + + + + + + diff --git a/docs/sniffs.md b/docs/sniffs.md index 3f29935..d9dd83c 100644 --- a/docs/sniffs.md +++ b/docs/sniffs.md @@ -1,7 +1,7 @@ # PhpCollective Code Sniffer -The PhpCollectiveStrict standard contains 251 sniffs +The PhpCollectiveStrict standard contains 250 sniffs Generic (27 sniffs) ------------------- @@ -163,7 +163,7 @@ PSR2 (12 sniffs) - PSR2.Namespaces.NamespaceDeclaration - PSR2.Namespaces.UseDeclaration -PSR12 (14 sniffs) +PSR12 (13 sniffs) ----------------- - PSR12.Classes.AnonClassDeclaration - PSR12.Classes.ClassInstantiation @@ -174,7 +174,6 @@ PSR12 (14 sniffs) - PSR12.Files.ImportStatement - PSR12.Functions.NullableTypeDeclaration - PSR12.Functions.ReturnTypeDeclaration -- PSR12.Keywords.ShortFormTypeKeywords - PSR12.Namespaces.CompoundNamespaceDepth - PSR12.Operators.OperatorSpacing - PSR12.Properties.ConstantVisibility diff --git a/tests/_data/ImplicitCastSpacing/after.php b/tests/_data/ImplicitCastSpacing/after.php index c7448d6..f8432c9 100644 --- a/tests/_data/ImplicitCastSpacing/after.php +++ b/tests/_data/ImplicitCastSpacing/after.php @@ -10,15 +10,22 @@ public function run(bool $ready, int $count, int $mask, callable $callable): int { $not = !$ready; $silenced = @$callable(); - $flipped = ~ $mask; - ++$count; - $count--; + $negated = -$count; + $flipped = ~ $mask; $validNot = !$ready; $validSilenced = @$callable(); - ++$count; - $count++; + $validNegated = -$count; + $validSubtraction = $count - $mask; + $constantMinus = __LINE__ - 1; + $boolMinus = (int)true - 1; + $nullMinus = (int)null - 1; + $fqcnMinus = \PHP_INT_MAX - 1; + $doubleNegated = - -$count; + $arrow = fn (): int => -$count; - return (int)$not + (int)$silenced + $flipped + $count + (int)$validNot + (int)$validSilenced; + return (int)$not + (int)$silenced + $negated + $flipped + (int)$validNot + + (int)$validSilenced + $validNegated + $validSubtraction + + $constantMinus + $boolMinus + $nullMinus + $fqcnMinus + $doubleNegated + $arrow(); } } diff --git a/tests/_data/ImplicitCastSpacing/before.php b/tests/_data/ImplicitCastSpacing/before.php index d2f31d9..89be656 100644 --- a/tests/_data/ImplicitCastSpacing/before.php +++ b/tests/_data/ImplicitCastSpacing/before.php @@ -10,15 +10,22 @@ public function run(bool $ready, int $count, int $mask, callable $callable): int { $not = ! $ready; $silenced = @ $callable(); - $flipped = ~ $mask; - ++ $count; - $count --; + $negated = - $count; + $flipped = ~ $mask; $validNot = !$ready; $validSilenced = @$callable(); - ++$count; - $count++; + $validNegated = -$count; + $validSubtraction = $count - $mask; + $constantMinus = __LINE__ - 1; + $boolMinus = (int)true - 1; + $nullMinus = (int)null - 1; + $fqcnMinus = \PHP_INT_MAX - 1; + $doubleNegated = - -$count; + $arrow = fn (): int => - $count; - return (int)$not + (int)$silenced + $flipped + $count + (int)$validNot + (int)$validSilenced; + return (int)$not + (int)$silenced + $negated + $flipped + (int)$validNot + + (int)$validSilenced + $validNegated + $validSubtraction + + $constantMinus + $boolMinus + $nullMinus + $fqcnMinus + $doubleNegated + $arrow(); } } diff --git a/tests/_data/ImplicitCastSpacing/before.tokens.php b/tests/_data/ImplicitCastSpacing/before.tokens.php new file mode 100644 index 0000000..88efc6e --- /dev/null +++ b/tests/_data/ImplicitCastSpacing/before.tokens.php @@ -0,0 +1,62 @@ + - $count; +// T_WHITESPACE T_VARIABLE T_WHITESPACE T_EQUAL T_WHITESPACE T_FN T_WHITESPACE T_OPEN_PARENTHESIS T_CLOSE_PARENTHESIS T_COLON T_WHITESPACE T_STRING T_WHITESPACE T_FN_ARROW T_WHITESPACE T_MINUS T_WHITESPACE T_VARIABLE T_SEMICOLON T_WHITESPACE + +// T_WHITESPACE + return (int)$not + (int)$silenced + $negated + $flipped + (int)$validNot +// T_WHITESPACE T_RETURN T_WHITESPACE T_INT_CAST T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_INT_CAST T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_INT_CAST T_VARIABLE T_WHITESPACE + + (int)$validSilenced + $validNegated + $validSubtraction +// T_WHITESPACE T_PLUS T_WHITESPACE T_INT_CAST T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE + + $constantMinus + $boolMinus + $nullMinus + $fqcnMinus + $doubleNegated + $arrow(); +// T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_WHITESPACE T_PLUS T_WHITESPACE T_VARIABLE T_OPEN_PARENTHESIS T_CLOSE_PARENTHESIS T_SEMICOLON T_WHITESPACE + } +// T_WHITESPACE T_CLOSE_CURLY_BRACKET T_WHITESPACE +} +// T_CLOSE_CURLY_BRACKET T_WHITESPACE \ No newline at end of file