From 3a22580b00ab7a33c1e79da1115ef95bab48f4c8 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sun, 25 Oct 2026 14:55:32 +0100 Subject: [PATCH] Add OperatorPosition sniff for wrapped operators Universal.Operators.ConcatPosition only covers `.`. The new PhpCollective.Operators.OperatorPosition sniff applies the same rule to `&&`, `||`, `and`, `or`, `xor`, `??`, the ternary `?` and `:` (with `?:` moved as one unit) and binary `+ - * / % **`: when the operator ends a line and the expression continues on the next, it moves to the start of the continuation line. Unary plus/minus, return-type and case colons, nullable `?` and operators followed by a comment (reported, not fixed) are left alone. The repository's own sources are updated to pass the new rule. On the 206 files of the cakephp PER-CS PR, the fixer output matches the PR's php-cs-fixer result for 171 of the 175 wrapped non-concat expressions. The rest are an `=` moved to the line start (not covered), a `phpcs:disable` block and a `??` that stood alone on its own line. --- .../Classes/Psr4/AutoloadabilityInspector.php | 18 +-- .../Psr4/AutoloadabilityInspectorsFactory.php | 6 +- .../DisallowArrayTypeHintSyntaxSniff.php | 4 +- .../Sniffs/Commenting/TypeHintSniff.php | 18 +-- .../Formatting/ArrayDeclarationSniff.php | 16 +- .../Operators/OperatorPositionSniff.php | 146 ++++++++++++++++++ PhpCollective/ruleset.xml | 1 + docs/sniffs.md | 5 +- .../Operators/OperatorPositionSniffTest.php | 26 ++++ tests/_data/OperatorPosition/after.php | 94 +++++++++++ tests/_data/OperatorPosition/before.php | 94 +++++++++++ 11 files changed, 395 insertions(+), 33 deletions(-) create mode 100644 PhpCollective/Sniffs/Operators/OperatorPositionSniff.php create mode 100644 tests/PhpCollective/Sniffs/Operators/OperatorPositionSniffTest.php create mode 100644 tests/_data/OperatorPosition/after.php create mode 100644 tests/_data/OperatorPosition/before.php diff --git a/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspector.php b/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspector.php index 1620587..a75d976 100644 --- a/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspector.php +++ b/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspector.php @@ -42,9 +42,9 @@ public function __construct(string $baseDirectory, string $namespacePrefix) public function inspect( ClassFileUnderInspection $classFile, ): InspectionResult { - return $this->classFileIsUnderBaseDirectory($classFile) ? - $this->inspectAutoloadability($classFile) : - new PSR4UnrelatedClass(); + return $this->classFileIsUnderBaseDirectory($classFile) + ? $this->inspectAutoloadability($classFile) + : new PSR4UnrelatedClass(); } /** @@ -69,9 +69,9 @@ protected function inspectAutoloadability( $expectedClassName = $this->guessExpectedClassName($classFile); $actualClassName = $classFile->getClassName(); - return $expectedClassName === $actualClassName ? - new AutoloadableClass() : - new NonAutoloadableClass($expectedClassName, $actualClassName); + return $expectedClassName === $actualClassName + ? new AutoloadableClass() + : new NonAutoloadableClass($expectedClassName, $actualClassName); } /** @@ -114,9 +114,9 @@ protected function guessRelativeClassName(string $relativeFileName): string { $basename = basename($relativeFileName); $filename = pathinfo($relativeFileName, PATHINFO_FILENAME); - $dirname = $basename === $relativeFileName ? - '' : - pathinfo($relativeFileName, PATHINFO_DIRNAME) . '/'; + $dirname = $basename === $relativeFileName + ? '' + : pathinfo($relativeFileName, PATHINFO_DIRNAME) . '/'; return str_replace('/', '\\', $dirname) . $filename; } diff --git a/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspectorsFactory.php b/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspectorsFactory.php index 05a082b..4723a68 100644 --- a/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspectorsFactory.php +++ b/PhpCollective/Sniffs/Classes/Psr4/AutoloadabilityInspectorsFactory.php @@ -102,9 +102,9 @@ protected static function resolveComposerJsonPath( ?string $basePath, string $composerJsonPath, ): string { - return $basePath === null ? - $composerJsonPath : - $basePath . '/' . $composerJsonPath; + return $basePath === null + ? $composerJsonPath + : $basePath . '/' . $composerJsonPath; } /** diff --git a/PhpCollective/Sniffs/Commenting/DisallowArrayTypeHintSyntaxSniff.php b/PhpCollective/Sniffs/Commenting/DisallowArrayTypeHintSyntaxSniff.php index b402bb1..725a1b5 100644 --- a/PhpCollective/Sniffs/Commenting/DisallowArrayTypeHintSyntaxSniff.php +++ b/PhpCollective/Sniffs/Commenting/DisallowArrayTypeHintSyntaxSniff.php @@ -510,8 +510,8 @@ protected function containsArrayTypeNode(array $types): bool */ protected function isArrayTypeNode(TypeNode $typeNode): bool { - return $typeNode instanceof ArrayTypeNode && - ($typeNode->type instanceof IdentifierTypeNode || $typeNode->type instanceof ArrayTypeNode); + return $typeNode instanceof ArrayTypeNode + && ($typeNode->type instanceof IdentifierTypeNode || $typeNode->type instanceof ArrayTypeNode); } /** diff --git a/PhpCollective/Sniffs/Commenting/TypeHintSniff.php b/PhpCollective/Sniffs/Commenting/TypeHintSniff.php index 4ce8993..8b2aa00 100644 --- a/PhpCollective/Sniffs/Commenting/TypeHintSniff.php +++ b/PhpCollective/Sniffs/Commenting/TypeHintSniff.php @@ -120,8 +120,8 @@ public function process(File $phpcsFile, $stackPtr): void foreach ($tokens[$stackPtr]['comment_tags'] as $tag) { if ( - $tokens[$tag + 2]['code'] !== T_DOC_COMMENT_STRING || - !in_array($tokens[$tag]['content'], static::$typeHintTags, true) + $tokens[$tag + 2]['code'] !== T_DOC_COMMENT_STRING + || !in_array($tokens[$tag]['content'], static::$typeHintTags, true) ) { continue; } @@ -199,8 +199,8 @@ public function process(File $phpcsFile, $stackPtr): void foreach ($tokens[$stackPtr]['comment_tags'] as $key => $tag) { if ( - $tokens[$tag + 2]['code'] !== T_DOC_COMMENT_STRING || - !in_array($tokens[$tag]['content'], static::$typeHintTags, true) + $tokens[$tag + 2]['code'] !== T_DOC_COMMENT_STRING + || !in_array($tokens[$tag]['content'], static::$typeHintTags, true) ) { continue; } @@ -278,9 +278,9 @@ protected function processNativeTypeHints(File $phpcsFile, int $stackPtr): void $methodProperties = $phpcsFile->getMethodProperties($stackPtr); if ( - $methodProperties['return_type'] === '' || - $methodProperties['return_type_token'] === false || - $methodProperties['return_type_end_token'] === false + $methodProperties['return_type'] === '' + || $methodProperties['return_type_token'] === false + || $methodProperties['return_type_end_token'] === false ) { return; } @@ -359,8 +359,8 @@ protected function getSortedTypeHint(array $types): string $sortName = 'array'; } } elseif ( - $type instanceof ArrayShapeNode || - ($type instanceof GenericTypeNode && $type->type->name === 'array') + $type instanceof ArrayShapeNode + || ($type instanceof GenericTypeNode && $type->type->name === 'array') ) { $sortName = 'array'; } diff --git a/PhpCollective/Sniffs/Formatting/ArrayDeclarationSniff.php b/PhpCollective/Sniffs/Formatting/ArrayDeclarationSniff.php index 24b6203..02aef17 100644 --- a/PhpCollective/Sniffs/Formatting/ArrayDeclarationSniff.php +++ b/PhpCollective/Sniffs/Formatting/ArrayDeclarationSniff.php @@ -639,8 +639,8 @@ protected function processMultiLineIndentation(File $phpcsFile, int $arrayStart, // Look for the first properly indented item in this array to match its indentation for ($searchIdx = $arrayStart + 1; $searchIdx < $arrayEnd; $searchIdx++) { if ( - $tokens[$searchIdx]['line'] > $tokens[$arrayStart]['line'] && - !in_array($tokens[$searchIdx]['code'], Tokens::$emptyTokens, true) + $tokens[$searchIdx]['line'] > $tokens[$arrayStart]['line'] + && !in_array($tokens[$searchIdx]['code'], Tokens::$emptyTokens, true) ) { // Extract actual indentation from the line (only leading whitespace) $lineStart = $phpcsFile->findFirstOnLine([], $searchIdx); @@ -670,8 +670,8 @@ protected function processMultiLineIndentation(File $phpcsFile, int $arrayStart, $count = count($tokens); for ($detectIdx = 0; $detectIdx < $count; $detectIdx++) { if ( - $tokens[$detectIdx]['code'] === T_WHITESPACE && - $tokens[$detectIdx]['line'] !== $tokens[$detectIdx - 1]['line'] + $tokens[$detectIdx]['code'] === T_WHITESPACE + && $tokens[$detectIdx]['line'] !== $tokens[$detectIdx - 1]['line'] ) { $whitespace = $tokens[$detectIdx]['content']; if (str_contains($whitespace, "\t")) { @@ -707,10 +707,10 @@ protected function processMultiLineIndentation(File $phpcsFile, int $arrayStart, // Determine if we should use chunking for this line $useChunking = false; if ( - $this->nonAssocExceedAction === 'chunk' && - $this->maxNonAssocItemsPerLine > 0 && - $nonAssocCount > $this->maxNonAssocItemsPerLine && - !$hasAssociative + $this->nonAssocExceedAction === 'chunk' + && $this->maxNonAssocItemsPerLine > 0 + && $nonAssocCount > $this->maxNonAssocItemsPerLine + && !$hasAssociative ) { // Use chunking only for pure non-associative arrays $useChunking = true; diff --git a/PhpCollective/Sniffs/Operators/OperatorPositionSniff.php b/PhpCollective/Sniffs/Operators/OperatorPositionSniff.php new file mode 100644 index 0000000..233ff3a --- /dev/null +++ b/PhpCollective/Sniffs/Operators/OperatorPositionSniff.php @@ -0,0 +1,146 @@ +getTokens(); + $code = $tokens[$stackPtr]['code']; + + if (($code === T_PLUS || $code === T_MINUS) && Operators::isUnaryPlusMinus($phpcsFile, $stackPtr)) { + return; + } + + $previous = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true); + if ($previous === false || $tokens[$previous]['line'] !== $tokens[$stackPtr]['line']) { + return; + } + + if ($code === T_INLINE_ELSE && $tokens[$previous]['code'] === T_INLINE_THEN) { + return; + } + + $end = $stackPtr; + if ($code === T_INLINE_THEN) { + $following = $phpcsFile->findNext(Tokens::$emptyTokens, $stackPtr + 1, null, true); + if ($following !== false && $tokens[$following]['code'] === T_INLINE_ELSE) { + if ($tokens[$following]['line'] !== $tokens[$stackPtr]['line']) { + $this->processSplitShortTernary($phpcsFile, $stackPtr, $following); + + return; + } + $end = $following; + } + } + + $next = $phpcsFile->findNext(Tokens::$emptyTokens, $end + 1, null, true); + if ($next === false || $tokens[$next]['line'] === $tokens[$end]['line']) { + return; + } + + $hasComment = false; + for ($i = $stackPtr + 1; $i < $next; $i++) { + if (isset(Tokens::$commentTokens[$tokens[$i]['code']])) { + $hasComment = true; + + break; + } + } + + $operator = $end === $stackPtr ? $tokens[$stackPtr]['content'] : '?:'; + $message = 'Operator "%s" must be placed at the start of the continuation line'; + if ($hasComment) { + $phpcsFile->addError($message, $stackPtr, 'IncorrectWithComment', [$operator]); + + return; + } + + if (!$phpcsFile->addFixableError($message, $stackPtr, 'Incorrect', [$operator])) { + return; + } + + $phpcsFile->fixer->beginChangeset(); + for ($i = $stackPtr; $i <= $end; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + if ($tokens[$stackPtr - 1]['code'] === T_WHITESPACE) { + $phpcsFile->fixer->replaceToken($stackPtr - 1, ''); + } + if ($tokens[$end + 1]['code'] === T_WHITESPACE) { + $whitespace = $tokens[$end + 1]['content']; + $phpcsFile->fixer->replaceToken($end + 1, preg_replace('/^[ \t]+(?=\r?\n)/', '', $whitespace)); + } + $phpcsFile->fixer->addContentBefore($next, $operator . ' '); + $phpcsFile->fixer->endChangeset(); + } + + /** + * Handles `$a ?` followed by `: $b` on the next line by joining both into `?:` there. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * @param int $thenPtr + * @param int $elsePtr + * + * @return void + */ + protected function processSplitShortTernary(File $phpcsFile, int $thenPtr, int $elsePtr): void + { + $tokens = $phpcsFile->getTokens(); + $message = 'Operator "?:" must be placed at the start of the continuation line'; + + for ($i = $thenPtr + 1; $i < $elsePtr; $i++) { + if (isset(Tokens::$commentTokens[$tokens[$i]['code']])) { + $phpcsFile->addError($message, $thenPtr, 'IncorrectWithComment'); + + return; + } + } + + if (!$phpcsFile->addFixableError($message, $thenPtr, 'Incorrect')) { + return; + } + + $phpcsFile->fixer->beginChangeset(); + $phpcsFile->fixer->replaceToken($thenPtr, ''); + if ($tokens[$thenPtr - 1]['code'] === T_WHITESPACE) { + $phpcsFile->fixer->replaceToken($thenPtr - 1, ''); + } + $whitespace = $tokens[$thenPtr + 1]['code'] === T_WHITESPACE ? $tokens[$thenPtr + 1]['content'] : ''; + if ($whitespace !== '') { + $phpcsFile->fixer->replaceToken($thenPtr + 1, (string)preg_replace('/^[ \t]+(?=\r?\n)/', '', $whitespace)); + } + $phpcsFile->fixer->addContentBefore($elsePtr, '?'); + $phpcsFile->fixer->endChangeset(); + } +} diff --git a/PhpCollective/ruleset.xml b/PhpCollective/ruleset.xml index 1ebd80b..bf98959 100644 --- a/PhpCollective/ruleset.xml +++ b/PhpCollective/ruleset.xml @@ -296,6 +296,7 @@ + diff --git a/docs/sniffs.md b/docs/sniffs.md index e85df3c..0cb8cbd 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 252 sniffs Generic (27 sniffs) ------------------- @@ -48,7 +48,7 @@ PEAR (4 sniffs) - PEAR.Functions.ValidDefaultValue - PEAR.NamingConventions.ValidClassName -PhpCollective (87 sniffs) +PhpCollective (88 sniffs) ------------------------- - PhpCollective.Arrays.ArrayBracketSpacing - PhpCollective.Arrays.DisallowImplicitArrayCreation @@ -107,6 +107,7 @@ PhpCollective (87 sniffs) - PhpCollective.Namespaces.FunctionNamespace - PhpCollective.Namespaces.UseStatement - PhpCollective.Namespaces.UseWithAliasing +- PhpCollective.Operators.OperatorPosition - PhpCollective.Operators.OperatorSpacing - PhpCollective.PHP.DeclareStrictTypes - PhpCollective.PHP.DisallowFunctions diff --git a/tests/PhpCollective/Sniffs/Operators/OperatorPositionSniffTest.php b/tests/PhpCollective/Sniffs/Operators/OperatorPositionSniffTest.php new file mode 100644 index 0000000..af17aff --- /dev/null +++ b/tests/PhpCollective/Sniffs/Operators/OperatorPositionSniffTest.php @@ -0,0 +1,26 @@ +assertSnifferFindsFixableErrors(new OperatorPositionSniff(), 28, 27); + $commentErrors = array_values($errors[57]); + $this->assertSame('PhpCollective.Operators.OperatorPosition.IncorrectWithComment', $commentErrors[0][0]['source']); + } + + public function testFixesOperators(): void + { + $this->assertSnifferCanFixErrors(new OperatorPositionSniff(), 27); + } +} diff --git a/tests/_data/OperatorPosition/after.php b/tests/_data/OperatorPosition/after.php new file mode 100644 index 0000000..c209889 --- /dev/null +++ b/tests/_data/OperatorPosition/after.php @@ -0,0 +1,94 @@ + 'one', + default => 'other', +}; +$splitShort = $a + ?: $b; diff --git a/tests/_data/OperatorPosition/before.php b/tests/_data/OperatorPosition/before.php new file mode 100644 index 0000000..0717230 --- /dev/null +++ b/tests/_data/OperatorPosition/before.php @@ -0,0 +1,94 @@ + 'one', + default => 'other', +}; +$splitShort = $a ? + : $b;