diff --git a/.gitignore b/.gitignore index 6ad5d12..b9e6100 100755 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ .build/ .idea .php-cs-fixer.cache +.phpunit.result.cache composer.lock vendor diff --git a/config.php b/config.php index 6689c58..27d2d6c 100644 --- a/config.php +++ b/config.php @@ -87,6 +87,7 @@ function config(Finder $finder, array $ruleOverrides = []): Config 'less_and_greater' => false, ], + Fixer\LineBreakBeforeThrowExpressionFixer::NAME => true, Fixer\PhpdocSimplifyArrayKeyFixer::NAME => true, PhpCsFixerCustomFixers\Fixer\ConstructorEmptyBracesFixer::name() => true, @@ -97,6 +98,7 @@ function config(Finder $finder, array $ruleOverrides = []): Config return (new Config()) ->registerCustomFixers(new PhpCsFixerCustomFixers\Fixers()) ->registerCustomFixers([ + new Fixer\LineBreakBeforeThrowExpressionFixer(), new Fixer\PhpdocSimplifyArrayKeyFixer(), ]) ->setFinder($finder) diff --git a/phpunit.xml b/phpunit.xml index f9735b5..db41662 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -8,6 +8,10 @@ tests + tests/Fixer/Php80 + + + tests/Fixer/Php80 diff --git a/src/Fixer/LineBreakBeforeThrowExpressionFixer.php b/src/Fixer/LineBreakBeforeThrowExpressionFixer.php new file mode 100644 index 0000000..668ab64 --- /dev/null +++ b/src/Fixer/LineBreakBeforeThrowExpressionFixer.php @@ -0,0 +1,209 @@ +fetchNullable() ?? throw new \RuntimeException('message'); + + PHP + ), + ] + ); + } + + public function isCandidate(Tokens $tokens): bool + { + return $tokens->isTokenKindFound(T_THROW); + } + + public function getPriority(): int + { + // Run before operator_linebreak (priority 0) + return 1; + } + + protected function applyFix(\SplFileInfo $file, Tokens $tokens): void + { + for ($index = $tokens->count() - 1; $index > 0; --$index) { + if (! $tokens[$index]->isGivenKind(T_THROW)) { + continue; + } + + $operatorIndex = $this->findPrecedingThrowOperator($tokens, $index); + if ($operatorIndex === null) { + continue; + } + + $this->ensureLineBreakBeforeOperator($tokens, $operatorIndex); + } + } + + /** + * Find `??` or `?:` operator immediately before the throw token. + * + * @return int|null The index of the operator (T_COALESCE or `?` for elvis), or null + */ + private function findPrecedingThrowOperator(Tokens $tokens, int $throwIndex): ?int + { + $prevIndex = $tokens->getPrevMeaningfulToken($throwIndex); + if ($prevIndex === null) { + return null; + } + + // Check for ?? (T_COALESCE) + if ($tokens[$prevIndex]->isGivenKind(T_COALESCE)) { + return $prevIndex; + } + + // Check for ?: (elvis operator - two separate tokens) + if ($tokens[$prevIndex]->equals(':')) { + $questionIndex = $tokens->getPrevMeaningfulToken($prevIndex); + if ($questionIndex !== null && $tokens[$questionIndex]->equals('?')) { + return $questionIndex; + } + } + + return null; + } + + private function ensureLineBreakBeforeOperator(Tokens $tokens, int $operatorIndex): void + { + $prevMeaningfulIndex = $tokens->getPrevMeaningfulToken($operatorIndex); + if ($prevMeaningfulIndex === null) { + return; + } + + // If the preceding expression is a multi-line block, don't add another line break + $blockType = $this->getBlockType($tokens[$prevMeaningfulIndex]); + if ($blockType !== null) { + $openIndex = $tokens->findBlockStart($blockType, $prevMeaningfulIndex); + if ($this->hasNewlineBetween($tokens, $openIndex, $prevMeaningfulIndex)) { + return; + } + } + + $statementStart = $this->findStatementStart($tokens, $operatorIndex); + $expressionAlreadyMultiline = $this->hasNewlineBetween($tokens, $statementStart, $prevMeaningfulIndex); + + if ($expressionAlreadyMultiline) { + // Use the existing indentation level from the multiline expression + $indentation = $this->getIndentAt($tokens, $operatorIndex); + } else { + // Add one indent level from the statement start + $indentation = $this->getIndentAt($tokens, $statementStart) . $this->whitespacesConfig->getIndent(); + } + + $newWhitespace = $this->whitespacesConfig->getLineEnding() . $indentation; + + $whitespaceIndex = $operatorIndex - 1; + if ($tokens[$whitespaceIndex]->isWhitespace()) { + if ($tokens[$whitespaceIndex]->getContent() === $newWhitespace) { + return; + } + + $tokens[$whitespaceIndex] = new Token([T_WHITESPACE, $newWhitespace]); + } else { + $tokens->insertAt($operatorIndex, new Token([T_WHITESPACE, $newWhitespace])); + } + } + + private function hasNewlineBetween(Tokens $tokens, int $startIndex, int $endIndex): bool + { + for ($i = $startIndex + 1; $i < $endIndex; ++$i) { + $token = $tokens[$i]; + if ($token->isWhitespace() && str_contains($token->getContent(), "\n")) { + return true; + } + } + + return false; + } + + /** Get the block type for closing braces, or null if not a closing brace. */ + private function getBlockType(Token $token): ?int + { + if ($token->equals(')')) { + return Tokens::BLOCK_TYPE_PARENTHESIS_BRACE; + } + + if ($token->equals('}')) { + return Tokens::BLOCK_TYPE_CURLY_BRACE; + } + + return null; + } + + // TODO: Replace with IndentationTrait once we require php-cs-fixer ^3.87.0 + private function getIndentAt(Tokens $tokens, int $index): string + { + for ($i = $index - 1; $i >= 0; --$i) { + $token = $tokens[$i]; + $content = $token->getContent(); + + if (Preg::match('/\R(\h*)$/', $content, $matches)) { + return $matches[1]; + } + } + + return ''; + } + + private function findStatementStart(Tokens $tokens, int $index): int + { + $nestingLevel = 0; + + for ($i = $index - 1; $i >= 0; --$i) { + $token = $tokens[$i]; + + // Track nesting to ignore commas/arrows inside parentheses, brackets, or braces + // CT::T_ARRAY_SQUARE_BRACE_* are custom tokens for array literal brackets + if ($token->equalsAny([')', ']']) || $token->isGivenKind(CT::T_ARRAY_SQUARE_BRACE_CLOSE)) { + ++$nestingLevel; + } elseif ($token->equalsAny(['(', '[']) || $token->isGivenKind(CT::T_ARRAY_SQUARE_BRACE_OPEN)) { + --$nestingLevel; + } elseif ($token->equals('}')) { + if ($nestingLevel === 0) { + return $tokens->getNextMeaningfulToken($i) ?? $index; + } + ++$nestingLevel; + } elseif ($token->equals('{')) { + return $tokens->getNextMeaningfulToken($i) ?? $index; + } elseif ($nestingLevel === 0) { + if ($token->equalsAny([';', ',']) || $token->isGivenKind([T_OPEN_TAG, T_DOUBLE_ARROW])) { + return $tokens->getNextMeaningfulToken($i) ?? $index; + } + } + } + + return 0; + } +} diff --git a/tests/Fixer/Php80/LineBreakBeforeThrowExpressionFixerTest.php b/tests/Fixer/Php80/LineBreakBeforeThrowExpressionFixerTest.php new file mode 100644 index 0000000..e9c1c12 --- /dev/null +++ b/tests/Fixer/Php80/LineBreakBeforeThrowExpressionFixerTest.php @@ -0,0 +1,510 @@ +fixer = new LineBreakBeforeThrowExpressionFixer(); + } + + /** @dataProvider provideFixCases */ + public function testFix(string $expected, ?string $input = null): void + { + $input ??= $expected; + + $tokens = Tokens::fromCode($input); + $this->fixer->fix(new \SplFileInfo(__FILE__), $tokens); + $actual = $tokens->generateCode(); + + self::assertSame($expected, $actual); + + $tokens = Tokens::fromCode($expected); + $this->fixer->fix(new \SplFileInfo(__FILE__), $tokens); + self::assertSame($expected, $tokens->generateCode()); + } + + /** @return iterable */ + public static function provideFixCases(): iterable + { + yield 'null coalesce throw on single line' => [ + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchNullable() ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'elvis throw on single line' => [ + <<<'PHP' + fetchFalsy() + ?: throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchFalsy() ?: throw new \RuntimeException('message'); + PHP, + ]; + + yield 'null coalesce throw with missing indent' => [ + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'elvis throw with missing indent' => [ + <<<'PHP' + fetchFalsy() + ?: throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchFalsy() + ?: throw new \RuntimeException('message'); + PHP, + ]; + + yield 'null coalesce throw with extra indent' => [ + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'elvis throw with extra indent' => [ + <<<'PHP' + fetchFalsy() + ?: throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchFalsy() + ?: throw new \RuntimeException('message'); + PHP, + ]; + + yield 'already multiline null coalesce throw - no change' => [ + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'already multiline elvis throw - no change' => [ + <<<'PHP' + fetchFalsy() + ?: throw new \RuntimeException('message'); + PHP, + ]; + + yield 'regular null coalesce without throw - no change' => [ + <<<'PHP' + fetchNullable() ?? 'default'; + PHP, + ]; + + yield 'regular elvis without throw - no change' => [ + <<<'PHP' + fetchFalsy() ?: 'default'; + PHP, + ]; + + yield 'exception with arguments' => [ + <<<'PHP' + findUser($id) + ?? throw new \InvalidArgumentException("User {$id} not found"); + PHP, + <<<'PHP' + findUser($id) ?? throw new \InvalidArgumentException("User {$id} not found"); + PHP, + ]; + + yield 'method chain' => [ + <<<'PHP' + getRepository()->find($id) + ?? throw new \RuntimeException('Not found'); + PHP, + <<<'PHP' + getRepository()->find($id) ?? throw new \RuntimeException('Not found'); + PHP, + ]; + + yield 'return statement with null coalesce throw' => [ + <<<'PHP' + cache->get($key) + ?? throw new \RuntimeException('Cache miss'); + PHP, + <<<'PHP' + cache->get($key) ?? throw new \RuntimeException('Cache miss'); + PHP, + ]; + + yield 'nested in method' => [ + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + + return $result; + } + } + PHP, + <<<'PHP' + fetchNullable() ?? throw new \RuntimeException('message'); + + return $result; + } + } + PHP, + ]; + + yield 'multiple statements' => [ + <<<'PHP' + [ + <<<'PHP' + bar() + ->baz() + ->qux() + ?? throw new \RuntimeException('a'); + PHP, + <<<'PHP' + bar() + ->baz() + ->qux() ?? throw new \RuntimeException('a'); + PHP, + ]; + + yield 'regular throw statement - no change' => [ + <<<'PHP' + [ + <<<'PHP' + [ + <<<'PHP' + [ + <<<'PHP' + 'foo', + default => 'asdf', + } ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'nested throw expression - no change' => [ + <<<'PHP' + setting->toMPSetting() + ?? throw new DefaultMPSettingMissingException( + $this->pnl454_name + ?? throw new \Exception("pnl454_name missing for Ngs454Panel {$this->id}."), + ); + PHP, + ]; + + yield 'no whitespace before operator' => [ + <<<'PHP' + fetchNullable() + ?? throw new \RuntimeException('message'); + PHP, + <<<'PHP' + fetchNullable()?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'array access' => [ + <<<'PHP' + [ + <<<'PHP' + find($id) + ?? throw UserNotFoundException::forId($id); + PHP, + <<<'PHP' + find($id) ?? throw UserNotFoundException::forId($id); + PHP, + ]; + + yield 'chained null coalesce - only last gets line break' => [ + <<<'PHP' + [ + <<<'PHP' + getUser()?->getProfile()?->getName() + ?? throw new \RuntimeException('message'); + PHP, + <<<'PHP' + getUser()?->getProfile()?->getName() ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'throw existing exception variable' => [ + <<<'PHP' + [ + <<<'PHP' + [ + <<<'PHP' + [ + <<<'PHP' + $this->findUser($id) + ?? throw new \RuntimeException('User not found'), + 'order' => $this->findOrder($id) + ?? throw new \RuntimeException('Order not found'), + }; + PHP, + <<<'PHP' + $this->findUser($id) ?? throw new \RuntimeException('User not found'), + 'order' => $this->findOrder($id) ?? throw new \RuntimeException('Order not found'), + }; + PHP, + ]; + + yield 'arrow function with throw expression' => [ + <<<'PHP' + $x + ?? throw new \RuntimeException('message'); + PHP, + <<<'PHP' + $x ?? throw new \RuntimeException('message'); + PHP, + ]; + + yield 'static property access' => [ + <<<'PHP' + [ + <<<'PHP' + [ + <<<'PHP' + [ + <<<'PHP' + find($id) + ?? throw new \RuntimeException('Not found'); + }; + PHP, + <<<'PHP' + find($id) ?? throw new \RuntimeException('Not found'); + }; + PHP, + ]; + + yield 'property access with variable property name' => [ + <<<'PHP' + $property + ?? throw new \RuntimeException('Property not found'); + PHP, + <<<'PHP' + $property ?? throw new \RuntimeException('Property not found'); + PHP, + ]; + + yield 'deeply nested class' => [ + <<<'PHP' + $x + ?? throw new \RuntimeException('message'); + }; + } + } + PHP, + <<<'PHP' + $x ?? throw new \RuntimeException('message'); + }; + } + } + PHP, + ]; + } +}