From 3fb03c9d57810c160ea5fdfa9d003d92c228b915 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 6 Aug 2026 13:36:24 +0200 Subject: [PATCH] Fix ConsistentIndent mis-indenting PHP 8.4 property hooks The sniff derived its expected indent from the size of a token's conditions map. PHP_CodeSniffer does not model property hooks as scopes: their braces carry bracket opener and closer metadata but contribute nothing to conditions, so every token inside a hook block reported the same depth as the property itself. Only the second hook actually tripped it. Lines preceded by an opening brace are let through as continuations, and the hook bodies and closing braces are skipped, which leaves just the line after the first hook's closing brace. That was enough for phpcbf to dedent set to the property's own level while get, the bodies and every brace stayed put, producing exactly the inconsistent indentation this sniff exists to prevent. Expected indent now also counts enclosing curly brace pairs that phpcs paired but did not map to a scope. That repairs the depth model rather than special casing one syntax. The ranges are collected in a single pass and cached per file alongside the token count, matching how arrow function scopes are already handled, so nothing walks the file per line. The fixtures gain a correctly indented hook pair, which must stay untouched, and a misindented one, which proves the sniff still sees inside hook bodies rather than going blind there. --- .../WhiteSpace/ConsistentIndentSniff.php | 89 ++++++++++++++++++- .../WhiteSpace/ConsistentIndentSniffTest.php | 2 +- tests/_data/ConsistentIndent/after.php | 16 ++++ tests/_data/ConsistentIndent/before.php | 16 ++++ 4 files changed, 118 insertions(+), 5 deletions(-) diff --git a/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php b/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php index ae230e7..3ae3ed1 100644 --- a/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php +++ b/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php @@ -65,7 +65,7 @@ public function process(File $phpcsFile, $stackPtr): void } // Get the expected indentation based on scope - $expectedIndent = $this->getExpectedIndent($tokens[$nextToken]); + $expectedIndent = $this->getExpectedIndent($phpcsFile, $nextToken, $tokens); // Skip anything that could be intentional (most things) if ($this->isInsideClosure($phpcsFile, $nextToken, $tokens)) { @@ -180,15 +180,18 @@ protected function getIndentLevel(File $phpcsFile, array $token): int /** * Get the expected indentation level based on scope. * - * @param array $token + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * @param int $stackPtr + * @param array> $tokens * * @return int */ - protected function getExpectedIndent(array $token): int + protected function getExpectedIndent(File $phpcsFile, int $stackPtr, array $tokens): int { + $token = $tokens[$stackPtr]; $conditions = $token['conditions']; - return count($conditions); + return count($conditions) + $this->getUnscopedBraceIndent($phpcsFile, $stackPtr, $tokens); } /** @@ -327,6 +330,17 @@ protected function isValidContinuation(int $prevToken, array $tokens): bool */ private static array $arrowFunctionScopesCache = []; + /** + * Per-file cache of paired curly-brace ranges that phpcs did not model as scopes. + * + * PHP 8.4 property hooks are one example: their braces have bracket + * opener/closer metadata, but they are not propagated through the + * `conditions` map. These ranges still affect block indentation. + * + * @var array}> + */ + private static array $unscopedBraceScopesCache = []; + /** * Check if the current position is inside a closure or arrow function. * @@ -403,6 +417,73 @@ protected function getArrowFunctionScopes(File $phpcsFile, array $tokens): array return $scopes; } + /** + * Count unscoped brace ranges enclosing the current token. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * @param int $stackPtr + * @param array> $tokens + * + * @return int + */ + protected function getUnscopedBraceIndent(File $phpcsFile, int $stackPtr, array $tokens): int + { + $indent = 0; + foreach ($this->getUnscopedBraceScopes($phpcsFile, $tokens) as $range) { + if ($stackPtr > $range[0] && $stackPtr < $range[1]) { + $indent++; + } + } + + return $indent; + } + + /** + * Build (and cache per file) unscoped paired curly-brace ranges. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * @param array> $tokens + * + * @return array + */ + protected function getUnscopedBraceScopes(File $phpcsFile, array $tokens): array + { + $cacheKey = $phpcsFile->getFilename(); + $tokenCount = count($tokens); + if ( + isset(self::$unscopedBraceScopesCache[$cacheKey]) + && self::$unscopedBraceScopesCache[$cacheKey]['count'] === $tokenCount + ) { + return self::$unscopedBraceScopesCache[$cacheKey]['scopes']; + } + + $scopeOpeners = []; + foreach ($tokens as $token) { + if (isset($token['scope_opener'])) { + $scopeOpeners[$token['scope_opener']] = true; + } + } + + $scopes = []; + foreach ($tokens as $stackPtr => $token) { + if ($token['code'] !== T_OPEN_CURLY_BRACKET) { + continue; + } + if (!isset($token['bracket_closer']) || isset($scopeOpeners[$stackPtr])) { + continue; + } + + $scopes[] = [$stackPtr, $token['bracket_closer']]; + } + + self::$unscopedBraceScopesCache[$cacheKey] = [ + 'count' => $tokenCount, + 'scopes' => $scopes, + ]; + + return $scopes; + } + /** * Check if the current position is inside an array where indentation tracking may be unreliable. * This includes multi-dimensional arrays and arrays with closures as values. diff --git a/tests/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniffTest.php b/tests/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniffTest.php index 6000efd..873f570 100644 --- a/tests/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniffTest.php +++ b/tests/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniffTest.php @@ -17,7 +17,7 @@ class ConsistentIndentSniffTest extends TestCase */ public function testConsistentIndentSniffer(): void { - $this->assertSnifferFindsErrors(new ConsistentIndentSniff(), 2); + $this->assertSnifferFindsErrors(new ConsistentIndentSniff(), 3); } /** diff --git a/tests/_data/ConsistentIndent/after.php b/tests/_data/ConsistentIndent/after.php index 6f54790..c551819 100644 --- a/tests/_data/ConsistentIndent/after.php +++ b/tests/_data/ConsistentIndent/after.php @@ -4,6 +4,22 @@ class FixMe { + public string $label = 'x' { + get { + return $this->label; + } + set { + $this->label = $value; + } + } + + public string $misindentedLabel = 'x' { + set { + $this->misindentedLabel = $value; + $this->misindentedLabel = trim($this->misindentedLabel); + } + } + public function orphanedIndentExample($params): array { if (!isset($params['url']['unresolved'])) { diff --git a/tests/_data/ConsistentIndent/before.php b/tests/_data/ConsistentIndent/before.php index 49b8c09..a246723 100644 --- a/tests/_data/ConsistentIndent/before.php +++ b/tests/_data/ConsistentIndent/before.php @@ -4,6 +4,22 @@ class FixMe { + public string $label = 'x' { + get { + return $this->label; + } + set { + $this->label = $value; + } + } + + public string $misindentedLabel = 'x' { + set { + $this->misindentedLabel = $value; + $this->misindentedLabel = trim($this->misindentedLabel); + } + } + public function orphanedIndentExample($params): array { if (!isset($params['url']['unresolved'])) {