Skip to content

Commit

Permalink
Resolve TODOs, cover edge cases, add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
erickskrauch committed Jan 19, 2024
1 parent 2d2e831 commit 0fe2ec5
Show file tree
Hide file tree
Showing 8 changed files with 323 additions and 81 deletions.
4 changes: 3 additions & 1 deletion .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
declare(strict_types=1);

$finder = PhpCsFixer\Finder::create()
->in(__DIR__);
->in(__DIR__)
->exclude('tests/ClassNotation/_data')
;

return Ely\CS\Config::create([
// Disable "parameters" and "match" to keep compatibility with PHP 7.4
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ Ensures that multiline if statement body curly brace placed on the right line.

Overridden and implemented methods must be sorted in the same order as they are defined in parent classes.

**Warning**: this fixer is implemented against the PHP-CS-Fixer principle and relies on runtime, classes autoloading and reflection. If dependencies are missing or the autoloader isn't configured correctly, the fixer will not be able to discover the order of methods in parents.

```diff
--- Original
+++ New
Expand Down
132 changes: 132 additions & 0 deletions src/Analyzer/ClassElementsAnalyzer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php
declare(strict_types=1);

namespace ErickSkrauch\PhpCsFixer\Analyzer;

use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Tokens;

/**
* Taken from the \PhpCsFixer\Fixer\ClassNotation\OrderedClassElementsFixer and simplified
*
* @phpstan-type AnalyzedClassElementType 'use_trait'|'case'|'constant'|'property'|'method'
* @phpstan-type AnalyzedClassElement array{
* start: int,
* visibility: string,
* abstract: bool,
* static: bool,
* readonly: bool,
* type: AnalyzedClassElementType,
* name: string,
* end: int,
* }
*/
final class ClassElementsAnalyzer {

/**
* @return list<AnalyzedClassElement>
*/
public function getClassElements(Tokens $tokens, int $classOpenBraceIndex): array {
static $elementTokenKinds = [CT::T_USE_TRAIT, T_CASE, T_CONST, T_VARIABLE, T_FUNCTION];

$startIndex = $classOpenBraceIndex + 1;
$elements = [];

while (true) {
$element = [
'start' => $startIndex,
'visibility' => 'public',
'abstract' => false,
'static' => false,
'readonly' => false,
];

for ($i = $startIndex; ; ++$i) {
$token = $tokens[$i];

// class end
if ($token->equals('}')) {
return $elements; // @phpstan-ignore return.type
}

if ($token->isGivenKind(T_ABSTRACT)) {
$element['abstract'] = true;

continue;
}

if ($token->isGivenKind(T_STATIC)) {
$element['static'] = true;

continue;
}

if (defined('T_READONLY') && $token->isGivenKind(T_READONLY)) {
$element['readonly'] = true;
}

if ($token->isGivenKind([T_PROTECTED, T_PRIVATE])) {
$element['visibility'] = strtolower($token->getContent());

continue;
}

if (!$token->isGivenKind($elementTokenKinds)) {
continue;
}

$element['type'] = $this->detectElementType($tokens, $i);
if ($element['type'] === 'property') {
$element['name'] = $tokens[$i]->getContent();
} elseif (in_array($element['type'], ['use_trait', 'case', 'constant', 'method', 'magic', 'construct', 'destruct'], true)) {
$element['name'] = $tokens[$tokens->getNextMeaningfulToken($i)]->getContent();
}

$element['end'] = $this->findElementEnd($tokens, $i);

break;
}

$elements[] = $element;
$startIndex = $element['end'] + 1; // @phpstan-ignore offsetAccess.notFound
}
}

/**
* @phpstan-return AnalyzedClassElementType
*/
private function detectElementType(Tokens $tokens, int $index): string {
$token = $tokens[$index];
if ($token->isGivenKind(CT::T_USE_TRAIT)) {
return 'use_trait';
}

if ($token->isGivenKind(T_CASE)) {
return 'case';
}

if ($token->isGivenKind(T_CONST)) {
return 'constant';
}

if ($token->isGivenKind(T_VARIABLE)) {
return 'property';
}

return 'method';
}

private function findElementEnd(Tokens $tokens, int $index): int {
$endIndex = $tokens->getNextTokenOfKind($index, ['{', ';']);
if ($tokens[$endIndex]->equals('{')) {
$endIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_CURLY_BRACE, $endIndex);
}

for (++$endIndex; $tokens[$endIndex]->isWhitespace(" \t") || $tokens[$endIndex]->isComment(); ++$endIndex);

--$endIndex;

return $tokens[$endIndex]->isWhitespace() ? $endIndex - 1 : $endIndex;
}

}
11 changes: 2 additions & 9 deletions src/Analyzer/ClassNameAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer;
use PhpCsFixer\Tokenizer\Tokens;

// TODO: better naming
// TODO: cover with tests
final class ClassNameAnalyzer {

private NamespacesAnalyzer $namespacesAnalyzer;
Expand Down Expand Up @@ -58,13 +56,8 @@ private function readClassName(Tokens $tokens, int $classNameStart): string {
$className = '';
$index = $classNameStart;
do {
$token = $tokens[$index];
if ($token->isWhitespace()) {
continue;
}

$className .= $token->getContent();
} while ($tokens[++$index]->isGivenKind([T_STRING, T_NS_SEPARATOR, T_WHITESPACE]));
$className .= $tokens[$index]->getContent();
} while ($tokens[++$index]->isGivenKind([T_STRING, T_NS_SEPARATOR]));

return $className;
}
Expand Down
112 changes: 41 additions & 71 deletions src/ClassNotation/OrderedOverridesFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,19 @@
namespace ErickSkrauch\PhpCsFixer\ClassNotation;

use ErickSkrauch\PhpCsFixer\AbstractFixer;
use ErickSkrauch\PhpCsFixer\Analyzer\ClassElementsAnalyzer;
use ErickSkrauch\PhpCsFixer\Analyzer\ClassNameAnalyzer;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
use PhpCsFixer\Tokenizer\Tokens;
use ReflectionClass;
use ReflectionException;
use SplFileInfo;
use SplStack;

/**
* @phpstan-type MethodData array{
* name: string,
* start: int,
* end: int,
* }
* @phpstan-import-type AnalyzedClassElement from \ErickSkrauch\PhpCsFixer\Analyzer\ClassElementsAnalyzer
*/
final class OrderedOverridesFixer extends AbstractFixer {

Expand All @@ -27,9 +25,15 @@ final class OrderedOverridesFixer extends AbstractFixer {
*/
private ClassNameAnalyzer $classNameAnalyzer;

/**
* @readonly
*/
private ClassElementsAnalyzer $classElementsAnalyzer;

public function __construct() {
parent::__construct();
$this->classNameAnalyzer = new ClassNameAnalyzer();
$this->classElementsAnalyzer = new ClassElementsAnalyzer();
}

public function isCandidate(Tokens $tokens): bool {
Expand All @@ -56,14 +60,12 @@ public function serialize() {}
/**
* Must run before OrderedClassElementsFixer
* Must run after OrderedInterfacesFixer TODO: it's invariant right now: x < 0, but x > 65
* see https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/issues/7760
*/
public function getPriority(): int {
return 75;
}

/**
* @throws \ReflectionException
*/
protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
for ($i = 1, $count = $tokens->count(); $i < $count; ++$i) {
$classToken = $tokens[$i];
Expand All @@ -81,7 +83,12 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
}

foreach ($extensions as $className) {
$classReflection = new ReflectionClass($className);
try {
$classReflection = new ReflectionClass($className);
} catch (ReflectionException $e) { // @phpstan-ignore catch.neverThrown
continue;
}

$parents = $this->getClassParents($classReflection, new SplStack());
foreach ($parents as $parent) {
foreach ($parent->getMethods() as $method) {
Expand All @@ -102,56 +109,29 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
$classBodyStart = $tokens->getNextTokenOfKind($i, ['{']);
$classBodyEnd = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_CURLY_BRACE, $classBodyStart);

/** @var list<MethodData> $unsortedMethods */
$unsortedMethods = [];
// TODO: actually there still might be properties and traits in between methods declarations
for ($j = $classBodyStart; $j < $classBodyEnd; $j++) {
$functionToken = $tokens[$j];
if (!$functionToken->isGivenKind(T_FUNCTION)) {
continue;
}

$methodNameToken = $tokens[$tokens->getNextMeaningfulToken($j)];
// Ensure it's not an anonymous function
if ($methodNameToken->equals('(')) {
continue;
}

$methodName = $methodNameToken->getContent();

// Take the closest whitespace to the beginning of the method
$methodStart = $tokens->getPrevTokenOfKind($j, ['}', ';', '{']) + 1;
$methodEnd = $this->findElementEnd($tokens, $j);

$unsortedMethods[] = [
'name' => $methodName,
'start' => $methodStart,
'end' => $methodEnd,
];
}

$sortedMethods = $this->sortMethods($unsortedMethods, $methodsPriority);
if ($sortedMethods === $unsortedMethods) {
$unsortedElements = $this->classElementsAnalyzer->getClassElements($tokens, $classBodyStart);
$sortedElements = $this->sortElements($unsortedElements, $methodsPriority);
if ($sortedElements === $unsortedElements) {
continue;
}

$startIndex = $unsortedMethods[array_key_first($unsortedMethods)]['start'];
$endIndex = $unsortedMethods[array_key_last($unsortedMethods)]['end'];
$startIndex = $unsortedElements[array_key_first($unsortedElements)]['start'];
$endIndex = $unsortedElements[array_key_last($unsortedElements)]['end'];
$replaceTokens = [];
foreach ($sortedMethods as $method) {
foreach ($sortedElements as $method) {
for ($k = $method['start']; $k <= $method['end']; ++$k) {
$replaceTokens[] = clone $tokens[$k];
}
}

$tokens->overrideRange($startIndex, $endIndex, $replaceTokens);

$i = $endIndex;
$i = $classBodyEnd + 1;
}
}

/**
* @return array<int, class-string>
* @return list<class-string>
*/
private function getClassExtensions(Tokens $tokens, int $classTokenIndex, int $extensionType): array {
$extensionTokenIndex = $tokens->getNextTokenOfKind($classTokenIndex, [[$extensionType], '{']);
Expand Down Expand Up @@ -194,34 +174,20 @@ private function getClassParents(ReflectionClass $class, SplStack $stack): SplSt
}

/**
* Taken from the OrderedClassElementsFixer
*/
private function findElementEnd(Tokens $tokens, int $index): int {
$blockStart = $tokens->getNextTokenOfKind($index, ['{', ';']);
if ($tokens[$blockStart]->equals('{')) {
$blockEnd = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_CURLY_BRACE, $blockStart);
} else {
$blockEnd = $blockStart;
}

for (++$blockEnd; $tokens[$blockEnd]->isWhitespace(" \t") || $tokens[$blockEnd]->isComment(); ++$blockEnd);

--$blockEnd;

return $tokens[$blockEnd]->isWhitespace() ? $blockEnd - 1 : $blockEnd;
}

/**
* @phpstan-param list<MethodData> $methods
* @phpstan-param list<AnalyzedClassElement> $elements
* @phpstan-param array<string, non-negative-int> $methodsPriority
*
* @phpstan-return list<MethodData>
* @phpstan-return list<AnalyzedClassElement>
*/
private function sortMethods(array $methods, array $methodsPriority): array {
$count = count($methods);
private function sortElements(array $elements, array $methodsPriority): array {
$count = count($elements);
$targetPriority = $methodsPriority[array_key_last($methodsPriority)];
for ($i = 0; $i < $count; $i++) {
$a = $methods[$i];
$a = $elements[$i];
if ($a['type'] !== 'method') {
continue;
}

if (!isset($methodsPriority[$a['name']])) {
continue;
}
Expand All @@ -234,15 +200,19 @@ private function sortMethods(array $methods, array $methodsPriority): array {

do {
for ($j = $i + 1; $j < $count; $j++) {
$b = $methods[$j];
$b = $elements[$j];
if ($b['type'] !== 'method') {
continue;
}

if (!isset($methodsPriority[$b['name']])) {
continue;
}

$priorityB = $methodsPriority[$b['name']];
if ($priorityB === $targetPriority) {
$methods[$i] = $b;
$methods[$j] = $a;
$elements[$i] = $b;
$elements[$j] = $a;
$targetPriority--;

continue 3;
Expand All @@ -252,7 +222,7 @@ private function sortMethods(array $methods, array $methodsPriority): array {
}

// @phpstan-ignore return.type
return $methods;
return $elements;
}

}
Loading

0 comments on commit 0fe2ec5

Please sign in to comment.