[SOLID] Add AbstractChildlessUnusedClassesRector

This commit is contained in:
Tomas Votruba 2019-05-26 13:47:23 +02:00
parent 2883e39869
commit 0735925796
20 changed files with 423 additions and 25 deletions

View File

@ -2,3 +2,4 @@ services:
Rector\SOLID\Rector\Class_\FinalizeClassesWithoutChildrenRector: ~
Rector\SOLID\Rector\ClassConst\PrivatizeLocalClassConstantRector: ~
Rector\SOLID\Rector\Class_\AbstractChildlessUnusedClassesRector: ~

View File

@ -1,4 +1,4 @@
# All 291 Rectors Overview
# All 298 Rectors Overview
- [Projects](#projects)
- [General](#general)
@ -770,6 +770,26 @@ Joins concat of 2 strings
## CodingStyle
### `SplitDoubleAssignRector`
- class: `Rector\CodingStyle\Rector\Assign\SplitDoubleAssignRector`
Split multiple inline assigns to each own lines default value, to prevent undefined array issues
```diff
class SomeClass
{
public function run()
{
- $one = $two = 1;
+ $one = 1;
+ $two = 1;
}
}
```
<br>
### `ReturnArrayClassMethodToYieldRector`
- class: `Rector\CodingStyle\Rector\ClassMethod\ReturnArrayClassMethodToYieldRector`
@ -826,6 +846,30 @@ services:
<br>
### `CatchExceptionNameMatchingTypeRector`
- class: `Rector\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector`
Type and name of catch exception should match
```diff
class SomeClass
{
public function run()
{
try {
// ...
- } catch (SomeException $typoException) {
- $typoException->getMessage();
+ } catch (SomeException $someException) {
+ $someException->getMessage();
}
}
}
```
<br>
### `SymplifyQuoteEscapeRector`
- class: `Rector\CodingStyle\Rector\String_\SymplifyQuoteEscapeRector`
@ -892,6 +936,31 @@ Import fully qualified names to use statements
<br>
### `ArrayPropertyDefaultValueRector`
- class: `Rector\CodingStyle\Rector\Property\ArrayPropertyDefaultValueRector`
Array property should have default value, to prevent undefined array issues
```diff
class SomeClass
{
/**
* @var int[]
*/
- private $items;
+ private $items = [];
public function run()
{
foreach ($items as $item) {
}
}
}
```
<br>
### `RemoveUnusedAliasRector`
- class: `Rector\CodingStyle\Rector\Use_\RemoveUnusedAliasRector`
@ -910,6 +979,25 @@ Removes unused use aliases
<br>
### `FollowRequireByDirRector`
- class: `Rector\CodingStyle\Rector\Include_\FollowRequireByDirRector`
include/require should be followed by absolute path
```diff
class SomeClass
{
public function run()
{
- require 'autoload.php';
+ require __DIR__ . '/autoload.php';
}
}
```
<br>
### `ConsistentImplodeRector`
- class: `Rector\CodingStyle\Rector\FuncCall\ConsistentImplodeRector`
@ -948,6 +1036,27 @@ Changes redundant anonymous bool functions to simple calls
<br>
### `ConsistentPregDelimiterRector`
- class: `Rector\CodingStyle\Rector\FuncCall\ConsistentPregDelimiterRector`
Replace PREG delimiter with configured one
```diff
class SomeClass
{
public function run()
{
- preg_match('~value~', $value);
- preg_match_all('~value~im', $value);
+ preg_match('#value#', $value);
+ preg_match_all('#value#im', $value);
}
}
```
<br>
### `NullableCompareToNullRector`
- class: `Rector\CodingStyle\Rector\If_\NullableCompareToNullRector`
@ -995,6 +1104,24 @@ Separate constant and properties to own lines
<br>
### `VarConstantCommentRector`
- class: `Rector\CodingStyle\Rector\ClassConst\VarConstantCommentRector`
Constant should have a @var comment with type
```diff
class SomeClass
{
+ /**
+ * @var string
+ */
const HI = 'hi';
}
```
<br>
### `CompleteVarDocTypeConstantRector`
- class: `Rector\CodingStyle\Rector\ClassConst\CompleteVarDocTypeConstantRector`
@ -4249,6 +4376,30 @@ Finalize every class constant that is used only locally
<br>
### `AbstractChildlessUnusedClassesRector`
- class: `Rector\SOLID\Rector\Class_\AbstractChildlessUnusedClassesRector`
Classes that have no children nor are used, should have abstract
```diff
class SomeClass extends PossibleAbstractClass
{
}
-class PossibleAbstractClass
+abstract class PossibleAbstractClass
{
}
function run()
{
return new SomeClass();
}
```
<br>
### `FinalizeClassesWithoutChildrenRector`
- class: `Rector\SOLID\Rector\Class_\FinalizeClassesWithoutChildrenRector`

View File

@ -23,6 +23,13 @@ $someVariable[0]
```
<br>
#### `PhpParser\Node\Expr\ArrowFunction`
```php
fn() => 1
```
<br>
#### `PhpParser\Node\Expr\Assign`
```php

View File

@ -54,11 +54,11 @@ CODE_SAMPLE
*/
public function refactor(Node $node): ?Node
{
if (! $node->expr instanceof Node\Expr\Assign) {
if (! $node->expr instanceof Assign) {
return null;
}
$newAssign = new Node\Expr\Assign($node->var, $node->expr->expr);
$newAssign = new Assign($node->var, $node->expr->expr);
$this->addNodeAfterNode($node->expr, $node);

View File

@ -3,6 +3,7 @@
namespace Rector\CodingStyle\Rector\Catch_;
use PhpParser\Node;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\Catch_;
use Rector\CodingStyle\Naming\ClassNaming;
use Rector\PhpParser\NodeTraverser\CallableNodeTraverser;
@ -100,16 +101,13 @@ CODE_SAMPLE
return $node;
}
private function renameVariableInStmts(
Node\Stmt\Catch_ $catch,
string $oldVariableName,
string $newVariableName
): void {
private function renameVariableInStmts(Catch_ $catch, string $oldVariableName, string $newVariableName): void
{
$this->callableNodeTraverser->traverseNodesWithCallable($catch->stmts, function (Node $node) use (
$oldVariableName,
$newVariableName
): void {
if (! $node instanceof Node\Expr\Variable) {
if (! $node instanceof Variable) {
return;
}

View File

@ -4,7 +4,9 @@ namespace Rector\CodingStyle\Rector\Include_;
use Nette\Utils\Strings;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Concat;
use PhpParser\Node\Expr\Include_;
use PhpParser\Node\Scalar\MagicConst\Dir;
use PhpParser\Node\Scalar\String_;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
@ -65,7 +67,7 @@ CODE_SAMPLE
$this->removeExtraDotSlash($includedPath);
$this->prependSlashIfMissing($includedPath);
$node->expr = new Node\Expr\BinaryOp\Concat(new Node\Scalar\MagicConst\Dir(), $includedPath);
$node->expr = new Concat(new Dir(), $includedPath);
return $node;
}

View File

@ -3,6 +3,8 @@
namespace Rector\CodingStyle\Rector\Property;
use PhpParser\Node;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\PropertyProperty;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator;
@ -78,7 +80,7 @@ CODE_SAMPLE
return null;
}
/** @var Node\Stmt\Property $parentNode */
/** @var Property $parentNode */
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
$varTypeInfo = $this->docBlockManipulator->getVarTypeInfo($parentNode);
@ -90,7 +92,7 @@ CODE_SAMPLE
return null;
}
$node->default = new Node\Expr\Array_();
$node->default = new Array_();
return $node;
}

View File

@ -54,7 +54,8 @@ final class TypeToStringResolver
$arrayTypes = array_unique($arrayTypes);
return array_map(function (string $arrayType) {
/** @var string[] $arrayType */
return array_map(function (string $arrayType): string {
return $arrayType . '[]';
}, $arrayTypes);
}

View File

@ -0,0 +1,93 @@
<?php declare(strict_types=1);
namespace Rector\SOLID\Rector\Class_;
use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use Rector\NodeContainer\ParsedNodesByType;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
final class MakeUnusedClassesWithChildrenAbstractRector extends AbstractRector
{
/**
* @var ParsedNodesByType
*/
private $parsedNodesByType;
public function __construct(ParsedNodesByType $parsedNodesByType)
{
$this->parsedNodesByType = $parsedNodesByType;
}
public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Classes that have no children nor are used, should have abstract', [
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass extends PossibleAbstractClass
{
}
class PossibleAbstractClass
{
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
class SomeClass extends PossibleAbstractClass
{
}
abstract class PossibleAbstractClass
{
}
CODE_SAMPLE
),
]);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [Class_::class];
}
/**
* @param Class_ $node
*/
public function refactor(Node $node): ?Node
{
$className = $this->getName($node);
if ($className === null) {
return null;
}
// 1. is in static call?
if ($this->parsedNodesByType->findMethodCallsOnClass($className)) {
return null;
}
// 2. is in new?
if ($this->parsedNodesByType->findNewNodesByClass($className)) {
return null;
}
// 3. does it have any children
if (! $this->parsedNodesByType->findChildrenOfClass($className)) {
return null;
}
// is abstract!
if ($node->isAbstract()) {
return null;
}
$this->makeAbstract($node);
return $node;
}
}

View File

@ -0,0 +1,27 @@
<?php
namespace Rector\SOLID\Tests\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector\Fixture;
class SomeClass extends PossibleAbstractClass
{
}
class PossibleAbstractClass
{
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector\Fixture;
class SomeClass extends PossibleAbstractClass
{
}
abstract class PossibleAbstractClass
{
}
?>

View File

@ -0,0 +1,26 @@
<?php
namespace Rector\SOLID\Tests\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector\Fixture;
class SkipMethodCall
{
public static function run()
{
}
}
class ClassCalling
{
public function go($skipMethodCall)
{
/** @var SkipMethodCall $skipMethodCall */
$skipMethodCall->run();
}
}
function localFunction()
{
// just so the ClassCalling is not made abstract
$classCalling = new ClassCalling();
}

View File

@ -0,0 +1,12 @@
<?php
namespace Rector\SOLID\Tests\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector\Fixture;
class SkipNew
{
}
function runMePlease()
{
return new SkipNew();
}

View File

@ -0,0 +1,16 @@
<?php
namespace Rector\SOLID\Tests\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector\Fixture;
class SkipStaticCall
{
public static function run()
{
}
}
function run()
{
SkipStaticCall::run();
}

View File

@ -0,0 +1,23 @@
<?php declare(strict_types=1);
namespace Rector\SOLID\Tests\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector;
use Rector\SOLID\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
final class MakeUnusedClassesWithChildrenAbstractRectorTest extends AbstractRectorTestCase
{
public function test(): void
{
$this->doTestFiles([
__DIR__ . '/Fixture/fixture.php.inc',
__DIR__ . '/Fixture/skip_new.php.inc',
__DIR__ . '/Fixture/skip_static_call.php.inc',
]);
}
protected function getRectorClass(): string
{
return MakeUnusedClassesWithChildrenAbstractRector::class;
}
}

View File

@ -158,13 +158,6 @@ parameters:
- '#Method Rector\\Collector\\CallableCollectorPopulator\:\:populate\(\) should return array<Closure\> but returns array<int\|string, callable\>#'
- '#Access to an undefined property PhpParser\\Node\\Expr\:\:\$args#'
# waits on php-parser release
- '#ArrowFunction#'
- '#Parameter \#6 \$fixup \(int\|null\) of method Rector\\PhpParser\\Printer\\BetterStandardPrinter\:\:pArray\(\) should be compatible with parameter \$subNodeName \(string\) of method PhpParser\\PrettyPrinterAbstract\:\:pArray\(\)#'
- '#Parameter \#6 \$subNodeName of method PhpParser\\PrettyPrinterAbstract\:\:pArray\(\) expects string, int\|null given#'
- '#Parameter \#7 \$fixup of method PhpParser\\PrettyPrinterAbstract\:\:pArray\(\) expects int\|null, string\|null given#'
- '#Class PhpParser\\Node\\Expr\\ArrayItem constructor invoked with 5 parameters, 1\-4 required#'
- '#Parameter \#2 \$name of method Rector\\Rector\\AbstractRector\:\:isName\(\) expects string, string\|null given#'
# file always exists here
- '#Cannot call method getRealPath\(\) on Symplify\\PackageBuilder\\FileSystem\\SmartFileInfo\|null#'
@ -174,3 +167,6 @@ parameters:
# known value
- '#Parameter \#1 \$node of method Rector\\Rector\\AbstractRector\:\:getName\(\) expects PhpParser\\Node, PhpParser\\Node\\Identifier\|null given#'
- '#Cannot cast array<string\>\|bool\|string\|null to string#'
- '#Parameter \#1 \$node of method Rector\\PhpParser\\Node\\Manipulator\\VisibilityManipulator\:\:makeAbstract\(\) expects PhpParser\\Node\\Stmt\\Class_\|PhpParser\\Node\\Stmt\\ClassMethod, PhpParser\\Node given#'

View File

@ -15,4 +15,3 @@ parameters:
php_version_features: '7.1'
services:
Rector\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector: ~

View File

@ -413,6 +413,29 @@ final class ParsedNodesByType
return $this->methodsCallsByTypeAndMethod[$className][$methodName] ?? [];
}
/**
* @return MethodCall[]|StaticCall[]
*/
public function findMethodCallsOnClass(string $className): array
{
return $this->methodsCallsByTypeAndMethod[$className] ?? [];
}
/**
* @return New_[]
*/
public function findNewNodesByClass(string $className): array
{
$newNodesByClass = [];
foreach ($this->getNewNodes() as $newNode) {
if ($this->nameResolver->isName($newNode->class, $className)) {
$newNodesByClass[] = $newNode;
}
}
return $newNodesByClass;
}
private function addClass(Class_ $classNode): void
{
if ($this->isClassAnonymous($classNode)) {

View File

@ -63,7 +63,7 @@ final class PropertyManipulator
}
// is it the name match?
if ($this->nameResolver->resolve($node) !== $this->nameResolver->resolve($property)) {
if (! $this->nameResolver->areNamesEqual($node, $property)) {
return null;
}

View File

@ -14,7 +14,7 @@ final class VisibilityManipulator
/**
* @var string[]
*/
private $allowedNodeTypes = [ClassMethod::class, Property::class, ClassConst::class];
private $allowedNodeTypes = [ClassMethod::class, Property::class, ClassConst::class, Class_::class];
/**
* @param ClassMethod|Property|ClassConst $node
@ -24,6 +24,14 @@ final class VisibilityManipulator
$this->addVisibilityFlag($node, 'static');
}
/**
* @param ClassMethod|Class_ $node
*/
public function makeAbstract(Node $node): void
{
$this->addVisibilityFlag($node, 'abstract');
}
/**
* @param ClassMethod|Property|ClassConst $node
*/
@ -31,7 +39,7 @@ final class VisibilityManipulator
{
$visibility = strtolower($visibility);
if ($visibility !== 'static') {
if ($visibility !== 'static' && $visibility !== 'abstract') {
$this->removeOriginalVisibilityFromFlags($node);
}
@ -66,7 +74,7 @@ final class VisibilityManipulator
}
/**
* @param ClassMethod|Property|ClassConst $node
* @param Class_|ClassMethod|Property|ClassConst $node
*/
private function addVisibilityFlag(Node $node, string $visibility): void
{
@ -87,6 +95,10 @@ final class VisibilityManipulator
if ($visibility === 'static') {
$node->flags |= Class_::MODIFIER_STATIC;
}
if ($visibility === 'abstract') {
$node->flags |= Class_::MODIFIER_ABSTRACT;
}
}
private function ensureIsClassMethodOrProperty(Node $node, string $location): void

View File

@ -3,6 +3,7 @@
namespace Rector\Rector;
use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassConst;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Property;
@ -51,6 +52,14 @@ trait VisibilityTrait
}
}
/**
* @param ClassMethod|Class_ $node
*/
public function makeAbstract(Node $node): void
{
$this->visibilityManipulator->makeAbstract($node);
}
/**
* @param ClassMethod|Property|ClassConst $node
*/